Re: [HACKERS] Memory leaks in relcache - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] Memory leaks in relcache
Date
Msg-id 25912.931357471@sss.pgh.pa.us
Whole thread Raw
In response to Re: [HACKERS] Memory leaks in relcache  (Bruce Momjian <maillist@candle.pha.pa.us>)
Responses Re: [HACKERS] Memory leaks in relcache
List pgsql-hackers
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> Tom, where are we on this.  As I remember, it is still an open issue,
> right?  I can add it to the TODO list.

I have not done anything about it yet; it ought to be in TODO.

I'm also aware of two or three other sources of small but permanent
memory leaks, btw; have them in my todo list.
        regards, tom lane

>> I have been looking into why a reference to a nonexistent table, eg
>> INSERT INTO nosuchtable VALUES(1);
>> leaks a small amount of memory per occurrence.  What I find is a
>> memory leak in the indexscan support.  Specifically,
>> RelationGetIndexScan in backend/access/index/genam.c palloc's both
>> an IndexScanDesc and some keydata storage.  The IndexScanDesc
>> block is eventually pfree'd, at the bottom of CatalogIndexFetchTuple
>> in backend/catalog/indexing.c.  But the keydata block is not.
>> 
>> This wouldn't matter so much if the palloc were coming from a
>> transaction-local context.  But what we're doing is a lookup in pg_class
>> on behalf of RelationBuildDesc in backend/utils/cache/relcache.c, and
>> it's done a MemoryContextSwitchTo into the global CacheCxt before
>> starting the lookup.  Therefore, the un-pfreed block represents a
>> permanent memory leak.
>> 
>> In fact, *every* reference to a relation that is not already present in
>> the relcache causes a similar leak.  The error case is just the one that
>> is easiest to repeat.  The missing pfree of the keydata block is
>> probably causing a bunch of other short-term and long-term leaks too.
>> 
>> It seems to me there are two things to fix here: indexscan ought to
>> pfree everything it pallocs, and RelationBuildDesc ought to be warier
>> about how much work gets done with CacheCxt as the active palloc
>> context.  (Even if indexscan didn't leak anything ordinarily, there's
>> still the risk of elog(ERROR) causing an abort before the indexscan code
>> gets to clean up.)
>> 
>> Comments?  In particular, where is the cleanest place to add the pfree
>> of the keydata block?  I don't especially like the fact that callers
>> of index_endscan have to clean up the toplevel scan block; I think that
>> ought to happen inside index_endscan.
>> 
>> regards, tom lane
>> 
>> 


> -- 
>   Bruce Momjian                        |  http://www.op.net/~candle
>   maillist@candle.pha.pa.us            |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] Some progress on INSERT/SELECT/GROUP BY bugs
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] Installation permissions