From: Tom Lane Date: Mon, 15 Nov 1999 03:28:07 +0000 (+0000) Subject: Clean up possible memory leakage in nodeSubplan X-Git-Tag: REL7_0~1172 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c8c3e07e5833d4d6dac60c8e3aa772e9dac1c079;p=postgresql Clean up possible memory leakage in nodeSubplan --- diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index dbb824fe64..65c42b61b3 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -6,7 +6,7 @@ * Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v 1.16 1999/11/15 02:00:01 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/executor/nodeSubplan.c,v 1.17 1999/11/15 03:28:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -107,9 +107,18 @@ ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext, bool *isNull) if (found) elog(ERROR, "ExecSubPlan: more than one tuple returned by expression subselect"); found = true; - /* XXX need to copy tuple in case pass by ref */ - /* XXX need to ref-count the tuple to avoid mem leak! */ + /* + * We need to copy the subplan's tuple in case the result is of + * pass-by-ref type --- our return value will point into this + * copied tuple! Can't use the subplan's instance of the tuple + * since it won't still be valid after next ExecProcNode() call. + * node->curTuple keeps track of the copied tuple for eventual + * freeing. + */ tup = heap_copytuple(tup); + if (node->curTuple) + pfree(node->curTuple); + node->curTuple = tup; result = heap_getattr(tup, col, tdesc, isNull); /* keep scanning subplan to make sure there's only one tuple */ continue; @@ -253,10 +262,13 @@ ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent) ExecCreateTupleTable(ExecCountSlotsNode(node->plan) + 10); sp_estate->es_snapshot = estate->es_snapshot; + node->shutdown = false; + node->curTuple = NULL; + if (!ExecInitNode(node->plan, sp_estate, NULL)) return false; - node->shutdown = true; + node->shutdown = true; /* now we need to shutdown the subplan */ /* * If this plan is un-correlated or undirect correlated one and want @@ -332,13 +344,15 @@ ExecSetParamPlan(SubPlan *node) found = true; /* - * If this is uncorrelated subquery then its plan will be closed - * (see below) and this tuple will be free-ed - bad for not byval - * types... But is free-ing possible in the next ExecProcNode in - * this loop ? Who knows... Someday we'll keep track of saved - * tuples... + * We need to copy the subplan's tuple in case any of the params + * are pass-by-ref type --- the pointers stored in the param structs + * will point at this copied tuple! node->curTuple keeps track + * of the copied tuple for eventual freeing. */ tup = heap_copytuple(tup); + if (node->curTuple) + pfree(node->curTuple); + node->curTuple = tup; foreach(lst, node->setParam) { @@ -387,13 +401,16 @@ ExecSetParamPlan(SubPlan *node) void ExecEndSubPlan(SubPlan *node) { - if (node->shutdown) { ExecEndNode(node->plan, node->plan); node->shutdown = false; } - + if (node->curTuple) + { + pfree(node->curTuple); + node->curTuple = NULL; + } } void diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index d353df58bd..5f23a954b1 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.95 1999/11/15 02:00:01 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.96 1999/11/15 03:28:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -551,6 +551,10 @@ _copySubPlan(SubPlan *from) newnode->parParam = listCopy(from->parParam); Node_Copy(from, newnode, sublink); + /* do not copy execution state */ + newnode->shutdown = false; + newnode->curTuple = NULL; + return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 2e3d67da60..fccb9d3160 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.50 1999/10/07 04:23:04 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.51 1999/11/15 03:28:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -406,9 +406,13 @@ _equalIndexScan(IndexScan *a, IndexScan *b) static bool _equalSubPlan(SubPlan *a, SubPlan *b) { + /* should compare plans, but have to settle for comparing plan IDs */ if (a->plan_id != b->plan_id) return false; + if (!equal(a->rtable, b->rtable)) + return false; + if (!equal(a->sublink, b->sublink)) return false; diff --git a/src/backend/nodes/freefuncs.c b/src/backend/nodes/freefuncs.c index cad49be76b..db09b6700b 100644 --- a/src/backend/nodes/freefuncs.c +++ b/src/backend/nodes/freefuncs.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/nodes/Attic/freefuncs.c,v 1.26 1999/08/21 03:48:57 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/nodes/Attic/freefuncs.c,v 1.27 1999/11/15 03:28:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -441,6 +441,9 @@ _freeSubPlan(SubPlan *node) freeList(node->parParam); freeObject(node->sublink); + if (node->curTuple) + pfree(node->curTuple); + pfree(node); } diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index e4dc5fde21..00e7025917 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: plannodes.h,v 1.32 1999/11/15 02:00:13 tgl Exp $ + * $Id: plannodes.h,v 1.33 1999/11/15 03:28:06 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -352,13 +352,18 @@ typedef struct SubPlan * funcs for plan nodes... actually, we * could put *plan itself somewhere else * (TopPlan node ?)... */ - List *rtable; /* range table */ + List *rtable; /* range table for subselect */ + /* setParam and parParam are lists of integers (param IDs) */ List *setParam; /* non-correlated EXPR & EXISTS subqueries * have to set some Params for paren Plan */ List *parParam; /* indices of corr. Vars from parent plan */ SubLink *sublink; /* SubLink node from parser; holds info about * what to do with subselect's results */ - bool shutdown; /* shutdown plan if TRUE */ + /* + * Remaining fields are working state for executor; not used in planning + */ + bool shutdown; /* TRUE = need to shutdown plan */ + HeapTuple curTuple; /* copy of most recent tuple from subplan */ } SubPlan; #endif /* PLANNODES_H */