Re: [bug fix] Memory leak in dblink - Mailing list pgsql-hackers
From | Joe Conway |
---|---|
Subject | Re: [bug fix] Memory leak in dblink |
Date | |
Msg-id | 53A1E510.70202@joeconway.com Whole thread Raw |
In response to | Re: [bug fix] Memory leak in dblink (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: [bug fix] Memory leak in dblink
(Tom Lane <tgl@sss.pgh.pa.us>)
Re: [bug fix] Memory leak in dblink (Joe Conway <mail@joeconway.com>) |
List | pgsql-hackers |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/18/2014 12:09 PM, Tom Lane wrote: > 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; 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-----
pgsql-hackers by date: