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

Commitc7b849a

Browse files
committed
Prevent leakage of cached plans and execution trees in plpgsql DO blocks.
plpgsql likes to cache query plans and simple-expression execution statetrees across calls. This is a considerable win for multiple executionsof the same function. However, it's useless for DO blocks, since bydefinition those are executed only once and discarded. Nonetheless,we were allowing a DO block's expression execution trees to surviveuntil end of transaction, resulting in a significant intra-transactionmemory leak, as reported by Yeb Havinga. Worse, if the DO block exitedwith an error, the compiled form of the block's code was leaked tillend of session --- along with subsidiary plancache entries.To fix, make DO blocks keep their expression execution trees in a privateEState that's deleted at exit from the block, and add a PG_TRY blockto plpgsql_inline_handler to make sure that memory cleanup happenseven on error exits. Also add a regression test covering error handlingin a DO block, because my first try at this broke that. (The test isnot meant to prove that we don't leak memory anymore, though it couldbe used for that with a much larger loop count.)Ideally we'd back-patch this into all versions supporting DO blocks;but the patch needs to add a field to struct PLpgSQL_execstate, and thatwould break ABI compatibility for third-party plugins such as the plpgsqldebugger. Given the small number of complaints so far, fixing this inHEAD only seems like an acceptable choice.
1 parent80e3a47 commitc7b849a

File tree

5 files changed

+150
-22
lines changed

5 files changed

+150
-22
lines changed

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

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@ typedef struct
6666
* so that we don't have to re-prepare simple expressions on each trip through
6767
* a function.(We assume the case to optimize is many repetitions of a
6868
* function within a transaction.)
69+
*
70+
* However, there's no value in trying to amortize simple expression setup
71+
* across multiple executions of a DO block (inline code block), since there
72+
* can never be any. If we use the shared EState for a DO block, the expr
73+
* state trees are effectively leaked till end of transaction, and that can
74+
* add up if the user keeps on submitting DO blocks. Therefore, each DO block
75+
* has its own simple-expression EState, which is cleaned up at exit from
76+
* plpgsql_inline_handler(). DO blocks still use the simple_econtext_stack,
77+
* though, so that subxact abort cleanup does the right thing.
6978
*/
7079
typedefstructSimpleEcontextStackEntry
7180
{
@@ -74,7 +83,7 @@ typedef struct SimpleEcontextStackEntry
7483
structSimpleEcontextStackEntry*next;/* next stack entry up */
7584
}SimpleEcontextStackEntry;
7685

77-
staticEState*simple_eval_estate=NULL;
86+
staticEState*shared_simple_eval_estate=NULL;
7887
staticSimpleEcontextStackEntry*simple_econtext_stack=NULL;
7988

8089
/************************************************************
@@ -136,7 +145,8 @@ static int exec_stmt_dynfors(PLpgSQL_execstate *estate,
136145

137146
staticvoidplpgsql_estate_setup(PLpgSQL_execstate*estate,
138147
PLpgSQL_function*func,
139-
ReturnSetInfo*rsi);
148+
ReturnSetInfo*rsi,
149+
EState*simple_eval_estate);
140150
staticvoidexec_eval_cleanup(PLpgSQL_execstate*estate);
141151

142152
staticvoidexec_prepare_plan(PLpgSQL_execstate*estate,
@@ -230,10 +240,17 @@ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
230240
/* ----------
231241
* plpgsql_exec_functionCalled by the call handler for
232242
*function execution.
243+
*
244+
* This is also used to execute inline code blocks (DO blocks). The only
245+
* difference that this code is aware of is that for a DO block, we want
246+
* to use a private simple_eval_estate, which is created and passed in by
247+
* the caller. For regular functions, pass NULL, which implies using
248+
* shared_simple_eval_estate.
233249
* ----------
234250
*/
235251
Datum
236-
plpgsql_exec_function(PLpgSQL_function*func,FunctionCallInfofcinfo)
252+
plpgsql_exec_function(PLpgSQL_function*func,FunctionCallInfofcinfo,
253+
EState*simple_eval_estate)
237254
{
238255
PLpgSQL_execstateestate;
239256
ErrorContextCallbackplerrcontext;
@@ -243,7 +260,8 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo)
243260
/*
244261
* Setup the execution state
245262
*/
246-
plpgsql_estate_setup(&estate,func, (ReturnSetInfo*)fcinfo->resultinfo);
263+
plpgsql_estate_setup(&estate,func, (ReturnSetInfo*)fcinfo->resultinfo,
264+
simple_eval_estate);
247265

248266
/*
249267
* Setup error traceback support for ereport()
@@ -503,7 +521,7 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
503521
/*
504522
* Setup the execution state
505523
*/
506-
plpgsql_estate_setup(&estate,func,NULL);
524+
plpgsql_estate_setup(&estate,func,NULL,NULL);
507525

508526
/*
509527
* Setup error traceback support for ereport()
@@ -782,7 +800,7 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
782800
/*
783801
* Setup the execution state
784802
*/
785-
plpgsql_estate_setup(&estate,func,NULL);
803+
plpgsql_estate_setup(&estate,func,NULL,NULL);
786804

787805
/*
788806
* Setup error traceback support for ereport()
@@ -3087,7 +3105,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
30873105
staticvoid
30883106
plpgsql_estate_setup(PLpgSQL_execstate*estate,
30893107
PLpgSQL_function*func,
3090-
ReturnSetInfo*rsi)
3108+
ReturnSetInfo*rsi,
3109+
EState*simple_eval_estate)
30913110
{
30923111
/* this link will be restored at exit from plpgsql_call_handler */
30933112
func->cur_estate=estate;
@@ -3126,6 +3145,12 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
31263145
estate->datums=palloc(sizeof(PLpgSQL_datum*)*estate->ndatums);
31273146
/* caller is expected to fill the datums array */
31283147

3148+
/* set up for use of appropriate simple-expression EState */
3149+
if (simple_eval_estate)
3150+
estate->simple_eval_estate=simple_eval_estate;
3151+
else
3152+
estate->simple_eval_estate=shared_simple_eval_estate;
3153+
31293154
estate->eval_tuptable=NULL;
31303155
estate->eval_processed=0;
31313156
estate->eval_lastoid=InvalidOid;
@@ -5148,7 +5173,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
51485173
*/
51495174
if (expr->expr_simple_lxid!=curlxid)
51505175
{
5151-
oldcontext=MemoryContextSwitchTo(simple_eval_estate->es_query_cxt);
5176+
oldcontext=MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt);
51525177
expr->expr_simple_state=ExecInitExpr(expr->expr_simple_expr,NULL);
51535178
expr->expr_simple_in_use= false;
51545179
expr->expr_simple_lxid=curlxid;
@@ -6190,8 +6215,8 @@ exec_set_found(PLpgSQL_execstate *estate, bool state)
61906215
/*
61916216
* plpgsql_create_econtext --- create an eval_econtext for the current function
61926217
*
6193-
* We may need to create a newsimple_eval_estate too, if there's not one
6194-
* already for the current transaction. The EState will be cleaned up at
6218+
* We may need to create a newshared_simple_eval_estate too, if there's not
6219+
*onealready for the current transaction. The EState will be cleaned up at
61956220
* transaction end.
61966221
*/
61976222
staticvoid
@@ -6203,20 +6228,25 @@ plpgsql_create_econtext(PLpgSQL_execstate *estate)
62036228
* Create an EState for evaluation of simple expressions, if there's not
62046229
* one already in the current transaction.The EState is made a child of
62056230
* TopTransactionContext so it will have the right lifespan.
6231+
*
6232+
* Note that this path is never taken when executing a DO block; the
6233+
* required EState was already made by plpgsql_inline_handler.
62066234
*/
6207-
if (simple_eval_estate==NULL)
6235+
if (estate->simple_eval_estate==NULL)
62086236
{
62096237
MemoryContextoldcontext;
62106238

6239+
Assert(shared_simple_eval_estate==NULL);
62116240
oldcontext=MemoryContextSwitchTo(TopTransactionContext);
6212-
simple_eval_estate=CreateExecutorState();
6241+
shared_simple_eval_estate=CreateExecutorState();
6242+
estate->simple_eval_estate=shared_simple_eval_estate;
62136243
MemoryContextSwitchTo(oldcontext);
62146244
}
62156245

62166246
/*
62176247
* Create a child econtext for the current function.
62186248
*/
6219-
estate->eval_econtext=CreateExprContext(simple_eval_estate);
6249+
estate->eval_econtext=CreateExprContext(estate->simple_eval_estate);
62206250

62216251
/*
62226252
* Make a stack entry so we can clean up the econtext at subxact end.
@@ -6275,14 +6305,14 @@ plpgsql_xact_cb(XactEvent event, void *arg)
62756305
/* Shouldn't be any econtext stack entries left at commit */
62766306
Assert(simple_econtext_stack==NULL);
62776307

6278-
if (simple_eval_estate)
6279-
FreeExecutorState(simple_eval_estate);
6280-
simple_eval_estate=NULL;
6308+
if (shared_simple_eval_estate)
6309+
FreeExecutorState(shared_simple_eval_estate);
6310+
shared_simple_eval_estate=NULL;
62816311
}
62826312
elseif (event==XACT_EVENT_ABORT)
62836313
{
62846314
simple_econtext_stack=NULL;
6285-
simple_eval_estate=NULL;
6315+
shared_simple_eval_estate=NULL;
62866316
}
62876317
}
62886318

