Preventing tuple-table leakage in plpgsql

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

Responses

Browse pgsql-hackers by date

  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