From c8c3e07e5833d4d6dac60c8e3aa772e9dac1c079 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 15 Nov 1999 03:28:07 +0000
Subject: [PATCH] Clean up possible memory leakage in nodeSubplan
---
src/backend/executor/nodeSubplan.c | 39 +++++++++++++++++++++---------
src/backend/nodes/copyfuncs.c | 6 ++++-
src/backend/nodes/equalfuncs.c | 6 ++++-
src/backend/nodes/freefuncs.c | 5 +++-
src/include/nodes/plannodes.h | 11 ++++++---
5 files changed, 50 insertions(+), 17 deletions(-)
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index dbb824fe64c..65c42b61b3b 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 d353df58bdc..5f23a954b13 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 2e3d67da608..fccb9d31608 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 cad49be76bd..db09b6700bc 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 e4dc5fde210..00e7025917d 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 */
--
GitLab