Re: [HACKERS] Memory leaks in relcache

Added to TODO:

* fix indexscan() so it does leak memory by not requiring caller to free
* improve dynamic memory allocation by introducing tuple-context memory allocation
* fix memory leak in cache code when non-existant table is referenced

> 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
> 


--  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,
Pennsylvania19026