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(dot)GA325106(at)tornado(dot)leadboat(dot)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