-----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

Reply via email to