-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/10/2014 02:57 AM, MauMau wrote: > From: "Amit Kapila" <amit.kapil...@gmail.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-----
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index a81853f..46c4a51 100644 *** a/contrib/dblink/dblink.c --- b/contrib/dblink/dblink.c *************** storeQueryResult(storeInfo *sinfo, PGcon *** 1076,1081 **** --- 1076,1089 ---- if (!PQsetSingleRowMode(conn)) /* shouldn't fail */ elog(ERROR, "failed to set single-row mode for dblink query"); + /* Create short-lived memory context for data conversions */ + if (!sinfo->tmpcontext) + sinfo->tmpcontext = AllocSetContextCreate(CurrentMemoryContext, + "dblink temporary context", + ALLOCSET_DEFAULT_MINSIZE, + ALLOCSET_DEFAULT_INITSIZE, + ALLOCSET_DEFAULT_MAXSIZE); + for (;;) { CHECK_FOR_INTERRUPTS(); *************** storeQueryResult(storeInfo *sinfo, PGcon *** 1119,1124 **** --- 1127,1136 ---- /* clean up GUC settings, if we changed any */ restoreLocalGucs(nestlevel); + /* clean up data conversion short-lived memory context */ + if (sinfo->tmpcontext != NULL) + MemoryContextDelete(sinfo->tmpcontext); + /* return last_res */ res = sinfo->last_res; sinfo->last_res = NULL; *************** storeRow(storeInfo *sinfo, PGresult *res *** 1204,1218 **** if (sinfo->cstrs) pfree(sinfo->cstrs); sinfo->cstrs = (char **) palloc(nfields * sizeof(char *)); - - /* Create short-lived memory context for data conversions */ - if (!sinfo->tmpcontext) - sinfo->tmpcontext = - AllocSetContextCreate(CurrentMemoryContext, - "dblink temporary context", - ALLOCSET_DEFAULT_MINSIZE, - ALLOCSET_DEFAULT_INITSIZE, - ALLOCSET_DEFAULT_MAXSIZE); } /* Should have a single-row result if we get here */ --- 1216,1221 ----
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers