Lists: | pgsql-hackers |
---|
From: | Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Fix for memory leak in dblink |
Date: | 2010-01-12 03:43:00 |
Message-ID: | 20100112124300.C520.52131E4D@oss.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
There is a memory leak in dblink when we cancel a query during
returning tuples. It could leak a PGresult because memory used
by it is not palloc'ed one. I wrote a patch[1] before, but I've
badly used global variables to track the resource.
The attached is a cleaned up patch rewritten to use a tuplestore
(SFRM_Materialize mode) to return tuples suggested at [2]. Since
we don't return from the dblink function in tuplestore mode, we
can surely release the PGresult with a PG_CATCH block even on error.
Also, dblink_record_internal() and dblink_fetch() are rearranged
to share the same code to return tuples for code refactoring.
[1] http://archives.postgresql.org/pgsql-hackers/2009-06/msg01358.php
[2] http://archives.postgresql.org/pgsql-hackers/2009-10/msg00292.php
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
Attachment | Content-Type | Size |
---|---|---|
dblink-memleak_20100112.patch | application/octet-stream | 19.8 KB |
From: | Joe Conway <mail(at)joeconway(dot)com> |
---|---|
To: | Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fix for memory leak in dblink |
Date: | 2010-01-12 17:40:00 |
Message-ID: | 4B4CB3F0.1000605@joeconway.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 01/11/2010 07:43 PM, Takahiro Itagaki wrote:
> There is a memory leak in dblink when we cancel a query during
> returning tuples. It could leak a PGresult because memory used
> by it is not palloc'ed one. I wrote a patch[1] before, but I've
> badly used global variables to track the resource.
>
> The attached is a cleaned up patch rewritten to use a tuplestore
> (SFRM_Materialize mode) to return tuples suggested at [2]. Since
> we don't return from the dblink function in tuplestore mode, we
> can surely release the PGresult with a PG_CATCH block even on error.
Thanks -- I'll review this weekend.
Joe
From: | Joe Conway <mail(at)joeconway(dot)com> |
---|---|
To: | Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fix for memory leak in dblink |
Date: | 2010-01-22 00:57:52 |
Message-ID: | 4B58F810.7000700@joeconway.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 01/11/2010 07:43 PM, Takahiro Itagaki wrote:
> There is a memory leak in dblink when we cancel a query during
> returning tuples. It could leak a PGresult because memory used
> by it is not palloc'ed one. I wrote a patch[1] before, but I've
> badly used global variables to track the resource.
>
> The attached is a cleaned up patch rewritten to use a tuplestore
> (SFRM_Materialize mode) to return tuples suggested at [2]. Since
> we don't return from the dblink function in tuplestore mode, we
> can surely release the PGresult with a PG_CATCH block even on error.
>
> Also, dblink_record_internal() and dblink_fetch() are rearranged
> to share the same code to return tuples for code refactoring.
>
> [1] http://archives.postgresql.org/pgsql-hackers/2009-06/msg01358.php
> [2] http://archives.postgresql.org/pgsql-hackers/2009-10/msg00292.php
This looks good to me. I'll commit in the next 24 hours if there are no
objections.
Thanks,
Joe