From e932a724a4a372c7db21ce7bf40250576b085041 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 17 Dec 2002 01:18:35 +0000 Subject: [PATCH] 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. --- src/backend/nodes/list.c | 6 +++-- src/backend/optimizer/path/pathkeys.c | 13 ++++++---- src/backend/optimizer/plan/initsplan.c | 11 +++++---- src/backend/parser/analyze.c | 33 ++++++++++++++++---------- src/backend/rewrite/rewriteHandler.c | 6 +++-- src/backend/utils/adt/selfuncs.c | 14 ++++++----- 6 files changed, 51 insertions(+), 32 deletions(-) diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c index 6dc6001a0f..b3c6a18496 100644 --- a/src/backend/nodes/list.c +++ b/src/backend/nodes/list.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/nodes/list.c,v 1.42 2002/11/24 21:52:13 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/nodes/list.c,v 1.43 2002/12/17 01:18:18 tgl Exp $ * * NOTES * XXX a few of the following functions are duplicated to handle @@ -269,7 +269,6 @@ llasti(List *l) * Free the List nodes of a list * The pointed-to nodes, if any, are NOT freed. * This works for integer lists too. - * */ void freeList(List *list) @@ -487,6 +486,7 @@ lremove(void *elem, List *list) result = lnext(l); else lnext(prev) = lnext(l); + pfree(l); } return result; } @@ -518,6 +518,7 @@ LispRemove(void *elem, List *list) result = lnext(l); else lnext(prev) = lnext(l); + pfree(l); } return result; } @@ -545,6 +546,7 @@ lremovei(int elem, List *list) result = lnext(l); else lnext(prev) = lnext(l); + pfree(l); } return result; } diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index af0b61a403..9c6ed4d1bd 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -11,7 +11,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.42 2002/12/12 15:49:32 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.43 2002/12/17 01:18:22 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -101,12 +101,17 @@ add_equijoined_keys(Query *root, RestrictInfo *restrictinfo) */ newset = NIL; - foreach(cursetlink, root->equi_key_list) + /* cannot use foreach here because of possible lremove */ + cursetlink = root->equi_key_list; + while (cursetlink) { List *curset = lfirst(cursetlink); bool item1here = member(item1, curset); bool item2here = member(item2, curset); + /* must advance cursetlink before lremove possibly pfree's it */ + cursetlink = lnext(cursetlink); + if (item1here || item2here) { /* @@ -128,9 +133,7 @@ add_equijoined_keys(Query *root, RestrictInfo *restrictinfo) newset = set_union(newset, curset); /* - * Remove old set from equi_key_list. NOTE this does not - * change lnext(cursetlink), so the foreach loop doesn't - * break. + * Remove old set from equi_key_list. */ root->equi_key_list = lremove(curset, root->equi_key_list); freeList(curset); /* might as well recycle old cons cells */ diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index aca2c6f4f6..151a37a888 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.78 2002/12/12 15:49:32 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/initsplan.c,v 1.79 2002/12/17 01:18:25 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -910,13 +910,18 @@ qual_is_redundant(Query *root, do { someadded = false; - foreach(olditem, oldquals) + /* cannot use foreach here because of possible lremove */ + olditem = oldquals; + while (olditem) { RestrictInfo *oldrinfo = (RestrictInfo *) lfirst(olditem); Node *oldleft = (Node *) get_leftop(oldrinfo->clause); Node *oldright = (Node *) get_rightop(oldrinfo->clause); Node *newguy = NULL; + /* must advance olditem before lremove possibly pfree's it */ + olditem = lnext(olditem); + if (member(oldleft, equalvars)) newguy = oldright; else if (member(oldright, equalvars)) @@ -930,8 +935,6 @@ qual_is_redundant(Query *root, /* * Remove this qual from list, since we don't need it anymore. - * Note this doesn't break the foreach() loop, since lremove - * doesn't touch the next-link of the removed cons cell. */ oldquals = lremove(oldrinfo, oldquals); } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 83a322cf14..e771d66659 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.257 2002/12/13 19:45:56 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.258 2002/12/17 01:18:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -521,32 +521,38 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt, * Prepare columns for assignment to target table. */ attnos = attrnos; - foreach(tl, qry->targetList) + /* cannot use foreach here because of possible lremove */ + tl = qry->targetList; + while (tl) { TargetEntry *tle = (TargetEntry *) lfirst(tl); ResTarget *col; + /* must advance tl before lremove possibly pfree's it */ + tl = lnext(tl); + if (icolumns == NIL || attnos == NIL) elog(ERROR, "INSERT has more expressions than target columns"); + col = (ResTarget *) lfirst(icolumns); + Assert(IsA(col, ResTarget)); /* * When the value is to be set to the column default we can simply - * drop it now and handle it later on using methods for missing + * drop the TLE now and handle it later on using methods for missing * columns. */ - if (!IsA(tle, InsertDefault)) + if (IsA(tle, InsertDefault)) { - Assert(IsA(col, ResTarget)); - Assert(!tle->resdom->resjunk); - updateTargetListEntry(pstate, tle, col->name, lfirsti(attnos), - col->indirection); + qry->targetList = lremove(tle, qry->targetList); + /* Note: the stmt->cols list is not adjusted to match */ } else { - icolumns = lremove(icolumns, icolumns); - attnos = lremove(attnos, attnos); - qry->targetList = lremove(tle, qry->targetList); + /* Normal case */ + Assert(!tle->resdom->resjunk); + updateTargetListEntry(pstate, tle, col->name, lfirsti(attnos), + col->indirection); } icolumns = lnext(icolumns); @@ -554,8 +560,9 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt, } /* - * Ensure that the targetlist has the same number of entries that - * were present in the columns list. Don't do the check for select + * Ensure that the targetlist has the same number of entries that + * were present in the columns list. Don't do the check unless + * an explicit columns list was given, though. * statements. */ if (stmt->cols != NIL && (icolumns != NIL || attnos != NIL)) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index d0badcc154..c19d15b4f0 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.114 2002/12/12 15:49:40 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.115 2002/12/17 01:18:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -205,9 +205,11 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index) { RangeTblRef *rtr = lfirst(jjt); - if (IsA(rtr, RangeTblRef) &&rtr->rtindex == rt_index) + if (IsA(rtr, RangeTblRef) && + rtr->rtindex == rt_index) { newjointree = lremove(rtr, newjointree); + /* foreach is safe because we exit loop after lremove... */ break; } } diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index c378f8b50e..58c63c210c 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -15,7 +15,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.123 2002/12/12 15:49:40 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/adt/selfuncs.c,v 1.124 2002/12/17 01:18:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1953,10 +1953,15 @@ estimate_num_groups(Query *root, List *groupClauses, double input_rows) if (HeapTupleIsValid(statsTuple)) ReleaseSysCache(statsTuple); - foreach(l2, varinfos) + /* cannot use foreach here because of possible lremove */ + l2 = varinfos; + while (l2) { MyVarInfo *varinfo = (MyVarInfo *) lfirst(l2); + /* must advance l2 before lremove possibly pfree's it */ + l2 = lnext(l2); + if (var->varno != varinfo->var->varno && vars_known_equal(root, var, varinfo->var)) { @@ -1969,10 +1974,7 @@ estimate_num_groups(Query *root, List *groupClauses, double input_rows) } else { - /* - * Delete the older item. We assume lremove() will not - * break the lnext link of the item... - */ + /* Delete the older item */ varinfos = lremove(varinfo, varinfos); } } -- GitLab