mirror of
https://git.postgresql.org/git/postgresql.git
synced 2025-01-06 15:24:56 +08:00
Fix two ancient memory-leak bugs in relcache.c.
RelationCacheInsert() ignored the possibility that hash_search(HASH_ENTER) might find a hashtable entry already present for the same OID. However, that can in fact occur during recursive relcache load scenarios. When it did happen, we overwrote the pointer to the pre-existing Relation, causing a session-lifespan leakage of that entire structure. As far as is known, the pre-existing Relation would always have reference count zero by the time we arrive back at the outer insertion, so add code that deletes the pre-existing Relation if so. If by some chance its refcount is positive, elog a WARNING and allow the pre-existing Relation to be leaked as before. Also, AttrDefaultFetch() was sloppy about leaking the cstring form of the pg_attrdef.adbin value it's copying into the relcache structure. This is only a query-lifespan leakage, and normally not very significant, but it adds up during CLOBBER_CACHE testing. These bugs are of very ancient vintage, but I'll refrain from back-patching since there's no evidence that these leaks amount to anything in ordinary usage.
This commit is contained in:
parent
44cd47c1d4
commit
078b2ed291
69
src/backend/utils/cache/relcache.c
vendored
69
src/backend/utils/cache/relcache.c
vendored
@ -171,24 +171,36 @@ static int NextEOXactTupleDescNum = 0;
|
|||||||
static int EOXactTupleDescArrayLen = 0;
|
static int EOXactTupleDescArrayLen = 0;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* macros to manipulate the lookup hashtables
|
* macros to manipulate the lookup hashtable
|
||||||
*/
|
*/
|
||||||
#define RelationCacheInsert(RELATION) \
|
#define RelationCacheInsert(RELATION, replace_allowed) \
|
||||||
do { \
|
do { \
|
||||||
RelIdCacheEnt *idhentry; bool found; \
|
RelIdCacheEnt *hentry; bool found; \
|
||||||
idhentry = (RelIdCacheEnt*)hash_search(RelationIdCache, \
|
hentry = (RelIdCacheEnt *) hash_search(RelationIdCache, \
|
||||||
(void *) &(RELATION->rd_id), \
|
(void *) &((RELATION)->rd_id), \
|
||||||
HASH_ENTER, &found); \
|
HASH_ENTER, &found); \
|
||||||
/* used to give notice if found -- now just keep quiet */ \
|
if (found) \
|
||||||
idhentry->reldesc = RELATION; \
|
{ \
|
||||||
|
/* this can happen, see comments in RelationBuildDesc */ \
|
||||||
|
Relation _old_rel = hentry->reldesc; \
|
||||||
|
Assert(replace_allowed); \
|
||||||
|
hentry->reldesc = (RELATION); \
|
||||||
|
if (RelationHasReferenceCountZero(_old_rel)) \
|
||||||
|
RelationDestroyRelation(_old_rel, false); \
|
||||||
|
else \
|
||||||
|
elog(WARNING, "leaking still-referenced relcache entry for \"%s\"", \
|
||||||
|
RelationGetRelationName(_old_rel)); \
|
||||||
|
} \
|
||||||
|
else \
|
||||||
|
hentry->reldesc = (RELATION); \
|
||||||
} while(0)
|
} while(0)
|
||||||
|
|
||||||
#define RelationIdCacheLookup(ID, RELATION) \
|
#define RelationIdCacheLookup(ID, RELATION) \
|
||||||
do { \
|
do { \
|
||||||
RelIdCacheEnt *hentry; \
|
RelIdCacheEnt *hentry; \
|
||||||
hentry = (RelIdCacheEnt*)hash_search(RelationIdCache, \
|
hentry = (RelIdCacheEnt *) hash_search(RelationIdCache, \
|
||||||
(void *) &(ID), \
|
(void *) &(ID), \
|
||||||
HASH_FIND, NULL); \
|
HASH_FIND, NULL); \
|
||||||
if (hentry) \
|
if (hentry) \
|
||||||
RELATION = hentry->reldesc; \
|
RELATION = hentry->reldesc; \
|
||||||
else \
|
else \
|
||||||
@ -197,12 +209,13 @@ do { \
|
|||||||
|
|
||||||
#define RelationCacheDelete(RELATION) \
|
#define RelationCacheDelete(RELATION) \
|
||||||
do { \
|
do { \
|
||||||
RelIdCacheEnt *idhentry; \
|
RelIdCacheEnt *hentry; \
|
||||||
idhentry = (RelIdCacheEnt*)hash_search(RelationIdCache, \
|
hentry = (RelIdCacheEnt *) hash_search(RelationIdCache, \
|
||||||
(void *) &(RELATION->rd_id), \
|
(void *) &((RELATION)->rd_id), \
|
||||||
HASH_REMOVE, NULL); \
|
HASH_REMOVE, NULL); \
|
||||||
if (idhentry == NULL) \
|
if (hentry == NULL) \
|
||||||
elog(WARNING, "trying to delete a rd_id reldesc that does not exist"); \
|
elog(WARNING, "failed to delete relcache entry for OID %u", \
|
||||||
|
(RELATION)->rd_id); \
|
||||||
} while(0)
|
} while(0)
|
||||||
|
|
||||||
|
|
||||||
@ -982,9 +995,18 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
|
|||||||
|
|
||||||
/*
|
/*
|
||||||
* Insert newly created relation into relcache hash table, if requested.
|
* Insert newly created relation into relcache hash table, if requested.
|
||||||
|
*
|
||||||
|
* There is one scenario in which we might find a hashtable entry already
|
||||||
|
* present, even though our caller failed to find it: if the relation is a
|
||||||
|
* system catalog or index that's used during relcache load, we might have
|
||||||
|
* recursively created the same relcache entry during the preceding steps.
|
||||||
|
* So allow RelationCacheInsert to delete any already-present relcache
|
||||||
|
* entry for the same OID. The already-present entry should have refcount
|
||||||
|
* zero (else somebody forgot to close it); in the event that it doesn't,
|
||||||
|
* we'll elog a WARNING and leak the already-present entry.
|
||||||
*/
|
*/
|
||||||
if (insertIt)
|
if (insertIt)
|
||||||
RelationCacheInsert(relation);
|
RelationCacheInsert(relation, true);
|
||||||
|
|
||||||
/* It's fully valid */
|
/* It's fully valid */
|
||||||
relation->rd_isvalid = true;
|
relation->rd_isvalid = true;
|
||||||
@ -1599,7 +1621,7 @@ formrdesc(const char *relationName, Oid relationReltype,
|
|||||||
/*
|
/*
|
||||||
* add new reldesc to relcache
|
* add new reldesc to relcache
|
||||||
*/
|
*/
|
||||||
RelationCacheInsert(relation);
|
RelationCacheInsert(relation, false);
|
||||||
|
|
||||||
/* It's fully valid */
|
/* It's fully valid */
|
||||||
relation->rd_isvalid = true;
|
relation->rd_isvalid = true;
|
||||||
@ -2841,7 +2863,7 @@ RelationBuildLocalRelation(const char *relname,
|
|||||||
/*
|
/*
|
||||||
* Okay to insert into the relcache hash tables.
|
* Okay to insert into the relcache hash tables.
|
||||||
*/
|
*/
|
||||||
RelationCacheInsert(rel);
|
RelationCacheInsert(rel, false);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Flag relation as needing eoxact cleanup (to clear rd_createSubid). We
|
* Flag relation as needing eoxact cleanup (to clear rd_createSubid). We
|
||||||
@ -3489,8 +3511,13 @@ AttrDefaultFetch(Relation relation)
|
|||||||
NameStr(relation->rd_att->attrs[adform->adnum - 1]->attname),
|
NameStr(relation->rd_att->attrs[adform->adnum - 1]->attname),
|
||||||
RelationGetRelationName(relation));
|
RelationGetRelationName(relation));
|
||||||
else
|
else
|
||||||
attrdef[i].adbin = MemoryContextStrdup(CacheMemoryContext,
|
{
|
||||||
TextDatumGetCString(val));
|
/* detoast and convert to cstring in caller's context */
|
||||||
|
char *s = TextDatumGetCString(val);
|
||||||
|
|
||||||
|
attrdef[i].adbin = MemoryContextStrdup(CacheMemoryContext, s);
|
||||||
|
pfree(s);
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -4717,7 +4744,7 @@ load_relcache_init_file(bool shared)
|
|||||||
*/
|
*/
|
||||||
for (relno = 0; relno < num_rels; relno++)
|
for (relno = 0; relno < num_rels; relno++)
|
||||||
{
|
{
|
||||||
RelationCacheInsert(rels[relno]);
|
RelationCacheInsert(rels[relno], false);
|
||||||
/* also make a list of their OIDs, for RelationIdIsInInitFile */
|
/* also make a list of their OIDs, for RelationIdIsInInitFile */
|
||||||
if (!shared)
|
if (!shared)
|
||||||
initFileRelationIds = lcons_oid(RelationGetRelid(rels[relno]),
|
initFileRelationIds = lcons_oid(RelationGetRelid(rels[relno]),
|
||||||
|
Loading…
Reference in New Issue
Block a user