Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fa7f6ff

Browse files
committed
Fix possible crash at transaction end when a plpgsql function is used and
then modified within the same transaction. The code was using a linked list of active PLpgSQL_expr structs, which was OK when it was written because plpgsql never released any parse data structures for the life of the backend. But since Neil fixed plpgsql's memory management, elements of the linked list could be freed, leading to crash when the list is chased. Per report and test case from Kris Jurka.
1 parent 181f55e commit fa7f6ff

File tree

2 files changed

+13
-28
lines changed

2 files changed

+13
-28
lines changed

src/pl/plpgsql/src/pl_exec.c

+7-25
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* procedural language
44
*
55
* IDENTIFICATION
6-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.160 2006/01/10 18:50:43 neilc Exp $
6+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.161 2006/03/02 05:34:12 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -64,12 +64,8 @@ static const char *const raise_skip_msg = "RAISE";
6464
* function call creates its own "eval_econtext" ExprContext within this
6565
* estate. We destroy the estate at transaction shutdown to ensure there
6666
* is no permanent leakage of memory (especially for xact abort case).
67-
*
68-
* If a simple PLpgSQL_expr has been used in the current xact, it is
69-
* linked into the active_simple_exprs list.
7067
*/
7168
static EState *simple_eval_estate = NULL;
72-
static PLpgSQL_expr *active_simple_exprs = NULL;
7369

7470
/************************************************************
7571
* Local function forward declarations
@@ -3783,6 +3779,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
37833779
{
37843780
Datum retval;
37853781
ExprContext *econtext = estate->eval_econtext;
3782+
TransactionId curxid = GetTopTransactionId();
37863783
ParamListInfo paramLI;
37873784
int i;
37883785
Snapshot saveActiveSnapshot;
@@ -3796,13 +3793,11 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
37963793
* Prepare the expression for execution, if it's not been done already in
37973794
* the current transaction.
37983795
*/
3799-
if (expr->expr_simple_state == NULL)
3796+
if (expr->expr_simple_xid != curxid)
38003797
{
38013798
expr->expr_simple_state = ExecPrepareExpr(expr->expr_simple_expr,
38023799
simple_eval_estate);
3803-
/* Add it to list for cleanup */
3804-
expr->expr_simple_next = active_simple_exprs;
3805-
active_simple_exprs = expr;
3800+
expr->expr_simple_xid = curxid;
38063801
}
38073802

38083803
/*
@@ -4454,11 +4449,11 @@ exec_simple_check_plan(PLpgSQL_expr *expr)
44544449

44554450
/*
44564451
* Yes - this is a simple expression. Mark it as such, and initialize
4457-
* state to "not executing".
4452+
* state to "not valid in current transaction".
44584453
*/
44594454
expr->expr_simple_expr = tle->expr;
44604455
expr->expr_simple_state = NULL;
4461-
expr->expr_simple_next = NULL;
4456+
expr->expr_simple_xid = InvalidTransactionId;
44624457
/* Also stash away the expression result type */
44634458
expr->expr_simple_type = exprType((Node *) tle->expr);
44644459
}
@@ -4502,8 +4497,7 @@ exec_set_found(PLpgSQL_execstate *estate, bool state)
45024497
* plpgsql_xact_cb --- post-transaction-commit-or-abort cleanup
45034498
*
45044499
* If a simple_eval_estate was created in the current transaction,
4505-
* it has to be cleaned up, and we have to mark all active PLpgSQL_expr
4506-
* structs that are using it as no longer active.
4500+
* it has to be cleaned up.
45074501
*
45084502
* XXX Do we need to do anything at subtransaction events?
45094503
* Maybe subtransactions need to have their own simple_eval_estate?
@@ -4512,18 +4506,6 @@ exec_set_found(PLpgSQL_execstate *estate, bool state)
45124506
void
45134507
plpgsql_xact_cb(XactEvent event, void *arg)
45144508
{
4515-
PLpgSQL_expr *expr;
4516-
PLpgSQL_expr *enext;
4517-
4518-
/* Mark all active exprs as inactive */
4519-
for (expr = active_simple_exprs; expr; expr = enext)
4520-
{
4521-
enext = expr->expr_simple_next;
4522-
expr->expr_simple_state = NULL;
4523-
expr->expr_simple_next = NULL;
4524-
}
4525-
active_simple_exprs = NULL;
4526-
45274509
/*
45284510
* If we are doing a clean transaction shutdown, free the EState (so that
45294511
* any remaining resources will be released correctly). In an abort, we

src/pl/plpgsql/src/plpgsql.h

+6-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* procedural language
44
*
55
* IDENTIFICATION
6-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.66 2005/11/22 18:17:33 momjian Exp $
6+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.67 2006/03/02 05:34:12 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -200,9 +200,12 @@ typedef struct PLpgSQL_expr
200200
/* fields for "simple expression" fast-path execution: */
201201
Expr *expr_simple_expr; /* NULL means not a simple expr */
202202
Oid expr_simple_type;
203-
/* if expr is simple AND in use in current xact, these fields are set: */
203+
/*
204+
* if expr is simple AND in use in current xact, expr_simple_state is
205+
* valid. Test validity by seeing if expr_simple_xid matches current XID.
206+
*/
204207
ExprState *expr_simple_state;
205-
struct PLpgSQL_expr *expr_simple_next;
208+
TransactionId expr_simple_xid;
206209
/* params to pass to expr */
207210
int nparams;
208211
int params[1]; /* VARIABLE SIZE ARRAY ... must be last */

0 commit comments

Comments
 (0)