Re: [bug fix] Memory leak in dblink

Joe Conway <mail(at)joeconway(dot)com> writes:
> I think the context deletion was missed in the first place because
> storeRow() is the wrong place to create the context. Rather than
> creating the context in storeRow() and deleting it two levels up in
> materializeQueryResult(), I think it should be created and deleted in
> the interim layer, storeQueryResult(). Patch along those lines attached.

Since the storeInfo struct is longer-lived than storeQueryResult(),
it'd probably be better if the cleanup looked like

+ if (sinfo->tmpcontext != NULL)
+ MemoryContextDelete(sinfo->tmpcontext);
+ sinfo->tmpcontext = NULL;

I find myself a bit suspicious of this whole thing though. If it's
necessary to explicitly clean up the tmpcontext, why not also the
sinfo->cstrs allocation? And what about the tupdesc, attinmeta,
etc created further up in that "if (first)" block? I'd have expected
that all this stuff gets allocated in a context that's short-lived
enough that we don't really need to clean it up explicitly.

regards, tom lane