Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-25 Thread Tom Lane
I wrote: > Another point worth making is that this version of the patch deletes the > tuple tables during AtEOSubXact_SPI(), earlier in cleanup than would > happen with the prior version. That increases the risk that external > code might try to delete an already-deleted tuple table, if it tries >

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-24 Thread Tom Lane
I wrote: > Hmm ... good point. The other plan I'd been considering was to add > explicit tracking inside spi.c of all tuple tables created within the > current procedure, and then have AtEOSubXact_SPI flush any that were > created inside a failed subxact. The tables would still be children of > t

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-23 Thread Noah Misch
On Mon, Jul 22, 2013 at 10:02:30PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Sun, Jul 21, 2013 at 12:40:38PM -0400, Tom Lane wrote: > >> Hmm ... good point. The other plan I'd been considering was to add > >> explicit tracking inside spi.c of all tuple tables created within the > >> curr

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-22 Thread Tom Lane
Noah Misch writes: > On Sun, Jul 21, 2013 at 12:40:38PM -0400, Tom Lane wrote: >> Hmm ... good point. The other plan I'd been considering was to add >> explicit tracking inside spi.c of all tuple tables created within the >> current procedure, and then have AtEOSubXact_SPI flush any that were >>

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-22 Thread Noah Misch
On Sun, Jul 21, 2013 at 12:40:38PM -0400, Tom Lane wrote: > Noah Misch writes: > > Reasonable enough. Code that does use subtransactions will need to be more > > careful than before to manually free tuple tables in the non-error case. > > Failure to do so has been creating a leak that lasts until

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-21 Thread Tom Lane
Noah Misch writes: > On Fri, Jul 19, 2013 at 07:34:14PM -0400, Tom Lane wrote: >> However, we can use your idea when running inside a subtransaction, >> while still attaching the tuple table to the procedure's own procCxt >> when no subtransaction is involved. The attached draft patch does it >>

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-21 Thread Noah Misch
On Fri, Jul 19, 2013 at 07:34:14PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Fri, Jul 05, 2013 at 02:47:06PM -0400, Tom Lane wrote: > >> So I'm inclined to propose that SPI itself should offer some mechanism > >> for cleaning up tuple tables at subtransaction abort. We could just > >> ha

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-21 Thread Tom Lane
Noah Misch writes: > On Thu, Jul 11, 2013 at 09:14:38PM -0400, Chad Wagner wrote: >> Should SPI_connect be called again after the subtransaction is created? And >> SPI_finish before the subtransaction is committed or aborted? > Hmm. An SPI_push()+SPI_connect() every time PL/pgSQL starts a subtr

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-21 Thread Noah Misch
On Thu, Jul 11, 2013 at 09:14:38PM -0400, Chad Wagner wrote: > It looks like to me when AtEOSubXact_SPI is called the > _SPI_current->connectSubId is always 1 (since it is only set when > SPI_connect is called, which is only once for plpgsql), but the > CurrentSubTransactionId is incremented each t

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-19 Thread Tom Lane
Noah Misch writes: > On Fri, Jul 05, 2013 at 02:47:06PM -0400, Tom Lane wrote: >> 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 >> s

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-11 Thread Chad Wagner
It looks like to me exec_stmt_block creates a subtransaction if the block has an exception handler by calling BeginInternalSubTransaction. Then inside the PG_TRY it calls exec_stmts which runs the actual body of the begin block. If an exception is thrown then I presume we are hitting the PG_CATCH

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-11 Thread Chad Wagner
It looks like to me when AtEOSubXact_SPI is called the _SPI_current->connectSubId is always 1 (since it is only set when SPI_connect is called, which is only once for plpgsql), but the CurrentSubTransactionId is incremented each time a subtransaction is started. As a result, the memory for procCxt

Re: [HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-05 Thread Noah Misch
On Fri, Jul 05, 2013 at 02:47:06PM -0400, Tom Lane wrote: > 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

[HACKERS] Preventing tuple-table leakage in plpgsql

2013-07-05 Thread Tom Lane
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