Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit078b2ed

Browse files
committed
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 itdid happen, we overwrote the pointer to the pre-existing Relation, causinga session-lifespan leakage of that entire structure. As far as is known,the pre-existing Relation would always have reference count zero by thetime we arrive back at the outer insertion, so add code that deletes thepre-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 thepg_attrdef.adbin value it's copying into the relcache structure. This isonly a query-lifespan leakage, and normally not very significant, but itadds up during CLOBBER_CACHE testing.These bugs are of very ancient vintage, but I'll refrain from back-patchingsince there's no evidence that these leaks amount to anything in ordinaryusage.
1 parent44cd47c commit078b2ed

File tree

1 file changed

+48
-21
lines changed

1 file changed

+48
-21
lines changed

‎src/backend/utils/cache/relcache.c‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -171,24 +171,36 @@ static intNextEOXactTupleDescNum = 0;
171171
staticintEOXactTupleDescArrayLen=0;
172172

173173
/*
174-
*macros to manipulate the lookuphashtables
174+
*macros to manipulate the lookuphashtable
175175
*/
176-
#defineRelationCacheInsert(RELATION)\
176+
#defineRelationCacheInsert(RELATION,replace_allowed)\
177177
do { \
178-
RelIdCacheEnt *idhentry; bool found; \
179-
idhentry = (RelIdCacheEnt*)hash_search(RelationIdCache, \
180-
(void *) &(RELATION->rd_id), \
178+
RelIdCacheEnt *hentry; bool found; \
179+
hentry = (RelIdCacheEnt *)hash_search(RelationIdCache, \
180+
(void *) &((RELATION)->rd_id), \
181181
HASH_ENTER, &found); \
182-
/* used to give notice if found -- now just keep quiet */ \
183-
idhentry->reldesc=RELATION; \
182+
if (found) \
183+
{ \
184+
/* this can happen, see comments in RelationBuildDesc */ \
185+
Relation_old_rel=hentry->reldesc; \
186+
Assert(replace_allowed); \
187+
hentry->reldesc= (RELATION); \
188+
if (RelationHasReferenceCountZero(_old_rel)) \
189+
RelationDestroyRelation(_old_rel, false); \
190+
else \
191+
elog(WARNING,"leaking still-referenced relcache entry for \"%s\"", \
192+
RelationGetRelationName(_old_rel)); \
193+
} \
194+
else \
195+
hentry->reldesc= (RELATION); \
184196
}while(0)
185197

186198
#defineRelationIdCacheLookup(ID,RELATION) \
187199
do { \
188200
RelIdCacheEnt *hentry; \
189-
hentry = (RelIdCacheEnt*)hash_search(RelationIdCache, \
190-
(void *) &(ID), \
191-
HASH_FIND, NULL); \
201+
hentry = (RelIdCacheEnt *)hash_search(RelationIdCache, \
202+
(void *) &(ID), \
203+
HASH_FIND, NULL); \
192204
if (hentry) \
193205
RELATION = hentry->reldesc; \
194206
else \
@@ -197,12 +209,13 @@ do { \
197209

198210
#defineRelationCacheDelete(RELATION) \
199211
do { \
200-
RelIdCacheEnt *idhentry; \
201-
idhentry = (RelIdCacheEnt*)hash_search(RelationIdCache, \
202-
(void *) &(RELATION->rd_id), \
212+
RelIdCacheEnt *hentry; \
213+
hentry = (RelIdCacheEnt *)hash_search(RelationIdCache, \
214+
(void *) &((RELATION)->rd_id), \
203215
HASH_REMOVE, NULL); \
204-
if (idhentry == NULL) \
205-
elog(WARNING, "trying to delete a rd_id reldesc that does not exist"); \
216+
if (hentry == NULL) \
217+
elog(WARNING, "failed to delete relcache entry for OID %u", \
218+
(RELATION)->rd_id); \
206219
} while(0)
207220

208221

@@ -982,9 +995,18 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
982995

983996
/*
984997
* Insert newly created relation into relcache hash table, if requested.
998+
*
999+
* There is one scenario in which we might find a hashtable entry already
1000+
* present, even though our caller failed to find it: if the relation is a
1001+
* system catalog or index that's used during relcache load, we might have
1002+
* recursively created the same relcache entry during the preceding steps.
1003+
* So allow RelationCacheInsert to delete any already-present relcache
1004+
* entry for the same OID. The already-present entry should have refcount
1005+
* zero (else somebody forgot to close it); in the event that it doesn't,
1006+
* we'll elog a WARNING and leak the already-present entry.
9851007
*/
9861008
if (insertIt)
987-
RelationCacheInsert(relation);
1009+
RelationCacheInsert(relation, true);
9881010

9891011
/* It's fully valid */
9901012
relation->rd_isvalid= true;
@@ -1599,7 +1621,7 @@ formrdesc(const char *relationName, Oid relationReltype,
15991621
/*
16001622
* add new reldesc to relcache
16011623
*/
1602-
RelationCacheInsert(relation);
1624+
RelationCacheInsert(relation, false);
16031625

16041626
/* It's fully valid */
16051627
relation->rd_isvalid= true;
@@ -2841,7 +2863,7 @@ RelationBuildLocalRelation(const char *relname,
28412863
/*
28422864
* Okay to insert into the relcache hash tables.
28432865
*/
2844-
RelationCacheInsert(rel);
2866+
RelationCacheInsert(rel, false);
28452867

28462868
/*
28472869
* Flag relation as needing eoxact cleanup (to clear rd_createSubid). We
@@ -3489,8 +3511,13 @@ AttrDefaultFetch(Relation relation)
34893511
NameStr(relation->rd_att->attrs[adform->adnum-1]->attname),
34903512
RelationGetRelationName(relation));
34913513
else
3492-
attrdef[i].adbin=MemoryContextStrdup(CacheMemoryContext,
3493-
TextDatumGetCString(val));
3514+
{
3515+
/* detoast and convert to cstring in caller's context */
3516+
char*s=TextDatumGetCString(val);
3517+
3518+
attrdef[i].adbin=MemoryContextStrdup(CacheMemoryContext,s);
3519+
pfree(s);
3520+
}
34943521
break;
34953522
}
34963523

@@ -4717,7 +4744,7 @@ load_relcache_init_file(bool shared)
47174744
*/
47184745
for (relno=0;relno<num_rels;relno++)
47194746
{
4720-
RelationCacheInsert(rels[relno]);
4747+
RelationCacheInsert(rels[relno], false);
47214748
/* also make a list of their OIDs, for RelationIdIsInInitFile */
47224749
if (!shared)
47234750
initFileRelationIds=lcons_oid(RelationGetRelid(rels[relno]),

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp