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

Commita8e0b66

Browse files
committed
Fix up plpgsql's "simple expression" evaluation mechanism so that it behaves
safely in the presence of subtransactions. To ensure that any ExprContextshutdown callbacks are called at the right times, we have to have a separateEState for each level of subtransaction. Per "TupleDesc reference leak" bugreport from Stefan Kaltenbrunner.Although I'm convinced the code is wrong as far back as 8.0, it doesn't seemthat there are any ways for the problem to really manifest before 8.2: AFAICS,8.0 and 8.1 only use the ExprContextCallback mechanism to handle set-returningfunctions, which cannot usefully be executed in a "simple expression" anyway.Hence, no backpatch before 8.2 --- the risk of unforeseen breakage seemsto outweigh the chance of fixing something.
1 parent76c7d2a commita8e0b66

File tree

3 files changed

+156
-46
lines changed

3 files changed

+156
-46
lines changed

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

Lines changed: 143 additions & 40 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.183 2007/01/09 22:01:00 momjian Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.184 2007/01/28 16:15:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -37,15 +37,33 @@
3737

3838
staticconstchar*constraise_skip_msg="RAISE";
3939

40-
4140
/*
42-
* All plpgsql function executions within a single transaction share
43-
* the same executor EState for evaluating "simple" expressions. Each
44-
* function call creates its own "eval_econtext" ExprContext within this
45-
* estate.We destroy the estate at transaction shutdown to ensure there
46-
* is no permanent leakage of memory (especially for xact abort case).
41+
* All plpgsql function executions within a single transaction share the same
42+
* executor EState for evaluating "simple" expressions. Each function call
43+
* creates its own "eval_econtext" ExprContext within this estate for
44+
* per-evaluation workspace. eval_econtext is freed at normal function exit,
45+
* and the EState is freed at transaction end (in case of error, we assume
46+
* that the abort mechanisms clean it all up). In order to be sure
47+
* ExprContext callbacks are handled properly, each subtransaction has to have
48+
* its own such EState; hence we need a stack. We use a simple counter to
49+
* distinguish different instantiations of the EState, so that we can tell
50+
* whether we have a current copy of a prepared expression.
51+
*
52+
* This arrangement is a bit tedious to maintain, but it's worth the trouble
53+
* so that we don't have to re-prepare simple expressions on each trip through
54+
* a function. (We assume the case to optimize is many repetitions of a
55+
* function within a transaction.)
4756
*/
48-
staticEState*simple_eval_estate=NULL;
57+
typedefstructSimpleEstateStackEntry
58+
{
59+
EState*xact_eval_estate;/* EState for current xact level */
60+
longintxact_estate_simple_id;/* ID for xact_eval_estate */
61+
SubTransactionIdxact_subxid;/* ID for current subxact */
62+
structSimpleEstateStackEntry*next;/* next stack entry up */
63+
}SimpleEstateStackEntry;
64+
65+
staticSimpleEstateStackEntry*simple_estate_stack=NULL;
66+
staticlongintsimple_estate_id_counter=0;
4967

5068
/************************************************************
5169
* Local function forward declarations
@@ -154,6 +172,7 @@ static Datum exec_simple_cast_value(Datum value, Oid valtype,
154172
staticvoidexec_init_tuple_store(PLpgSQL_execstate*estate);
155173
staticboolcompatible_tupdesc(TupleDesctd1,TupleDesctd2);
156174
staticvoidexec_set_found(PLpgSQL_execstate*estate,boolstate);
175+
staticvoidplpgsql_create_econtext(PLpgSQL_execstate*estate);
157176
staticvoidfree_var(PLpgSQL_var*var);
158177

159178

@@ -892,20 +911,37 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
892911
*/
893912
MemoryContextoldcontext=CurrentMemoryContext;
894913
ResourceOwneroldowner=CurrentResourceOwner;
914+
ExprContext*old_eval_econtext=estate->eval_econtext;
915+
EState*old_eval_estate=estate->eval_estate;
916+
longintold_eval_estate_simple_id=estate->eval_estate_simple_id;
895917

