Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3d13623

Browse files
committed
Prevent leakage of SPI tuple tables during subtransaction abort.
plpgsql often just remembers SPI-result tuple tables in local variables,and has no mechanism for freeing them if an ereport(ERROR) causes an escapeout of the execution function whose local variable it is. In the originalcoding, that wasn't a problem because the tuple table would be cleaned upwhen the function's SPI context went away during transaction abort.However, once plpgsql grew the ability to trap exceptions, repeatedtrapping of errors within a function could result in significantintra-function-call memory leakage, as illustrated in bug #8279 fromChad Wagner.We could fix this locally in plpgsql with a bunch of PG_TRY/PG_CATCHcoding, but that would be tedious, probably slow, and prone to bugs ofomission; moreover it would do nothing for similar risks elsewhere.What seems like a better plan is to make SPI itself responsible forfreeing tuple tables at subtransaction abort. This patch attacks theproblem that way, keeping a list of live tuple tables within each SPIfunction context. Currently, such freeing is automatic for tuple tablesmade within the failed subtransaction. We might later add a SPI call tomark a tuple table as not to be freed this way, allowing callers to optout; but until someone exhibits a clear use-case for such behavior, itdoesn't seem worth bothering.A very useful side-effect of this change is that SPI_freetuptable() cannow defend itself against bad calls, such as duplicate free requests;this should make things more robust in many places. (In particular,this reduces the risks involved if a third-party extension containsnow-redundant SPI_freetuptable() calls in error cleanup code.)Even though the leakage problem is of long standing, it seems imprudentto back-patch this into stable branches, since it does represent an APIsemantics change for SPI users. We'll patch this in 9.3, but live withthe leakage in older branches.
1 parentfd27b99 commit3d13623

File tree

7 files changed

+121
-17
lines changed

7 files changed

+121
-17
lines changed

‎doc/src/sgml/spi.sgml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3934,8 +3934,8 @@ void SPI_freetuptable(SPITupleTable * <parameter>tuptable</parameter>)
39343934
<para>
39353935
<function>SPI_freetuptable</function> frees a row set created by a
39363936
prior SPI command execution function, such as
3937-
<function>SPI_execute</>. Therefore, this function isusually called
3938-
with the global variable <varname>SPI_tupletable</varname> as
3937+
<function>SPI_execute</>. Therefore, this function isoften called
3938+
with the global variable <varname>SPI_tuptable</varname> as
39393939
argument.
39403940
</para>
39413941

@@ -3944,6 +3944,16 @@ void SPI_freetuptable(SPITupleTable * <parameter>tuptable</parameter>)
39443944
multiple commands and does not want to keep the results of earlier
39453945
commands around until it ends. Note that any unfreed row sets will
39463946
be freed anyway at <function>SPI_finish</>.
3947+
Also, if a subtransaction is started and then aborted within execution
3948+
of a SPI procedure, SPI automatically frees any row sets created while
3949+
the subtransaction was running.
3950+
</para>
3951+
3952+
<para>
3953+
Beginning in <productname>PostgreSQL</> 9.3,
3954+
<function>SPI_freetuptable</function> contains guard logic to protect
3955+
against duplicate deletion requests for the same row set. In previous
3956+
releases, duplicate deletions would lead to crashes.
39473957
</para>
39483958
</refsect1>
39493959

@@ -3955,7 +3965,7 @@ void SPI_freetuptable(SPITupleTable * <parameter>tuptable</parameter>)
39553965
<term><literal>SPITupleTable * <parameter>tuptable</parameter></literal></term>
39563966
<listitem>
39573967
<para>
3958-
pointer to row set to free
3968+
pointer to row set to free, or NULL to do nothing
39593969
</para>
39603970
</listitem>
39613971
</varlistentry>

‎src/backend/executor/spi.c

Lines changed: 95 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ SPI_connect(void)
126126
_SPI_current->processed=0;
127127
_SPI_current->lastoid=InvalidOid;
128128
_SPI_current->tuptable=NULL;
129+
slist_init(&_SPI_current->tuptables);
129130
_SPI_current->procCxt=NULL;/* in case we fail to create 'em */
130131
_SPI_current->execCxt=NULL;
131132
_SPI_current->connectSubid=GetCurrentSubTransactionId();
@@ -166,7 +167,7 @@ SPI_finish(void)
166167
/* Restore memory context as it was before procedure call */
167168
MemoryContextSwitchTo(_SPI_current->savedcxt);
168169

