From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)postgreSQL(dot)org |
Cc: | Chad Wagner <chad(dot)wagner(at)gmail(dot)com> |
Subject: | Preventing tuple-table leakage in plpgsql |
Date: | 2013-07-05 18:47:06 |
Message-ID: | 24295.1373050026@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Bug #8279 exhibits an intra-transaction memory leak in a plpgsql
function that repeatedly traps an error. The cause of the leak is that
the SPITupleTable created during exec_stmt_execsql is never cleaned up.
(It will get cleaned up at function exit, but that's not soon enough in
this usage.)
The submitter proposes fixing this by inserting some more
SPI_freetuptable calls in exec_stmt_execsql, but that's not much of a
fix. The patch only covers the two ereports associated with "strict"
mode (and not, say, any errors thrown in functions called by
exec_stmt_execsql). Nor does it do anything for similar leakage
scenarios elsewhere. I count at least four other functions with the
same kind of oversight in plpgsql, and there are suspicious-looking
usages elsewhere as well.
One approach we could take is to insert PG_TRY blocks to ensure that
SPI_freetuptable is called on error exit from such functions. (plpython
seems to have adopted this solution already.) However, that tends to be
pretty messy notationally, and possibly could represent a noticeable
performance hit depending on what you assume about the speed of
sigsetjmp(). Moreover, fixing known trouble spots this way does nothing
to guard against similar errors elsewhere.
So I'm inclined to propose that SPI itself should offer some mechanism
for cleaning up tuple tables at subtransaction abort. We could just
have it automatically throw away tuple tables made in the current
subtransaction, or we could allow callers to exercise some control,
perhaps by calling a function that says "don't reclaim this tuple table
automatically". I'm not sure if there's any real use-case for such a
call though.
It's also not very clear to me if tuple tables should be thrown away
automatically at subtransaction commit. We could do that, or leave
things alone, or add some logic to emit warning bleats about unreleased
tuple tables (comparable to what is done for many other resource types).
If we change anything about the commit case, I think we run significant
risk of breaking third-party code that works now, so maybe it's best
to leave that alone.
It might also be worth debating whether to back-patch such a fix.
This issue has been there ever since plpgsql grew the ability to trap
errors, so the lack of previous reports might mean that it's not worth
taking risks to fix such leaks in back branches.
Comments, better ideas?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Josh Berkus | 2013-07-05 18:49:31 | Re: Changing recovery.conf parameters into GUCs |
Previous Message | Pavel Stehule | 2013-07-05 18:35:32 | Re: Proposal - Support for National Characters functionality |