896918
BeginInternalSubTransaction(NULL);
897919
/* Want to run statements inside function's memory context */
898920
MemoryContextSwitchTo(oldcontext);
899921

900922
PG_TRY();
901923
{
924+
/*
925+
* We need to run the block's statements with a new eval_econtext
926+
* that belongs to the current subtransaction; if we try to use
927+
* the outer econtext then ExprContext shutdown callbacks will be
928+
* called at the wrong times.
929+
*/
930+
plpgsql_create_econtext(estate);
931+
932+
/* Run the block's statements */
902933
rc=exec_stmts(estate,block->body);
903934

904935
/* Commit the inner transaction, return to outer xact context */
905936
ReleaseCurrentSubTransaction();
906937
MemoryContextSwitchTo(oldcontext);
907938
CurrentResourceOwner=oldowner;
908939

940+
/* Revert to outer eval_econtext */
941+
estate->eval_econtext=old_eval_econtext;
942+
estate->eval_estate=old_eval_estate;
943+
estate->eval_estate_simple_id=old_eval_estate_simple_id;
944+
909945
/*
910946
* AtEOSubXact_SPI() should not have popped any SPI context, but
911947
* just in case it did, make sure we remain connected.
@@ -927,6 +963,11 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
927963
MemoryContextSwitchTo(oldcontext);
928964
CurrentResourceOwner=oldowner;
929965

966+
/* Revert to outer eval_econtext */
967+
estate->eval_econtext=old_eval_econtext;
968+
estate->eval_estate=old_eval_estate;
969+
estate->eval_estate_simple_id=old_eval_estate_simple_id;
970+
930971
/*
931972
* If AtEOSubXact_SPI() popped any SPI context of the subxact, it
932973
* will have left us in a disconnected state. We need this hack
@@ -2139,24 +2180,9 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
21392180
estate->err_text=NULL;
21402181

21412182
/*
2142-
* Create an EState for evaluation of simple expressions, if there's not
2143-
* one already in the current transaction.The EState is made a child of
2144-
* TopTransactionContext so it will have the right lifespan.
2183+
* Create an EState and ExprContext for evaluation of simple expressions.
21452184
*/
2146-
if (simple_eval_estate==NULL)
2147-
{
2148-
MemoryContextoldcontext;
2149-
2150-
oldcontext=MemoryContextSwitchTo(TopTransactionContext);
2151-
simple_eval_estate=CreateExecutorState();
2152-
MemoryContextSwitchTo(oldcontext);
2153-
}
2154-
2155-
/*
2156-
* Create an expression context for simple expressions. This must be a
2157-
* child of simple_eval_estate.
2158-
*/
2159-
estate->eval_econtext=CreateExprContext(simple_eval_estate);
2185+
plpgsql_create_econtext(estate);
21602186

21612187
/*
21622188
* Let the plugin see this function before we initialize any local
@@ -3917,7 +3943,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
39173943
{
39183944
Datumretval;
39193945
ExprContext*econtext=estate->eval_econtext;
3920-
TransactionIdcurxid=GetTopTransactionId();
39213946
ParamListInfoparamLI;
39223947
inti;
39233948
SnapshotsaveActiveSnapshot;
@@ -3929,13 +3954,13 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
39293954

39303955
/*
39313956
* Prepare the expression for execution, if it's not been done already in
3932-
* the currenttransaction.
3957+
* the currenteval_estate.
39333958
*/
3934-
if (expr->expr_simple_xid!=curxid)
3959+
if (expr->expr_simple_id!=estate->eval_estate_simple_id)
39353960
{
39363961
expr->expr_simple_state=ExecPrepareExpr(expr->expr_simple_expr,
3937-
simple_eval_estate);
3938-
expr->expr_simple_xid=curxid;
3962+
estate->eval_estate);
3963+
expr->expr_simple_id=estate->eval_estate_simple_id;
39393964
}
39403965