169-
/* Release memory used in procedure call */
170+
/* Release memory used in procedure call(including tuptables)*/
170171
MemoryContextDelete(_SPI_current->execCxt);
171172
_SPI_current->execCxt=NULL;
172173
MemoryContextDelete(_SPI_current->procCxt);
@@ -282,11 +283,35 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
282283
*/
283284
if (_SPI_current&& !isCommit)
284285
{
286+
slist_mutable_itersiter;
287+
285288
/* free Executor memory the same as _SPI_end_call would do */
286289
MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
287-
/* throw away any partially created tuple-table */
288-
SPI_freetuptable(_SPI_current->tuptable);
289-
_SPI_current->tuptable=NULL;
290+
291+
/* throw away any tuple tables created within current subxact */
292+
slist_foreach_modify(siter,&_SPI_current->tuptables)
293+
{
294+
SPITupleTable*tuptable;
295+
296+
tuptable=slist_container(SPITupleTable,next,siter.cur);
297+
if (tuptable->subid >=mySubid)
298+
{
299+
/*
300+
* If we used SPI_freetuptable() here, its internal search of
301+
* the tuptables list would make this operation O(N^2).
302+
* Instead, just free the tuptable manually. This should
303+
* match what SPI_freetuptable() does.
304+
*/
305+
slist_delete_current(&siter);
306+
if (tuptable==_SPI_current->tuptable)
307+
_SPI_current->tuptable=NULL;
308+
if (tuptable==SPI_tuptable)
309+
SPI_tuptable=NULL;
310+
MemoryContextDelete(tuptable->tuptabcxt);
311+
}
312+
}
313+
/* in particular we should have gotten rid of any in-progress table */
314+
Assert(_SPI_current->tuptable==NULL);
290315
}
291316
}
292317

@@ -1021,8 +1046,59 @@ SPI_freetuple(HeapTuple tuple)
10211046
void
10221047
SPI_freetuptable(SPITupleTable*tuptable)
10231048
{
1024-
if (tuptable!=NULL)
1025-
MemoryContextDelete(tuptable->tuptabcxt);
1049+
boolfound= false;
1050+
1051+
/* ignore call if NULL pointer */
1052+
if (tuptable==NULL)
1053+
return;
1054+
1055+
/*
1056+
* Since this function might be called during error recovery, it seems
1057+
* best not to insist that the caller be actively connected. We just
1058+
* search the topmost SPI context, connected or not.
1059+
*/
1060+
if (_SPI_connected >=0)
1061+
{
1062+
slist_mutable_itersiter;
1063+
1064+
if (_SPI_current!=&(_SPI_stack[_SPI_connected]))
1065+
elog(ERROR,"SPI stack corrupted");
1066+
1067+
/* find tuptable in active list, then remove it */
1068+
slist_foreach_modify(siter,&_SPI_current->tuptables)
1069+
{
1070+
SPITupleTable*tt;
1071+
1072+
tt=slist_container(SPITupleTable,next,siter.cur);
1073+
if (tt==tuptable)
1074+
{
1075+
slist_delete_current(&siter);
1076+
found= true;
1077+
break;
1078+
}
1079+
}
1080+
}
1081+
1082+
/*
1083+
* Refuse the deletion if we didn't find it in the topmost SPI context.
1084+
* This is primarily a guard against double deletion, but might prevent
1085+
* other errors as well. Since the worst consequence of not deleting a
1086+
* tuptable would be a transient memory leak, this is just a WARNING.
1087+
*/
1088+
if (!found)
1089+
{
1090+
elog(WARNING,"attempt to delete invalid SPITupleTable %p",tuptable);
1091+
return;
1092+
}
1093+
1094+
/* for safety, reset global variables that might point at tuptable */
1095+
if (tuptable==_SPI_current->tuptable)
1096+
_SPI_current->tuptable=NULL;
1097+
if (tuptable==SPI_tuptable)
1098+
SPI_tuptable=NULL;
1099+
1100+
/* release all memory belonging to tuptable */
1101+
MemoryContextDelete(tuptable->tuptabcxt);
10261102
}
10271103

10281104

@@ -1656,6 +1732,8 @@ spi_dest_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
16561732
if (_SPI_current->tuptable!=NULL)
16571733
elog(ERROR,"improper call to spi_dest_startup");
16581734

