Re: buildfarm: strange OOM failures on markhor (running CLOBBER_CACHE_RECURSIVELY)

On Mon, May 19, 2014 at 05:08:04PM +0200, Andres Freund wrote:
> On 2014-05-18 14:49:10 -0400, Tom Lane wrote:
> > While I'm at it: I could not help noticing RememberToFreeTupleDescAtEOX,
> > which was not there last time I looked at this code.  Isn't that broken
> > by design?  It's basically a deliberately induced transaction-lifespan
> > memory leak, and AFAICS if it does anything at all, it's supporting
> > incorrect calling code.  There should *never* be any situation where it's
> > not okay to free a tupledesc with zero refcount.
> 
> I am not sure I understand that code. The whole keep_tupdesc/keep_rules
> logic isn't new and a bit underdocumented as well :(. Seems to come from
> 491dd4a9.
> If I understand correctly the issue is that callers have accessed the
> tupledesc with RelationGetDescr() which doesn't increase the
> refcount.

Right.  See here for background:
http://www.postgresql.org/message-id/flat/20130801005351.GA325106@tornado.leadboat.com

> >          * If we Rebuilt a relcache entry during a transaction then its
> >          * possible we did that because the TupDesc changed as the result of
> >          * an ALTER TABLE that ran at less than AccessExclusiveLock. It's
> >          * possible someone copied that TupDesc, in which case the copy would
> >          * point to free'd memory. So if we rebuild an entry we keep the
> > 
> > If someone copied the tupdesc, how would freeing the original result
> > in the copy being damaged?
> 
> The sentence only seems to make sense with s/copy/accessed without
> increasing the refcoung, e.g. with RelationGetDescr,/.

Agreed.  The hazards arise when copying the rd_att pointer, not when
deep-copying the whole TupleDesc.

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com