Re: [bug fix] Memory leak in dblink

Joe Conway <mail@joeconway.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