From 7942df89eed700dfcf71294e9e0aed821b737281 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Fri, 13 Mar 2020 19:53:42 -0500
Subject: [PATCH v2 2/2] Clean up existing anti-pattern of freeing SRF
 resources

See also: e4186762ffaa4188e16702e8f4f299ea70988b96

Discussion: https://www.postgresql.org/message-id/8034.1583699444%40sss.pgh.pa.us
---
 contrib/pageinspect/btreefuncs.c       | 17 ++++++-----------
 contrib/pageinspect/ginfuncs.c         |  5 +++--
 contrib/pageinspect/hashfuncs.c        |  8 +++-----
 doc/src/sgml/xfunc.sgml                | 12 +++++++++---
 src/backend/access/transam/multixact.c |  5 +----
 src/backend/tsearch/wparser.c          | 13 ++++---------
 src/backend/utils/adt/jsonfuncs.c      | 16 ++--------------
 src/backend/utils/adt/tsvector_op.c    |  2 +-
 src/include/funcapi.h                  |  7 ++++---
 9 files changed, 33 insertions(+), 52 deletions(-)

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index e6a2fc1e15..0f244e4f30 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -502,12 +502,9 @@ bt_page_items(PG_FUNCTION_ARGS)
 		uargs->offset++;
 		SRF_RETURN_NEXT(fctx, result);
 	}
-	else
-	{
-		pfree(uargs->page);
-		pfree(uargs);
-		SRF_RETURN_DONE(fctx);
-	}
+
+	/* allocations in multi_call_memory_ctx are released automatically */
+	SRF_RETURN_DONE(fctx);
 }
 
 /*-------------------------------------------------------
@@ -590,11 +587,9 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
 		uargs->offset++;
 		SRF_RETURN_NEXT(fctx, result);
 	}
-	else
-	{
-		pfree(uargs);
-		SRF_RETURN_DONE(fctx);
-	}
+
+	/* allocations in multi_call_memory_ctx are released automatically */
+	SRF_RETURN_DONE(fctx);
 }
 
 /* Number of output arguments (columns) for bt_metap() */
diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c
index 7e2cafab74..5109c4b133 100644
--- a/contrib/pageinspect/ginfuncs.c
+++ b/contrib/pageinspect/ginfuncs.c
@@ -260,6 +260,7 @@ gin_leafpage_items(PG_FUNCTION_ARGS)
 
 		SRF_RETURN_NEXT(fctx, result);
 	}
-	else
-		SRF_RETURN_DONE(fctx);
+
+	/* allocations in multi_call_memory_ctx are released automatically */
+	SRF_RETURN_DONE(fctx);
 }
diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c
index 984ac33186..d024f1ac7b 100644
--- a/contrib/pageinspect/hashfuncs.c
+++ b/contrib/pageinspect/hashfuncs.c
@@ -374,11 +374,9 @@ hash_page_items(PG_FUNCTION_ARGS)
 
 		SRF_RETURN_NEXT(fctx, result);
 	}
-	else
-	{
-		pfree(uargs);
-		SRF_RETURN_DONE(fctx);
-	}
+
+	/* allocations in multi_call_memory_ctx are released automatically */
+	SRF_RETURN_DONE(fctx);
 }
 
 /* ------------------------------------------------
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 432758e75e..f3e96087fa 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -2861,7 +2861,8 @@ typedef struct FuncCallContext
      * OPTIONAL pointer to miscellaneous user-provided context information
      *
      * user_fctx is for use as a pointer to your own data to retain
-     * arbitrary context information between calls of your function.
+     * context information between calls of your function.
+     * It should probably not include resources other than allocated memory.
      */
     void *user_fctx;
 
@@ -2881,7 +2882,8 @@ typedef struct FuncCallContext
      * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and used
      * by SRF_RETURN_DONE() for cleanup. It is the most appropriate memory
      * context for any memory that is to be reused across multiple calls
-     * of the SRF.
+     * of the SRF.  Memory palloc'd in this context will be pfree'd
+     * automatically.
      */
     MemoryContext multi_call_memory_ctx;
 
@@ -2935,6 +2937,7 @@ SRF_RETURN_NEXT(funcctx, result)
 SRF_RETURN_DONE(funcctx)
 </programlisting>
      to clean up and end the <acronym>SRF</acronym>.
+     Allocations within multi_call_memory_ctx are automatically pfree'd.
     </para>
 
     <para>
