Skip to content

Commit e932a72

Browse files
tglsfdctglsfdc
committed
To suppress memory leakage in long-lived Lists, lremove() should pfree
the cons cell it's deleting from the list. Do this, and fix a few callers that were bogusly assuming it wouldn't free the cons cell.
1 parent 9f76d0d commit e932a72

File tree

6 files changed

+51
-32
lines changed

6 files changed

+51
-32
lines changed

‎src/backend/nodes/list.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/nodes/list.c,v 1.42 2002/11/24 21:52:13 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/nodes/list.c,v 1.43 2002/12/17 01:18:18 tgl Exp $
1212
*
1313
* NOTES
1414
* XXX a few of the following functions are duplicated to handle
@@ -269,7 +269,6 @@ llasti(List *l)
269269
* Free the List nodes of a list
270270
* The pointed-to nodes, if any, are NOT freed.
271271
* This works for integer lists too.
272-
*
273272
*/
274273
void
275274
freeList(List *list)
@@ -487,6 +486,7 @@ lremove(void *elem, List *list)
487486
result = lnext(l);
488487
else
489488
lnext(prev) = lnext(l);
489+
pfree(l);
490490
}
491491
return result;
492492
}
@@ -518,6 +518,7 @@ LispRemove(void *elem, List *list)
518518
result = lnext(l);
519519
else
520520
lnext(prev) = lnext(l);
521+
pfree(l);
521522
}
522523
return result;
523524
}
@@ -545,6 +546,7 @@ lremovei(int elem, List *list)
545546
result = lnext(l);
546547
else
547548
lnext(prev) = lnext(l);
549+
pfree(l);
548550
}
549551
return result;
550552
}

‎src/backend/optimizer/path/pathkeys.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Portions Copyright (c) 1994, Regents of the University of California
1212
*
1313
* IDENTIFICATION
14-
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.42 2002/12/12 15:49:32 tgl Exp $
14+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.43 2002/12/17 01:18:22 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -101,12 +101,17 @@ add_equijoined_keys(Query *root, RestrictInfo *restrictinfo)
101101
*/
102102
newset = NIL;
103103

