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

Commitc1e044b

Browse files
committed
Fix memory leak in plpgsql's CALL processing.
When executing a CALL or DO in a non-atomic context (i.e., not insidea function or query), plpgsql creates a new plan each time through,as a rather hacky solution to some resource management issues. Butit failed to free this plan until exit of the current procedure or DOblock, resulting in serious memory bloat in procedures that calledother procedures many times. Fix by remembering to free the plan,and by being more honest about restoring the previous state (otherwise,recursive procedure calls have a problem).There was also a smaller leak associated with recalculation of the"target" list of output variables. Fix that by using the statement-lifespan context to hold non-permanent values.Back-patch to v11 where procedures were introduced.Pavel Stehule and Tom LaneDiscussion:https://postgr.es/m/CAFj8pRDiiU1dqym+_P4_GuTWm76knJu7z9opWayBJTC0nQGUUA@mail.gmail.com
1 parent4d342b9 commitc1e044b

File tree

1 file changed

+70
-21
lines changed

1 file changed

+70
-21
lines changed

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

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2122,55 +2122,82 @@ exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt)
21222122

21232123
/*
21242124
* exec_stmt_call
2125+
*
2126+
* NOTE: this is used for both CALL and DO statements.
21252127
*/
21262128
staticint
21272129
exec_stmt_call(PLpgSQL_execstate*estate,PLpgSQL_stmt_call*stmt)
21282130
{
21292131
PLpgSQL_expr*expr=stmt->expr;
2132+
SPIPlanPtrorig_plan=expr->plan;
2133+
boollocal_plan;
2134+
PLpgSQL_variable*volatilecur_target=stmt->target;
21302135
volatileLocalTransactionIdbefore_lxid;
21312136
LocalTransactionIdafter_lxid;
21322137
volatileboolpushed_active_snap= false;
21332138
volatileintrc;
21342139

2140+
/*
2141+
* If not in atomic context, we make a local plan that we'll just use for
2142+
* this invocation, and will free at the end. Otherwise, transaction ends
2143+
* would cause errors about plancache leaks.
2144+
*
2145+
* XXX This would be fixable with some plancache/resowner surgery
2146+
* elsewhere, but for now we'll just work around this here.
2147+
*/
2148+
local_plan= !estate->atomic;
2149+
21352150
/* PG_TRY to ensure we clear the plan link, if needed, on failure */
21362151
PG_TRY();
21372152
{
21382153
SPIPlanPtrplan=expr->plan;
21392154
ParamListInfoparamLI;
21402155

2141-
if (plan==NULL)
2156+
/*
2157+
* Make a plan if we don't have one, or if we need a local one. Note
2158+
* that we'll overwrite expr->plan either way; the PG_TRY block will
2159+
* ensure we undo that on the way out, if the plan is local.
2160+
*/
2161+
if (plan==NULL||local_plan)
21422162
{
2163+
/* Don't let SPI save the plan if it's going to be local */
2164+
exec_prepare_plan(estate,expr,0, !local_plan);
2165+
plan=expr->plan;
21432166

21442167
/*
2145-
* Don't save the plan if not in atomic context. Otherwise,
2146-
* transaction ends would cause errors about plancache leaks.
2147-
*
2148-
* XXX This would be fixable with some plancache/resowner surgery
2149-
* elsewhere, but for now we'll just work around this here.
2168+
* A CALL or DO can never be a simple expression. (If it could
2169+
* be, we'd have to worry about saving/restoring the previous
2170+
* values of the related expr fields, not just expr->plan.)
21502171
*/
2151-
exec_prepare_plan(estate,expr,0,estate->atomic);
2172+
Assert(!expr->expr_simple_expr);
21522173

21532174
/*
21542175
* The procedure call could end transactions, which would upset
21552176
* the snapshot management in SPI_execute*, so don't let it do it.
21562177
* Instead, we set the snapshots ourselves below.
21572178
*/
2158-
plan=expr->plan;
21592179
plan->no_snapshots= true;
21602180

21612181
/*
21622182
* Force target to be recalculated whenever the plan changes, in
21632183
* case the procedure's argument list has changed.
21642184
*/
21652185
stmt->target=NULL;
2186+
cur_target=NULL;
21662187
}
21672188

21682189
/*
21692190
* We construct a DTYPE_ROW datum representing the plpgsql variables
21702191
* associated with the procedure's output arguments. Then we can use
21712192
* exec_move_row() to do the assignments.
2193+
*
2194+
* If we're using a local plan, also make a local target; otherwise,
2195+
* since the above code will force a new plan each time through, we'd
2196+
* repeatedly leak the memory for the target. (Note: we also leak the
2197+
* target when a plan change is forced, but that isn't so likely to
2198+
* cause excessive memory leaks.)
21722199
*/
2173-
if (stmt->is_call&&stmt->target==NULL)
2200+
if (stmt->is_call&&cur_target==NULL)
21742201
{
21752202
Node*node;
21762203
FuncExpr*funcexpr;
@@ -2185,6 +2212,9 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
21852212
inti;
21862213
ListCell*lc;
21872214

2215+
/* Use eval_mcontext for any cruft accumulated here */
2216+
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
2217+
21882218
/*
21892219
* Get the parsed CallStmt, and look up the called procedure
21902220
*/
@@ -2216,17 +2246,20 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
22162246
ReleaseSysCache(func_tuple);
22172247

22182248
/*
2219-
* Begin constructing row Datum
2249+
* Begin constructing row Datum; keep it in fn_cxt if it's to be
2250+
* long-lived.
22202251
*/
2221-
oldcontext=MemoryContextSwitchTo(estate->func->fn_cxt);
2252+
if (!local_plan)
2253+
MemoryContextSwitchTo(estate->func->fn_cxt);
22222254

22232255
row= (PLpgSQL_row*)palloc0(sizeof(PLpgSQL_row));
22242256
row->dtype=PLPGSQL_DTYPE_ROW;
22252257
row->refname="(unnamed row)";
22262258
row->lineno=-1;
22272259
row->varnos= (int*)palloc(sizeof(int)*list_length(funcargs));
22282260

2229-
MemoryContextSwitchTo(oldcontext);
2261+
if (!local_plan)
2262+
MemoryContextSwitchTo(get_eval_mcontext(estate));
22302263

22312264
/*
22322265
* Examine procedure's argument list. Each output arg position
@@ -2270,7 +2303,13 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
22702303

22712304
row->nfields=nfields;
22722305

2273-
stmt->target= (PLpgSQL_variable*)row;
2306+
cur_target= (PLpgSQL_variable*)row;
2307+
2308+
/* We can save and re-use the target datum, if it's not local */
2309+
if (!local_plan)
2310+
stmt->target=cur_target;
2311+
2312+
MemoryContextSwitchTo(oldcontext);
22742313
}
22752314

22762315
paramLI=setup_param_list(estate,expr);
@@ -2293,17 +2332,27 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
22932332
PG_CATCH();
22942333
{
22952334
/*
2296-
* If we aren't saving the plan, unset the pointer. Note that it
2297-
* could have been unset already, in case of a recursive call.
2335+
* If we are using a local plan, restore the old plan link.
22982336
*/
2299-
if (expr->plan&& !expr->plan->saved)
2300-
expr->plan=NULL;
2337+
if (local_plan)
2338+
expr->plan=orig_plan;
23012339
PG_RE_THROW();
23022340
}
23032341
PG_END_TRY();
23042342

2305-
if (expr->plan&& !expr->plan->saved)
2306-
expr->plan=NULL;
2343+
/*
2344+
* If we are using a local plan, restore the old plan link; then free the
2345+
* local plan to avoid memory leaks. (Note that the error exit path above
2346+
* just clears the link without risking calling SPI_freeplan; we expect
2347+
* that xact cleanup will take care of the mess in that case.)
2348+
*/
2349+
if (local_plan)
2350+
{
2351+
SPIPlanPtrplan=expr->plan;
2352+
2353+
expr->plan=orig_plan;
2354+
SPI_freeplan(plan);
2355+
}
23072356

23082357
if (rc<0)
23092358
elog(ERROR,"SPI_execute_plan_with_paramlist failed executing query \"%s\": %s",
@@ -2339,10 +2388,10 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
23392388
{
23402389
SPITupleTable*tuptab=SPI_tuptable;
23412390

2342-
if (!stmt->target)
2391+
if (!cur_target)
23432392
elog(ERROR,"DO statement returned a row");
23442393

2345-
exec_move_row(estate,stmt->target,tuptab->vals[0],tuptab->tupdesc);
2394+
exec_move_row(estate,cur_target,tuptab->vals[0],tuptab->tupdesc);
23462395
}
23472396
elseif (SPI_processed>1)
23482397
elog(ERROR,"procedure call returned more than one row");

0 commit comments

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