@@ -3004,7 +3007,10 @@ my_set_returning_function(PG_FUNCTION_ARGS)
     }
     else
     {
-        /* Here we are done returning items and just need to clean up: */
+        /*
+         * Here we are done returning items and just need to clean up, but no
+         * need to pfree allocations within multi_call_memory_ctx.
+         */
         <replaceable>user code</replaceable>
         SRF_RETURN_DONE(funcctx);
     }
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 50e98caaeb..cb92e049aa 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -3388,9 +3388,6 @@ pg_get_multixact_members(PG_FUNCTION_ARGS)
 		SRF_RETURN_NEXT(funccxt, HeapTupleGetDatum(tuple));
 	}
 
-	if (multi->nmembers > 0)
-		pfree(multi->members);
-	pfree(multi);
-
+	/* allocations in multi_call_memory_ctx are released automatically */
 	SRF_RETURN_DONE(funccxt);
 }
diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c
index 88005c0519..85eaf041d6 100644
--- a/src/backend/tsearch/wparser.c
+++ b/src/backend/tsearch/wparser.c
@@ -104,9 +104,8 @@ tt_process_call(FuncCallContext *funcctx)
 		st->cur++;
 		return result;
 	}
-	if (st->list)
-		pfree(st->list);
-	pfree(st);
+
+	/* allocations in multi_call_memory_ctx are released automatically */
 	return (Datum) 0;
 }
 
@@ -245,12 +244,8 @@ prs_process_call(FuncCallContext *funcctx)
 		st->cur++;
 		return result;
 	}
-	else
-	{
-		if (st->list)
-			pfree(st->list);
-		pfree(st);
-	}
+
+	/* allocations in multi_call_memory_ctx are released automatically */
 	return (Datum) 0;
 }
 
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index f92861d8d2..c5a286dd58 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -535,7 +535,6 @@ jsonb_object_keys(PG_FUNCTION_ARGS)
 {
 	FuncCallContext *funcctx;
 	OkeysState *state;
-	int			i;
 
 	if (SRF_IS_FIRSTCALL())
 	{
@@ -598,12 +597,7 @@ jsonb_object_keys(PG_FUNCTION_ARGS)
 		SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(nxt));
 	}
 
-	/* cleanup to reduce or eliminate memory leaks */
-	for (i = 0; i < state->result_count; i++)
-		pfree(state->result[i]);
-	pfree(state->result);
-	pfree(state);
-
+	/* allocations in multi_call_memory_ctx are released automatically */
 	SRF_RETURN_DONE(funcctx);
 }
 
@@ -706,7 +700,6 @@ json_object_keys(PG_FUNCTION_ARGS)
 {
 	FuncCallContext *funcctx;
 	OkeysState *state;
-	int			i;
 
 	if (SRF_IS_FIRSTCALL())
 	{
@@ -755,12 +748,7 @@ json_object_keys(PG_FUNCTION_ARGS)
 		SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(nxt));
 	}
 
-	/* cleanup to reduce or eliminate memory leaks */
-	for (i = 0; i < state->result_count; i++)
-		pfree(state->result[i]);
-	pfree(state->result);
-	pfree(state);
-
+	/* allocations in multi_call_memory_ctx are released automatically */
 	SRF_RETURN_DONE(funcctx);
 }
 
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 108dd998c7..e7b8fbc3ca 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -706,7 +706,7 @@ tsvector_unnest(PG_FUNCTION_ARGS)
 	}
 	else
 	{
-		pfree(tsin);
+		/* allocations in multi_call_memory_ctx are released automatically */
 		SRF_RETURN_DONE(funcctx);
 	}
 }
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index 471100b7e5..8f6db97fcf 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -77,7 +77,7 @@ typedef struct FuncCallContext
 	 * OPTIONAL pointer to miscellaneous user-provided context information
 	 *
 	 * user_fctx is for use as a pointer to your own struct to retain
-	 * arbitrary context information between calls of your function.
+	 * context information between calls of your function.
 	 */
 	void	   *user_fctx;
 
@@ -93,8 +93,9 @@ typedef struct FuncCallContext
 	/*
 	 * memory context used for structures that must live for multiple calls
 	 *
-	 * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and used
-	 * by SRF_RETURN_DONE() for cleanup. It is the most appropriate memory
+	 * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and
+	 * automatically cleaned up by SRF_RETURN_DONE(). It is the most
+	 * appropriate memory
 	 * context for any memory that is to be reused across multiple calls of
 	 * the SRF.
 	 */
-- 
2.17.0

