Re: [bug fix] Memory leak in dblink

From: Joe Conway <mail(at)joeconway(dot)com>
To: MauMau <maumau307(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [bug fix] Memory leak in dblink
Date: 2014-06-18 18:51:00
Message-ID: 53A1DF94.70003@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

On 06/10/2014 02:57 AM, MauMau wrote:
> From: "Amit Kapila" <amit(dot)kapila16(at)gmail(dot)com>
>> Is there a need to free memory context in PG_CATCH()? In error
>> path the memory due to this temporary context will get freed as
>> it will delete the transaction context and this temporary context
>> will definitely be in the hierarchy of it, so it should also get
>> deleted. I think such handling can be useful incase we use
>> PG_CATCH() to suppress the error.
>
> I thought the same, but I also felt that I should make an effort
> to release resources as soon as possible, considering the memory
> context auto deletion as a last resort. However, looking at other
> places where PG_CATCH() is used, memory context is not deleted.
> So, I removed the modification from PG_CATCH() block. Thanks.

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.

Any objections to me back-patching it this way?

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/

iQIcBAEBAgAGBQJTod+UAAoJEDfy90M199hlaWgP/iitSQm7m+Sf2i8hP73TAdTX
Naw+TKtag822xzVT6AoVJafeZvEv0PNRYurP69y4zE11cwiG9u+RfoO1X1hP7FYu
mdHo5++YG+znmqDVC57Qav0qAheYaCk2Y9RKEZk9gEJLUO+HJRxuzc+L277lsAni
Iyu3DyaiogzmGBtjrPPex4VMtpFAPHzHWfABaL11kvNBXXpUjO/Z02ex+1nuZDV7
+wNSV4IdTTsmpdbbi/NRhDeU7sZOerIAY29B6vI/GXfHdwhDhg+3tG4PuoDg6YG2
jX/H37UE0zq0+J3ySnM92HZtn9RxfrjdkHTz/X7DAjZjHNwBNVi18oNfrsXUyqHO
3j3VEaRelow2+tUDaTxziEkZRA0vCRoeG20B7dgQY3op60wm9lpJ+pCQH8UGwPYC
HDxaOYcVKmxbWq+j86l2ZyYlfP8sVbqTMEc00dnYoc2sdDzU36+KJadIqoMoUgds
G5uNYOZ5eTxumua9exz2cqGVOdgzcDD3r/ZAUUVM0jk3LwdA4CKLorsy26Ce59lF
mSIO58uAJe198tyOCmbpZjbypIRRO3Qm73SfUmioCbcUzSg2tDdPpf0LP62V/YEm
gFIsPHNyZnaVm0baLilbJFg+88sxFSo1hvpoaUfNPVww7woAfFxi6uBt5h5KthUO
JoDWxYjYUy9VFDdC4rh4
=gj6p
-----END PGP SIGNATURE-----

Attachment Content-Type Size
dblink_memleak_v3.patch text/x-diff 1.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-06-18 18:55:10 Re: pg_control is missing a field for LOBLKSIZE
Previous Message Kevin Grittner 2014-06-18 18:50:01 Re: idle_in_transaction_timeout