39413966
/*
@@ -4612,7 +4637,7 @@ exec_simple_check_plan(PLpgSQL_expr *expr)
46124637
*/
46134638
expr->expr_simple_expr=tle->expr;
46144639
expr->expr_simple_state=NULL;
4615-
expr->expr_simple_xid=InvalidTransactionId;
4640+
expr->expr_simple_id=-1;
46164641
/* Also stash away the expression result type */
46174642
expr->expr_simple_type=exprType((Node*)tle->expr);
46184643
}
@@ -4652,15 +4677,56 @@ exec_set_found(PLpgSQL_execstate *estate, bool state)
46524677
var->isnull= false;
46534678
}
46544679

4680+
/*
4681+
* plpgsql_create_econtext --- create an eval_econtext for the current function
4682+
*
4683+
* We may need to create a new eval_estate too, if there's not one already
4684+
* for the current (sub) transaction. The EState will be cleaned up at
4685+
* (sub) transaction end.
4686+
*/
4687+
staticvoid
4688+
plpgsql_create_econtext(PLpgSQL_execstate*estate)
4689+
{
4690+
SubTransactionIdmy_subxid=GetCurrentSubTransactionId();
4691+
SimpleEstateStackEntry*entry=simple_estate_stack;
4692+
4693+
/* Create new EState if not one for current subxact */
4694+
if (entry==NULL||
4695+
entry->xact_subxid!=my_subxid)
4696+
{
4697+
MemoryContextoldcontext;
4698+
4699+
/* Stack entries are kept in TopTransactionContext for simplicity */
4700+
entry= (SimpleEstateStackEntry*)
4701+
MemoryContextAlloc(TopTransactionContext,
4702+
sizeof(SimpleEstateStackEntry));
4703+
4704+
/* But each EState should be a child of its CurTransactionContext */
4705+
oldcontext=MemoryContextSwitchTo(CurTransactionContext);
4706+
entry->xact_eval_estate=CreateExecutorState();
4707+
MemoryContextSwitchTo(oldcontext);
4708+
4709+
/* Assign a reasonably-unique ID to this EState */
4710+
entry->xact_estate_simple_id=simple_estate_id_counter++;
4711+
entry->xact_subxid=my_subxid;
4712+
4713+
entry->next=simple_estate_stack;
4714+
simple_estate_stack=entry;
4715+
}
4716+
4717+
/* Link plpgsql estate to it */
4718+
estate->eval_estate=entry->xact_eval_estate;
4719+
estate->eval_estate_simple_id=entry->xact_estate_simple_id;
4720+
4721+
/* And create a child econtext for the current function */
4722+
estate->eval_econtext=CreateExprContext(estate->eval_estate);
4723+
}
4724+
46554725
/*
46564726
* plpgsql_xact_cb --- post-transaction-commit-or-abort cleanup
46574727
*
4658-
* If asimple_eval_estate was created in the current transaction,
4728+
* If asimple-expression EState was created in the current transaction,
46594729
* it has to be cleaned up.
4660-
*
4661-
* XXX Do we need to do anything at subtransaction events?
4662-
* Maybe subtransactions need to have their own simple_eval_estate?
4663-
* It would get a lot messier, so for now let's assume we don't need that.
46644730
*/
46654731
void
46664732
plpgsql_xact_cb(XactEventevent,void*arg)
@@ -4669,11 +4735,48 @@ plpgsql_xact_cb(XactEvent event, void *arg)
46694735
* If we are doing a clean transaction shutdown, free the EState (so that
46704736
* any remaining resources will be released correctly). In an abort, we
46714737
* expect the regular abort recovery procedures to release everything of
4672-
* interest.
4738+
* interest. We don't need to free the individual stack entries since
4739+
* TopTransactionContext is about to go away anyway.
4740+
*
4741+
* Note: if plpgsql_subxact_cb is doing its job, there should be at most
4742+
* one stack entry, but we may as well code this as a loop.
46734743
*/
4674-
if (event==XACT_EVENT_COMMIT&&simple_eval_estate)
4675-
FreeExecutorState(simple_eval_estate);
4676-
simple_eval_estate=NULL;
4744+
if (event!=XACT_EVENT_ABORT)
4745+
{
4746+
while (simple_estate_stack!=NULL)
4747+
{
4748+
FreeExecutorState(simple_estate_stack->xact_eval_estate);
4749+
simple_estate_stack=simple_estate_stack->next;
4750+
}
4751+
}
4752+
else
4753+
simple_estate_stack=NULL;
4754+
}
4755+
4756+
/*
4757+
* plpgsql_subxact_cb --- post-subtransaction-commit-or-abort cleanup
4758+
*
4759+
* If a simple-expression EState was created in the current subtransaction,
4760+
* it has to be cleaned up.
4761+
*/
4762+
void
4763+
plpgsql_subxact_cb(SubXactEventevent,SubTransactionIdmySubid,
4764+
SubTransactionIdparentSubid,void*arg)
4765+
{
4766+
if (event==SUBXACT_EVENT_START_SUB)
4767+
return;
4768+
4769+
if (simple_estate_stack!=NULL&&
4770+
simple_estate_stack->xact_subxid==mySubid)
4771+
{
4772+
SimpleEstateStackEntry*next;
4773+
4774+
if (event==SUBXACT_EVENT_COMMIT_SUB)
4775+
FreeExecutorState(simple_estate_stack->xact_eval_estate);
4776+
next=simple_estate_stack->next;
4777+
pfree(simple_estate_stack);
4778+
simple_estate_stack=next;
4779+
}
46774780
}
46784781

