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

Commit011eae6

Browse files
committed
Fix error cleanup failure caused by 8.4 changes in plpgsql to try to avoid
memory leakage in error recovery. We were calling FreeExprContext, andtherefore invoking ExprContextCallback callbacks, in both normal and errorexits from subtransactions. However this isn't very safe, as shown inrecent trouble report from Frank van Vugt, in which releasing a tupledescrefcount failed. It's also unnecessary, since the resources that callbacksmight wish to release should be cleaned up by other error recovery mechanisms(ie the resource owners). We only really want FreeExprContext to releasememory attached to the exprcontext in the error-exit case. So, add a boolparameter to FreeExprContext to tell it not to call the callbacks.A more general solution would be to pass the isCommit bool parameter on tothe callbacks, so they could do only safe things during error exit. Butthat would make the patch significantly more invasive and possibly breakthird-party code that registers ExprContextCallback callbacks. We might wantto do that later in HEAD, but for now I'll just do what seems reasonable toback-patch.
1 parentfb18055 commit011eae6

File tree

5 files changed

+28
-17
lines changed

5 files changed

+28
-17
lines changed

‎src/backend/executor/execUtils.c

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.159 2009/06/11 14:48:57 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.160 2009/07/18 19:15:41 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -68,7 +68,7 @@ intNIndexTupleProcessed;
6868

6969

7070
staticboolget_last_attnums(Node*node,ProjectionInfo*projInfo);
71-
staticvoidShutdownExprContext(ExprContext*econtext);
71+
staticvoidShutdownExprContext(ExprContext*econtext,boolisCommit);
7272

7373

7474
/* ----------------------------------------------------------------
@@ -257,7 +257,8 @@ FreeExecutorState(EState *estate)
257257
* XXX: seems there ought to be a faster way to implement this than
258258
* repeated list_delete(), no?
259259
*/
260-
FreeExprContext((ExprContext*)linitial(estate->es_exprcontexts));
260+
FreeExprContext((ExprContext*)linitial(estate->es_exprcontexts),
261+
true);
261262
/* FreeExprContext removed the list link for us */
262263
}
263264

@@ -408,16 +409,21 @@ CreateStandaloneExprContext(void)
408409
* Since we free the temporary context used for expression evaluation,
409410
* any previously computed pass-by-reference expression result will go away!
410411
*
412+
* If isCommit is false, we are being called in error cleanup, and should
413+
* not call callbacks but only release memory. (It might be better to call
414+
* the callbacks and pass the isCommit flag to them, but that would require
415+
* more invasive code changes than currently seems justified.)
416+
*
411417
* Note we make no assumption about the caller's memory context.
412418
* ----------------
413419
*/
414420
void
415-
FreeExprContext(ExprContext*econtext)
421+
FreeExprContext(ExprContext*econtext,boolisCommit)
416422
{
417423
EState*estate;
418424

419425
/* Call any registered callbacks */
420-
ShutdownExprContext(econtext);
426+
ShutdownExprContext(econtext,isCommit);
421427
/* And clean up the memory used */
422428
MemoryContextDelete(econtext->ecxt_per_tuple_memory);
423429
/* Unlink self from owning EState, if any */
@@ -442,7 +448,7 @@ void
442448
ReScanExprContext(ExprContext*econtext)
443449
{
444450
/* Call any registered callbacks */
445-
ShutdownExprContext(econtext);
451+
ShutdownExprContext(econtext, true);
446452
/* And clean up the memory used */
447453
MemoryContextReset(econtext->ecxt_per_tuple_memory);
448454
}
@@ -1222,9 +1228,12 @@ UnregisterExprContextCallback(ExprContext *econtext,
12221228
*
12231229
* The callback list is emptied (important in case this is only a rescan
12241230
* reset, and not deletion of the ExprContext).
1231+
*
1232+
* If isCommit is false, just clean the callback list but don't call 'em.
1233+
* (See comment for FreeExprContext.)
12251234
*/
12261235
staticvoid
1227-
ShutdownExprContext(ExprContext*econtext)
1236+
ShutdownExprContext(ExprContext*econtext,boolisCommit)
12281237
{
12291238
ExprContext_CB*ecxt_callback;
12301239
MemoryContextoldcontext;
@@ -1245,7 +1254,8 @@ ShutdownExprContext(ExprContext *econtext)
12451254
while ((ecxt_callback=econtext->ecxt_callbacks)!=NULL)
12461255
{
12471256
econtext->ecxt_callbacks=ecxt_callback->next;
1248-
(*ecxt_callback->function) (ecxt_callback->arg);
1257+
if (isCommit)
1258+
(*ecxt_callback->function) (ecxt_callback->arg);
12491259
pfree(ecxt_callback);
12501260
}
12511261

‎src/backend/executor/nodeBitmapIndexscan.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.30 2009/06/11 14:48:57 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.31 2009/07/18 19:15:41 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -182,7 +182,7 @@ ExecEndBitmapIndexScan(BitmapIndexScanState *node)
182182
*/
183183
#ifdefNOT_USED
184184
if (node->biss_RuntimeContext)
185-
FreeExprContext(node->biss_RuntimeContext);
185+
FreeExprContext(node->biss_RuntimeContext, true);
186186
#endif
187187

188188
/*

‎src/backend/executor/nodeIndexscan.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.132 2009/06/11 14:48:57 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.133 2009/07/18 19:15:41 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -423,7 +423,7 @@ ExecEndIndexScan(IndexScanState *node)
423423
#ifdefNOT_USED
424424
ExecFreeExprContext(&node->ss.ps);
425425
if (node->iss_RuntimeContext)
426-
FreeExprContext(node->iss_RuntimeContext);
426+
FreeExprContext(node->iss_RuntimeContext, true);
427427
#endif
428428

429429
/*

‎src/include/executor/executor.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.155 2009/06/11 14:49:11 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/executor/executor.h,v 1.156 2009/07/18 19:15:42 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -255,7 +255,7 @@ extern EState *CreateExecutorState(void);
255255
externvoidFreeExecutorState(EState*estate);
256256
externExprContext*CreateExprContext(EState*estate);
257257
externExprContext*CreateStandaloneExprContext(void);
258-
externvoidFreeExprContext(ExprContext*econtext);
258+
externvoidFreeExprContext(ExprContext*econtext,boolisCommit);
259259
externvoidReScanExprContext(ExprContext*econtext);
260260

261261
#defineResetExprContext(econtext) \

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.244 2009/06/17 13:46:12 petere Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.245 2009/07/18 19:15:42 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -5237,7 +5237,7 @@ plpgsql_destroy_econtext(PLpgSQL_execstate *estate)
52375237
pfree(simple_econtext_stack);
52385238
simple_econtext_stack=next;
52395239

5240-
FreeExprContext(estate->eval_econtext);
5240+
FreeExprContext(estate->eval_econtext, true);
52415241
estate->eval_econtext=NULL;
52425242
}
52435243

@@ -5292,7 +5292,8 @@ plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
52925292
{
52935293
SimpleEcontextStackEntry*next;
52945294

5295-
FreeExprContext(simple_econtext_stack->stack_econtext);
5295+
FreeExprContext(simple_econtext_stack->stack_econtext,
5296+
(event==SUBXACT_EVENT_COMMIT_SUB));
52965297
next=simple_econtext_stack->next;
52975298
pfree(simple_econtext_stack);
52985299
simple_econtext_stack=next;

0 commit comments

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