@@ -6291,8 +6321,7 @@ plpgsql_xact_cb(XactEvent event, void *arg)
62916321
*
62926322
* Make sure any simple-expression econtexts created in the current
62936323
* subtransaction get cleaned up. We have to do this explicitly because
6294-
* no other code knows which child econtexts of simple_eval_estate belong
6295-
* to which level of subxact.
6324+
* no other code knows which econtexts belong to which level of subxact.
62966325
*/
62976326
void
62986327
plpgsql_subxact_cb(SubXactEventevent,SubTransactionIdmySubid,

‎src/pl/plpgsql/src/pl_handler.c‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS)
136136
retval= (Datum)0;
137137
}
138138
else
139-
retval=plpgsql_exec_function(func,fcinfo);
139+
retval=plpgsql_exec_function(func,fcinfo,NULL);
140140
}
141141
PG_CATCH();
142142
{
@@ -175,6 +175,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
175175
PLpgSQL_function*func;
176176
FunctionCallInfoDatafake_fcinfo;
177177
FmgrInfoflinfo;
178+
EState*simple_eval_estate;
178179
Datumretval;
179180
intrc;
180181

@@ -203,7 +204,51 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
203204
flinfo.fn_oid=InvalidOid;
204205
flinfo.fn_mcxt=CurrentMemoryContext;
205206

206-
retval=plpgsql_exec_function(func,&fake_fcinfo);
207+
/* Create a private EState for simple-expression execution */
208+
simple_eval_estate=CreateExecutorState();
209+
210+
/* And run the function */
211+
PG_TRY();
212+
{
213+
retval=plpgsql_exec_function(func,&fake_fcinfo,simple_eval_estate);
214+
}
215+
PG_CATCH();
216+
{
217+
/*
218+
* We need to clean up what would otherwise be long-lived resources
219+
* accumulated by the failed DO block, principally cached plans for
220+
* statements (which can be flushed with plpgsql_free_function_memory)
221+
* and execution trees for simple expressions, which are in the
222+
* private EState.
223+
*
224+
* Before releasing the private EState, we must clean up any
225+
* simple_econtext_stack entries pointing into it, which we can do by
226+
* invoking the subxact callback. (It will be called again later if
227+
* some outer control level does a subtransaction abort, but no harm
228+
* is done.) We cheat a bit knowing that plpgsql_subxact_cb does not
229+
* pay attention to its parentSubid argument.
230+
*/
231+
plpgsql_subxact_cb(SUBXACT_EVENT_ABORT_SUB,
232+
GetCurrentSubTransactionId(),
233+
0,NULL);
234+
235+
/* Clean up the private EState */
236+
FreeExecutorState(simple_eval_estate);
237+
238+
/* Function should now have no remaining use-counts ... */
239+
func->use_count--;
240+
Assert(func->use_count==0);
241+
242+
/* ... so we can free subsidiary storage */
243+
plpgsql_free_function_memory(func);
244+
245+
/* And propagate the error */
246+
PG_RE_THROW();
247+
}
248+
PG_END_TRY();
249+
250+
/* Clean up the private EState */
251+
FreeExecutorState(simple_eval_estate);
207252

208253
/* Function should now have no remaining use-counts ... */
209254
func->use_count--;

‎src/pl/plpgsql/src/plpgsql.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -773,10 +773,14 @@ typedef struct PLpgSQL_execstate
773773
ResourceOwnertuple_store_owner;
774774
ReturnSetInfo*rsi;
775775

776+
/* the datums representing the function's local variables */
776777
intfound_varno;
777778
intndatums;
778779
PLpgSQL_datum**datums;
779780

781+
/* EState to use for "simple" expression evaluation */
782+
EState*simple_eval_estate;
783+
780784
/* temporary state for results from evaluation of query or expr */
781785
SPITupleTable*eval_tuptable;
782786
uint32eval_processed;
@@ -943,7 +947,8 @@ extern Datum plpgsql_validator(PG_FUNCTION_ARGS);
943947
* ----------
944948
*/
945949
externDatumplpgsql_exec_function(PLpgSQL_function*func,
946-
FunctionCallInfofcinfo);
950+
FunctionCallInfofcinfo,
951+
EState*simple_eval_estate);
947952
externHeapTupleplpgsql_exec_trigger(PLpgSQL_function*func,
948953
TriggerData*trigdata);
949954
externvoidplpgsql_exec_event_trigger(PLpgSQL_function*func,

‎src/test/regress/expected/plpgsql.out‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4651,6 +4651,35 @@ LINE 1: SELECT rtrim(roomno) AS roomno, foo FROM Room ORDER BY roomn...
46514651
^
46524652
QUERY: SELECT rtrim(roomno) AS roomno, foo FROM Room ORDER BY roomno
46534653
CONTEXT: PL/pgSQL function inline_code_block line 4 at FOR over SELECT rows
4654+
-- Check handling of errors thrown from/into anonymous code blocks.
4655+
do $outer$
4656+
begin
4657+
for i in 1..10 loop
4658+
begin
4659+
execute $ex$
4660+
do $$
4661+
declare x int = 0;
4662+
begin
4663+
x := 1 / x;
4664+
end;
4665+
$$;
4666+
$ex$;
4667+
exception when division_by_zero then
4668+
raise notice 'caught division by zero';
4669+
end;
4670+
end loop;
4671+
end;
4672+
$outer$;
4673+
NOTICE: caught division by zero
4674+
NOTICE: caught division by zero
4675+
NOTICE: caught division by zero
4676+
NOTICE: caught division by zero
4677+
NOTICE: caught division by zero
4678+
NOTICE: caught division by zero
4679+
NOTICE: caught division by zero
4680+
NOTICE: caught division by zero
4681+
NOTICE: caught division by zero
4682+
NOTICE: caught division by zero
46544683
-- Check variable scoping -- a var is not available in its own or prior
46554684
-- default expressions.
46564685
create function scope_test() returns int as $$

‎src/test/regress/sql/plpgsql.sql‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3751,6 +3751,26 @@ BEGIN
37513751
END LOOP;
37523752
END$$;
37533753
3754+
-- Check handling of errors thrown from/into anonymous code blocks.
3755+
do $outer$
3756+
begin
3757+
for i in 1..10 loop
3758+
begin
3759+
execute $ex$
3760+
do $$
3761+
declare x int = 0;
3762+
begin
3763+
x := 1 / x;
3764+
end;
3765+
$$;
3766+
$ex$;
3767+
exception when division_by_zero then
3768+
raise notice'caught division by zero';
3769+
end;
3770+
end loop;
3771+
end;
3772+
$outer$;
3773+
37543774
-- Check variable scoping -- a var is not available in its own or prior
37553775
-- default expressions.
37563776

0 commit comments

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