104-
foreach(cursetlink, root->equi_key_list)
104+
/* cannot use foreach here because of possible lremove */
105+
cursetlink = root->equi_key_list;
106+
while (cursetlink)
105107
{
106108
List *curset = lfirst(cursetlink);
107109
bool item1here = member(item1, curset);
108110
bool item2here = member(item2, curset);
109111

112+
/* must advance cursetlink before lremove possibly pfree's it */
113+
cursetlink = lnext(cursetlink);
114+
110115
if (item1here || item2here)
111116
{
112117
/*
@@ -128,9 +133,7 @@ add_equijoined_keys(Query *root, RestrictInfo *restrictinfo)
128133
newset = set_union(newset, curset);
129134

130135
/*
131-
* Remove old set from equi_key_list. NOTE this does not
132-
* change lnext(cursetlink), so the foreach loop doesn't
133-
* break.
136+
* Remove old set from equi_key_list.
134137
*/
135138
root->equi_key_list = lremove(curset, root->equi_key_list);
136139
freeList(curset); /* might as well recycle old cons cells */

‎src/backend/optimizer/plan/initsplan.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.78 2002/12/12 15:49:32 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.79 2002/12/17 01:18:25 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -910,13 +910,18 @@ qual_is_redundant(Query *root,
910910
do
911911
{
912912
someadded = false;
913-
foreach(olditem, oldquals)
913+
/* cannot use foreach here because of possible lremove */
914+
olditem = oldquals;
915+
while (olditem)
914916
{
915917
RestrictInfo *oldrinfo = (RestrictInfo *) lfirst(olditem);
916918
Node *oldleft = (Node *) get_leftop(oldrinfo->clause);
917919
Node *oldright = (Node *) get_rightop(oldrinfo->clause);
918920
Node *newguy = NULL;
919921

922+
/* must advance olditem before lremove possibly pfree's it */
923+
olditem = lnext(olditem);
924+
920925
if (member(oldleft, equalvars))
921926
newguy = oldright;
922927
else if (member(oldright, equalvars))
@@ -930,8 +935,6 @@ qual_is_redundant(Query *root,
930935

931936
/*
932937
* Remove this qual from list, since we don't need it anymore.
933-
* Note this doesn't break the foreach() loop, since lremove
934-
* doesn't touch the next-link of the removed cons cell.
935938
*/
936939
oldquals = lremove(oldrinfo, oldquals);
937940
}

‎src/backend/parser/analyze.c

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.257 2002/12/13 19:45:56 tgl Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.258 2002/12/17 01:18:29 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -521,41 +521,48 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt,
521521
* Prepare columns for assignment to target table.
522522
*/
523523
attnos = attrnos;
524-
foreach(tl, qry->targetList)
524+
/* cannot use foreach here because of possible lremove */
525+
tl = qry->targetList;
526+
while (tl)
525527
{
526528
TargetEntry *tle = (TargetEntry *) lfirst(tl);
527529
ResTarget *col;
528530

531+
/* must advance tl before lremove possibly pfree's it */
532+
tl = lnext(tl);
533+
529534
if (icolumns == NIL || attnos == NIL)
530535
elog(ERROR, "INSERT has more expressions than target columns");
536+
531537
col = (ResTarget *) lfirst(icolumns);
538+
Assert(IsA(col, ResTarget));
532539

533540
/*
534541
* When the value is to be set to the column default we can simply
535-
* drop it now and handle it later on using methods for missing
542+
* drop the TLE now and handle it later on using methods for missing
536543
* columns.
537544
*/
538-
if (!IsA(tle, InsertDefault))
545+
if (IsA(tle, InsertDefault))
539546
{
540-
Assert(IsA(col, ResTarget));
541-
Assert(!tle->resdom->resjunk);
542-
updateTargetListEntry(pstate, tle, col->name, lfirsti(attnos),
543-
col->indirection);
547+
qry->targetList = lremove(tle, qry->targetList);
548+
/* Note: the stmt->cols list is not adjusted to match */
544549
}
545550
else
546551
{
547-
icolumns = lremove(icolumns, icolumns);
548-
attnos = lremove(attnos, attnos);
549-
qry->targetList = lremove(tle, qry->targetList);
552+
/* Normal case */
553+
Assert(!tle->resdom->resjunk);
554+
updateTargetListEntry(pstate, tle, col->name, lfirsti(attnos),
555+
col->indirection);
550556
}
551557

552558
icolumns = lnext(icolumns);
553559
attnos = lnext(attnos);
554560
}
555561

556562
/*
557-
* Ensure that the targetlist has the same number of entries that
558-
* were present in the columns list. Don't do the check for select
563+
* Ensure that the targetlist has the same number of entries that
564+
* were present in the columns list. Don't do the check unless
565+
* an explicit columns list was given, though.
559566
* statements.
560567
*/
561568
if (stmt->cols != NIL && (icolumns != NIL || attnos != NIL))

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.114 2002/12/12 15:49:40 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.115 2002/12/17 01:18:32 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -205,9 +205,11 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
205205
{
206206
RangeTblRef *rtr = lfirst(jjt);
207207

208-
if (IsA(rtr, RangeTblRef) &&rtr->rtindex == rt_index)
208+
if (IsA(rtr, RangeTblRef) &&
209+
rtr->rtindex == rt_index)
209210
{
210211
newjointree = lremove(rtr, newjointree);
212+
/* foreach is safe because we exit loop after lremove... */
211213
break;
212214
}
213215
}

‎src/backend/utils/adt/selfuncs.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*
1616
*
1717
* IDENTIFICATION
18-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.123 2002/12/12 15:49:40 tgl Exp $
18+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.124 2002/12/17 01:18:35 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -1953,10 +1953,15 @@ estimate_num_groups(Query *root, List *groupClauses, double input_rows)
19531953
if (HeapTupleIsValid(statsTuple))
19541954
ReleaseSysCache(statsTuple);
19551955

1956-
foreach(l2, varinfos)
1956+
/* cannot use foreach here because of possible lremove */
1957+
l2 = varinfos;
1958+
while (l2)
19571959
{
19581960
MyVarInfo *varinfo = (MyVarInfo *) lfirst(l2);
19591961

1962+
/* must advance l2 before lremove possibly pfree's it */
1963+
l2 = lnext(l2);
1964+
19601965
if (var->varno != varinfo->var->varno &&
19611966
vars_known_equal(root, var, varinfo->var))
19621967
{
@@ -1969,10 +1974,7 @@ estimate_num_groups(Query *root, List *groupClauses, double input_rows)
19691974
}
19701975
else
19711976
{
1972-
/*
1973-
* Delete the older item. We assume lremove() will not
1974-
* break the lnext link of the item...
1975-
*/
1977+
/* Delete the older item */
19761978
varinfos = lremove(varinfo, varinfos);
19771979
}
19781980
}

0 commit comments

Comments
 (0)