1735+
/* We create the tuple table context as a child of procCxt */
1736+
16591737
oldcxt=_SPI_procmem();/* switch to procedure memory context */
16601738

16611739
tuptabcxt=AllocSetContextCreate(CurrentMemoryContext,
@@ -1666,8 +1744,18 @@ spi_dest_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
16661744
MemoryContextSwitchTo(tuptabcxt);
16671745

16681746
_SPI_current->tuptable=tuptable= (SPITupleTable*)
1669-
palloc(sizeof(SPITupleTable));
1747+
palloc0(sizeof(SPITupleTable));
16701748
tuptable->tuptabcxt=tuptabcxt;
1749+
tuptable->subid=GetCurrentSubTransactionId();
1750+
1751+
/*
1752+
* The tuptable is now valid enough to be freed by AtEOSubXact_SPI, so put
1753+
* it onto the SPI context's tuptables list. This will ensure it's not
1754+
* leaked even in the unlikely event the following few lines fail.
1755+
*/
1756+
slist_push_head(&_SPI_current->tuptables,&tuptable->next);
1757+
1758+
/* set up initial allocations */
16711759
tuptable->alloced=tuptable->free=128;
16721760
tuptable->vals= (HeapTuple*)palloc(tuptable->alloced*sizeof(HeapTuple));
16731761
tuptable->tupdesc=CreateTupleDescCopy(typeinfo);

‎src/include/executor/spi.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndefSPI_H
1414
#defineSPI_H
1515

16+
#include"lib/ilist.h"
1617
#include"nodes/parsenodes.h"
1718
#include"utils/portal.h"
1819

@@ -24,6 +25,8 @@ typedef struct SPITupleTable
2425
uint32free;/* # of free vals */
2526
TupleDesctupdesc;/* tuple descriptor */
2627
HeapTuple*vals;/* tuples */
28+
slist_nodenext;/* link for internal bookkeeping */
29+
SubTransactionIdsubid;/* subxact in which tuptable was created */
2730
}SPITupleTable;
2831

2932
/* Plans are opaque structs for standard users of SPI */

‎src/include/executor/spi_priv.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ typedef struct
2323
/* current results */
2424
uint32processed;/* by Executor */
2525
Oidlastoid;
26-
SPITupleTable*tuptable;
26+
SPITupleTable*tuptable;/* tuptable currently being built */
2727

28+
/* resources of this execution context */
29+
slist_headtuptables;/* list of all live SPITupleTables */
2830
MemoryContextprocCxt;/* procedure context */
2931
MemoryContextexecCxt;/* executor context */
3032
MemoryContextsavedcxt;/* context of SPI_connect's caller */

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1202,7 +1202,13 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
12021202
*/
12031203
SPI_restore_connection();
12041204

1205-
/* Must clean up the econtext too */
1205+
/*
1206+
* Must clean up the econtext too.However, any tuple table made
1207+
* in the subxact will have been thrown away by SPI during subxact
1208+
* abort, so we don't need to (and mustn't try to) free the
1209+
* eval_tuptable.
1210+
*/
1211+
estate->eval_tuptable=NULL;
12061212
exec_eval_cleanup(estate);
12071213

12081214
/* Look for a matching exception handler */

‎src/pl/plpython/plpy_cursorobject.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,6 @@ PLy_cursor_iternext(PyObject *self)
377377
}
378378
PG_CATCH();
379379
{
380-
SPI_freetuptable(SPI_tuptable);
381-
382380
PLy_spi_subtransaction_abort(oldcontext,oldowner);
383381
returnNULL;
384382
}
@@ -461,8 +459,6 @@ PLy_cursor_fetch(PyObject *self, PyObject *args)
461459
}
462460
PG_CATCH();
463461
{
464-
SPI_freetuptable(SPI_tuptable);
465-
466462
PLy_spi_subtransaction_abort(oldcontext,oldowner);
467463
returnNULL;
468464
}

‎src/pl/plpython/plpy_spi.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,6 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, int rows, int status)
439439
{
440440
MemoryContextSwitchTo(oldcontext);
441441
PLy_typeinfo_dealloc(&args);
442-
SPI_freetuptable(tuptable);
443442
Py_DECREF(result);
444443
PG_RE_THROW();
445444
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp