Обсуждение: Fix for memory leak in dblink
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
Вложения
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
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