postgres.git / summary / log / commit / refs

commit    ab8c84db2f7af008151b848cf1d6a4672a39eecd
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Wed Aug 13 15:27:28 2014 +0000

    Prevent memory leaks in RelationGetIndexList, RelationGetIndexAttrBitmap.
    
    When replacing rd_indexlist, rd_indexattr, etc, we neglected to pfree any
    old value of these fields.  Under ordinary circumstances, the old value
    would always be NULL, so this seemed reasonable enough.  However, in cases
    where we're rebuilding a system catalog's relcache entry and another cache
    flush occurs on that same catalog meanwhile, it's possible for the field to
    not be NULL when we return to the outer level, because we already refilled
    it while recovering from the inner flush.  This leads to a fairly small
    session-lifespan leak in CacheMemoryContext.  In real-world usage the leak
    would be too small to notice; but in testing with CLOBBER_CACHE_RECURSIVELY
    the leakage can add up to the point of causing OOM failures, as reported by
    Tomas Vondra.
    
    The issue has been there a long time, but it only seems worth fixing in
    HEAD, like the previous fix in this area (commit 078b2ed291c758e7).


 src/backend/utils/cache/relcache.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 10d300a3e88..e4697f68aeb 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -3646,6 +3646,7 @@ RelationGetIndexList(Relation relation)
 	ScanKeyData skey;
 	HeapTuple	htup;
 	List	   *result;
+	List	   *oldlist;
 	char		replident = relation->rd_rel->relreplident;
 	Oid			oidIndex = InvalidOid;
 	Oid			pkeyIndex = InvalidOid;
@@ -3737,6 +3738,7 @@ RelationGetIndexList(Relation relation)
 
 	/* Now save a copy of the completed list in the relcache entry. */
 	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+	oldlist = relation->rd_indexlist;
 	relation->rd_indexlist = list_copy(result);
 	relation->rd_oidindex = oidIndex;
 	if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex))
@@ -3748,6 +3750,9 @@ RelationGetIndexList(Relation relation)
 	relation->rd_indexvalid = 1;
 	MemoryContextSwitchTo(oldcxt);
 
+	/* Don't leak the old list, if there is one */
+	list_free(oldlist);
+
 	return result;
 }
 
@@ -4141,6 +4146,14 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 
 	list_free(indexoidlist);
 
+	/* Don't leak the old values of these bitmaps, if any */
+	bms_free(relation->rd_indexattr);
+	relation->rd_indexattr = NULL;
+	bms_free(relation->rd_keyattr);
+	relation->rd_keyattr = NULL;
+	bms_free(relation->rd_idattr);
+	relation->rd_idattr = NULL;
+
 	/*
 	 * Now save copies of the bitmaps in the relcache entry.  We intentionally
 	 * set rd_indexattr last, because that's the one that signals validity of


[parent: 52bffe3499d8]