Re: [bug fix] Memory leak in dblink

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/18/2014 12:09 PM, Tom Lane wrote:
> 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;

good point

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

Yeah, I thought about that too. My testing showed a small amount of
remaining leakage -- like 20 MB on 100,000 iterations -- I wasn't sure
that it was worthwhile to worry about. The memory context leak was
much larger.

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJToeUQAAoJEDfy90M199hls8kP/0hhsVdn8HKcY7OZ64cKju4e
dH0SwQNMZxklpQkjEqb6vpbTmqaQ/7At/i7b6RGTMWFbIBu7L5Vpz+NZnJ7SyfaC
UjSgKfSe6M2auDIXlLCiZ62d8KZJJmVQj6U2h8ljhqorJq22kvClgXAUlcxZMoVv
SEthE1y7Udxgf3IqBO0AN6SMPrxP8bZDgrPbtZqw9UEHkCGZK0MDdH8TSRip7p6V
heyg9GbWeTvwFWBUYomMnMEUB6UlgBRnmHYsZIAjmUgxZfGiKhlydGOrb7MDHopz
ejyb06fg2MsSQnrEnCElLbonUqb5S9bD4p9BZHF/bz67AhVieJvDvnsSIoDjzR1a
+JIYe36ZDqo2kIGx4PuESgGX4ZTxxsrw033AQhVBW+KGkIuxjnvhh7tvEj3ACSjd
0bC1ztrnzCJLyNFd6jKaq5KAcNCEb/zvfKQAdHygJ87JkwE8I6u+ms1hdDpc10uZ
w484lcv/qP/JO2SaqPerDMwRsDXaovT8kEcsbqr1D7XFXu4cufHVGwe64IJYMjt+
9Y09/+i+Xt9DKXfkxC68Q2QWxwst5NfShRWDF3NG02iWlMqU2OsoVTQB6137DCiB
7hBKoNBmOApLJIjwCzEjB5IJmeeAz9x7YxgsqicRPnXoCKO+aF3dTbS+1u04va84
5XQ0p0lwUZaNfYr/xbi6
=H5bR
-----END PGP SIGNATURE-----