46794782
staticvoid

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_handler.c,v 1.34 2007/01/05 22:20:02 momjian Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_handler.c,v 1.35 2007/01/28 16:15:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -46,6 +46,7 @@ _PG_init(void)
4646

4747
plpgsql_HashTableInit();
4848
RegisterXactCallback(plpgsql_xact_cb,NULL);
49+
RegisterSubXactCallback(plpgsql_subxact_cb,NULL);
4950

5051
/* Set up a rendezvous point with optional instrumentation plugin */
5152
plugin_ptr= (PLpgSQL_plugin**)find_rendezvous_variable("PLpgSQL_plugin");

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.82 2007/01/05 22:20:02 momjian Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.83 2007/01/28 16:15:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -180,11 +180,13 @@ typedef struct PLpgSQL_expr
180180
Oidexpr_simple_type;
181181

182182
/*
183-
* if expr is simple AND in use in current xact, expr_simple_state is
184-
* valid. Test validity by seeing if expr_simple_xid matches current XID.
183+
* if expr is simple AND prepared in current eval_estate,
184+
* expr_simple_state is valid. Test validity by seeing if expr_simple_id
185+
* matches eval_estate_simple_id.
185186
*/
186187
ExprState*expr_simple_state;
187-
TransactionIdexpr_simple_xid;
188+
longintexpr_simple_id;
189+
188190
/* params to pass to expr */
189191
intnparams;
190192
intparams[1];/* VARIABLE SIZE ARRAY ... must be last */
@@ -612,7 +614,9 @@ typedef struct
612614
SPITupleTable*eval_tuptable;
613615
uint32eval_processed;
614616
Oideval_lastoid;
615-
ExprContext*eval_econtext;
617+
ExprContext*eval_econtext;/* for executing simple expressions */
618+
EState*eval_estate;/* EState containing eval_econtext */
619+
longinteval_estate_simple_id;/* ID for eval_estate */
616620

617621
/* status information for error context reporting */
618622
PLpgSQL_function*err_func;/* current func */
@@ -738,6 +742,8 @@ extern Datum plpgsql_exec_function(PLpgSQL_function *func,
738742
externHeapTupleplpgsql_exec_trigger(PLpgSQL_function*func,
739743
TriggerData*trigdata);
740744
externvoidplpgsql_xact_cb(XactEventevent,void*arg);
745+
externvoidplpgsql_subxact_cb(SubXactEventevent,SubTransactionIdmySubid,
746+
SubTransactionIdparentSubid,void*arg);
741747

742748
/* ----------
743749
* Functions for the dynamic string handling in pl_funcs.c

0 commit comments

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