pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed

From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, Fabien COELHO <coelho@cri.ensmp.fr>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
Date: Mon, 16 Mar 2020 10:53:06 -0500
Message-ID: <20200316155306.GM26184@telsasoft.com> (raw)
In-Reply-To: <20200312121156.GB29065@telsasoft.com>
References: <20200308173103.GC1357@telsasoft.com>
	<27334.1583692669@sss.pgh.pa.us>
	<20200308191456.GD1357@telsasoft.com>
	<5679.1583696409@sss.pgh.pa.us>
	<20200311111921.GQ29065@telsasoft.com>
	<21724.1583955158@sss.pgh.pa.us>
	<20200312121156.GB29065@telsasoft.com>

On Thu, Mar 12, 2020 at 07:11:56AM -0500, Justin Pryzby wrote:
> > Do you want to have a go at that?
> 
> First draft attached.  Note that I handled pg_ls_dir, even though I'm proposing
> on the other thread to collapse/merge/meld it with pg_ls_dir_files [0].
> Possibly that's a bad idea with tuplestore, due to returning a scalar vs a row
> and needing to conditionally call CreateTemplateTupleDesc vs
> get_call_result_type.  I'll rebase that patch later today.
> 
> I didn't write test cases yet.  Also didn't look for functions not on your
> list.
> 
> I noticed this doesn't actually do anything, but kept it for now...except in
> pg_ls_dir error case:
> 
> src/include/utils/tuplestore.h:/* tuplestore_donestoring() used to be required, but is no longer used */
> src/include/utils/tuplestore.h:#define tuplestore_donestoring(state)    ((void) 0)

v2 attached - I will add to next CF in case you want to defer it until later.

-- 
Justin

Attachments:

  [text/x-diff] v2-0001-SRF-avoid-leaking-resources-if-not-run-to-complet.patch (28.9K, ../20200316155306.GM26184@telsasoft.com/2-v2-0001-SRF-avoid-leaking-resources-if-not-run-to-complet.patch)
  download | inline diff:
From 9ae75f693adf246ad551e258198606fd63190a20 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 11 Mar 2020 10:09:18 -0500
Subject: [PATCH v2 1/2] SRF: avoid leaking resources if not run to completion

Change to return a tuplestore populated immediately and returned in full.

Discussion: https://www.postgresql.org/message-id/20200308173103.GC1357%40telsasoft.com

See also: 9cb7db3f0 and 085b6b66
---
 contrib/adminpack/adminpack.c                |  82 +++++-----
 contrib/pgrowlocks/pgrowlocks.c              | 162 +++++++++----------
 doc/src/sgml/xfunc.sgml                      |  17 +-
 src/backend/utils/adt/datetime.c             |  99 +++++-------
 src/backend/utils/adt/genfile.c              | 111 +++++++------
 src/backend/utils/adt/misc.c                 | 116 +++++++------
 src/include/funcapi.h                        |   7 +-
 src/test/regress/expected/misc_functions.out |  18 +++
 src/test/regress/sql/misc_functions.sql      |   6 +
 9 files changed, 325 insertions(+), 293 deletions(-)

diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index bc45e79895..070025f5a8 100644
--- a/contrib/adminpack/adminpack.c
+++ b/contrib/adminpack/adminpack.c
@@ -504,50 +504,56 @@ pg_logdir_ls_v1_1(PG_FUNCTION_ARGS)
 static Datum
 pg_logdir_ls_internal(FunctionCallInfo fcinfo)
 {
-	FuncCallContext *funcctx;
 	struct dirent *de;
-	directory_fctx *fctx;
+
+	ReturnSetInfo	*rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	MemoryContext	oldcontext;
+	TupleDesc		tupdesc;
+	Tuplestorestate	*tupstore;
+	bool			randomAccess;
+	DIR				*dirdesc;
+
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsinfo->allowedModes & SFRM_Materialize))
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
 	if (strcmp(Log_filename, "postgresql-%Y-%m-%d_%H%M%S.log") != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("the log_filename parameter must equal 'postgresql-%%Y-%%m-%%d_%%H%%M%%S.log'")));
 
-	if (SRF_IS_FIRSTCALL())
-	{
-		MemoryContext oldcontext;
-		TupleDesc	tupdesc;
-
-		funcctx = SRF_FIRSTCALL_INIT();
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+	/* The Tuplestore and TupleDesc should be created in ecxt_per_query_memory */
+	oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+	randomAccess = (rsinfo->allowedModes&SFRM_Materialize_Random) != 0;
+	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
 
-		fctx = palloc(sizeof(directory_fctx));
+	/* Is this right ?? */
+	tupdesc = CreateTemplateTupleDesc(2);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 1, "starttime",
+			TIMESTAMPOID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 2, "filename",
+			TEXTOID, -1, 0);
 
-		tupdesc = CreateTemplateTupleDesc(2);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 1, "starttime",
-						   TIMESTAMPOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 2, "filename",
-						   TEXTOID, -1, 0);
+	rsinfo->returnMode = SFRM_Materialize;
+	rsinfo->setResult = tupstore;
+	rsinfo->setDesc = tupdesc;
 
-		funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+	MemoryContextSwitchTo(oldcontext);
 
-		fctx->location = pstrdup(Log_directory);
-		fctx->dirdesc = AllocateDir(fctx->location);
-
-		if (!fctx->dirdesc)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not open directory \"%s\": %m",
-							fctx->location)));
-
-		funcctx->user_fctx = fctx;
-		MemoryContextSwitchTo(oldcontext);
-	}
-
-	funcctx = SRF_PERCALL_SETUP();
-	fctx = (directory_fctx *) funcctx->user_fctx;
+	dirdesc = AllocateDir(Log_directory);
+	if (!dirdesc)
+		ereport(ERROR,
+				(errcode_for_file_access(),
+				 errmsg("could not open directory \"%s\": %m",
+						Log_directory)));
 
-	while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
+	while ((de = ReadDir(dirdesc, Log_directory)) != NULL)
 	{
 		char	   *values[2];
 		HeapTuple	tuple;
@@ -584,13 +590,13 @@ pg_logdir_ls_internal(FunctionCallInfo fcinfo)
 		/* Seems the timestamp is OK; prepare and return tuple */
 
 		values[0] = timestampbuf;
-		values[1] = psprintf("%s/%s", fctx->location, de->d_name);
-
-		tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
+		values[1] = psprintf("%s/%s", Log_directory, de->d_name);
 
-		SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
+		tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupdesc), values);
+		tuplestore_puttuple(tupstore, tuple);
 	}
 
-	FreeDir(fctx->dirdesc);
-	SRF_RETURN_DONE(funcctx);
+	tuplestore_donestoring(tupstore);
+	FreeDir(dirdesc);
+	return (Datum) 0;
 }
diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c
index a2c44a916c..baad83fcde 100644
--- a/contrib/pgrowlocks/pgrowlocks.c
+++ b/contrib/pgrowlocks/pgrowlocks.c
@@ -54,13 +54,6 @@ PG_FUNCTION_INFO_V1(pgrowlocks);
 
 #define NCHARS 32
 
-typedef struct
-{
-	Relation	rel;
-	TableScanDesc scan;
-	int			ncolumns;
-} MyData;
-
 #define		Atnum_tid		0
 #define		Atnum_xmax		1
 #define		Atnum_ismulti	2
@@ -71,82 +64,84 @@ typedef struct
 Datum
 pgrowlocks(PG_FUNCTION_ARGS)
 {
-	FuncCallContext *funcctx;
 	TableScanDesc scan;
 	HeapScanDesc hscan;
 	HeapTuple	tuple;
 	TupleDesc	tupdesc;
 	AttInMetadata *attinmeta;
-	Datum		result;
-	MyData	   *mydata;
 	Relation	rel;
 
-	if (SRF_IS_FIRSTCALL())
-	{
-		text	   *relname;
-		RangeVar   *relrv;
-		MemoryContext oldcontext;
-		AclResult	aclresult;
-
-		funcctx = SRF_FIRSTCALL_INIT();
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
-
-		/* Build a tuple descriptor for our result type */
-		if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
-			elog(ERROR, "return type must be a row type");
-
-		attinmeta = TupleDescGetAttInMetadata(tupdesc);
-		funcctx->attinmeta = attinmeta;
-
-		relname = PG_GETARG_TEXT_PP(0);
-		relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
-		rel = relation_openrv(relrv, AccessShareLock);
-
-		if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
-			ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							errmsg("only heap AM is supported")));
-
-		if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is a partitioned table",
-							RelationGetRelationName(rel)),
-					 errdetail("Partitioned tables do not contain rows.")));
-		else if (rel->rd_rel->relkind != RELKIND_RELATION)
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is not a table",
-							RelationGetRelationName(rel))));
+	ReturnSetInfo   *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	Tuplestorestate	*tupstore;
+	bool            randomAccess;
+
+	text	   *relname;
+	RangeVar   *relrv;
+	MemoryContext oldcontext;
+	AclResult	aclresult;
+	char	  **values;
+
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsinfo->allowedModes & SFRM_Materialize))
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
+
+	/* The Tuplestore and TupleDesc should be created in ecxt_per_query_memory */
+	oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+	randomAccess = (rsinfo->allowedModes&SFRM_Materialize_Random) != 0;
+	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
+	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, "return type must be a row type");
+	rsinfo->returnMode = SFRM_Materialize;
+	rsinfo->setResult = tupstore;
+	rsinfo->setDesc = tupdesc;
+
+	attinmeta = TupleDescGetAttInMetadata(tupdesc);
+
+	relname = PG_GETARG_TEXT_PP(0);
+	relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
+	rel = relation_openrv(relrv, AccessShareLock);
+
+	if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
+		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("only heap AM is supported")));
+
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("\"%s\" is a partitioned table",
+						RelationGetRelationName(rel)),
+				 errdetail("Partitioned tables do not contain rows.")));
+	else if (rel->rd_rel->relkind != RELKIND_RELATION)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("\"%s\" is not a table",
+						RelationGetRelationName(rel))));
+
+	/*
+	 * check permissions: must have SELECT on table or be in
+	 * pg_stat_scan_tables
+	 */
+	aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
+								  ACL_SELECT);
+	if (aclresult != ACLCHECK_OK)
+		aclresult = is_member_of_role(GetUserId(), DEFAULT_ROLE_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
+
+	if (aclresult != ACLCHECK_OK)
+		aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
+					   RelationGetRelationName(rel));
+
+	scan = table_beginscan(rel, GetActiveSnapshot(), 0, NULL);
+	hscan = (HeapScanDesc) scan;
 
-		/*
-		 * check permissions: must have SELECT on table or be in
-		 * pg_stat_scan_tables
-		 */
-		aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
-									  ACL_SELECT);
-		if (aclresult != ACLCHECK_OK)
-			aclresult = is_member_of_role(GetUserId(), DEFAULT_ROLE_STAT_SCAN_TABLES) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
-
-		if (aclresult != ACLCHECK_OK)
-			aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
-						   RelationGetRelationName(rel));
-
-		scan = table_beginscan(rel, GetActiveSnapshot(), 0, NULL);
-		hscan = (HeapScanDesc) scan;
-		mydata = palloc(sizeof(*mydata));
-		mydata->rel = rel;
-		mydata->scan = scan;
-		mydata->ncolumns = tupdesc->natts;
-		funcctx->user_fctx = mydata;
-
-		MemoryContextSwitchTo(oldcontext);
-	}
+	MemoryContextSwitchTo(oldcontext);
 
-	funcctx = SRF_PERCALL_SETUP();
-	attinmeta = funcctx->attinmeta;
-	mydata = (MyData *) funcctx->user_fctx;
-	scan = mydata->scan;
-	hscan = (HeapScanDesc) scan;
+	values = (char **) palloc(tupdesc->natts * sizeof(char *));
 
 	/* scan the relation */
 	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
@@ -169,9 +164,6 @@ pgrowlocks(PG_FUNCTION_ARGS)
 		 */
 		if (htsu == TM_BeingModified)
 		{
-			char	  **values;
-
-			values = (char **) palloc(mydata->ncolumns * sizeof(char *));
 
 			values[Atnum_tid] = (char *) DirectFunctionCall1(tidout,
 															 PointerGetDatum(&tuple->t_self));
@@ -297,16 +289,7 @@ pgrowlocks(PG_FUNCTION_ARGS)
 
 			/* build a tuple */
 			tuple = BuildTupleFromCStrings(attinmeta, values);
-
-			/* make the tuple into a datum */
-			result = HeapTupleGetDatum(tuple);
-
-			/*
-			 * no need to pfree what we allocated; it's on a short-lived
-			 * memory context anyway
-			 */
-
-			SRF_RETURN_NEXT(funcctx, result);
+			tuplestore_puttuple(tupstore, tuple);
 		}
 		else
 		{
@@ -314,8 +297,9 @@ pgrowlocks(PG_FUNCTION_ARGS)
 		}
 	}
 
+	tuplestore_donestoring(tupstore);
 	table_endscan(scan);
-	table_close(mydata->rel, AccessShareLock);
+	table_close(rel, AccessShareLock);
 
-	SRF_RETURN_DONE(funcctx);
+	return (Datum) 0;
 }
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index a91f887119..432758e75e 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -2812,7 +2812,7 @@ HeapTupleGetDatum(HeapTuple tuple)
     <title>Returning Sets</title>
 
     <para>
-     There is also a special API that provides support for returning
+     There is also a special, helper API that provides support for returning
      sets (multiple rows) from a C-language function.  A set-returning
      function must follow the version-1 calling conventions.  Also,
      source files must include <filename>funcapi.h</filename>, as
@@ -2820,10 +2820,19 @@ HeapTupleGetDatum(HeapTuple tuple)
     </para>
 
     <para>
-     A set-returning function (<acronym>SRF</acronym>) is called
-     once for each item it returns.  The <acronym>SRF</acronym> must
-     therefore save enough state to remember what it was doing and
+     The helper API allows writing a set-returning function
+     (<acronym>SRF</acronym>) using the ValuePerCall mode, in which the SRF is
+     called once for each item it returns.  The <acronym>SRF</acronym> must
+     then save enough state to remember what it was doing and
      return the next item on each call.
+     Note, however, that an SRF will possibly not be run to completion, due to
+     a LIMIT or a cursor, and should avoid leaving resources like DIR*s or
+     tablescans opened across calls.  To avoid leaking resources in those cases,
+     instead of using ValuePerCall mode, the SRF should populate and return a
+     tuplestore with SFRM_Materialize mode.  See fmgr/README.
+    </para>
+
+    <para>
      The structure <structname>FuncCallContext</structname> is provided to help
      control this process.  Within a function, <literal>fcinfo-&gt;flinfo-&gt;fn_extra</literal>
      is used to hold a pointer to <structname>FuncCallContext</structname>
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 4f109111d1..d0966c8305 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -4756,11 +4756,8 @@ Datum
 pg_timezone_names(PG_FUNCTION_ARGS)
 {
 	MemoryContext oldcontext;
-	FuncCallContext *funcctx;
 	pg_tzenum  *tzenum;
 	pg_tz	   *tz;
-	Datum		result;
-	HeapTuple	tuple;
 	Datum		values[4];
 	bool		nulls[4];
 	int			tzoff;
@@ -4770,58 +4767,44 @@ pg_timezone_names(PG_FUNCTION_ARGS)
 	Interval   *resInterval;
 	struct pg_tm itm;
 
-	/* stuff done only on the first call of the function */
-	if (SRF_IS_FIRSTCALL())
-	{
-		TupleDesc	tupdesc;
+	ReturnSetInfo   *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 
-		/* create a function context for cross-call persistence */
-		funcctx = SRF_FIRSTCALL_INIT();
+	TupleDesc	tupdesc;
+	Tuplestorestate *tupstore;
+	bool            randomAccess;
 
-		/*
-		 * switch to memory context appropriate for multiple function calls
-		 */
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsinfo->allowedModes & SFRM_Materialize))
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
-		/* initialize timezone scanning code */
-		tzenum = pg_tzenumerate_start();
-		funcctx->user_fctx = (void *) tzenum;
+	/* The Tuplestore and TupleDesc should be created in ecxt_per_query_memory */
+	oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+	randomAccess = (rsinfo->allowedModes&SFRM_Materialize_Random) != 0;
+	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
+	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, "return type must be a row type");
 
-		/*
-		 * build tupdesc for result tuples. This must match this function's
-		 * pg_proc entry!
-		 */
-		tupdesc = CreateTemplateTupleDesc(4);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
-						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 2, "abbrev",
-						   TEXTOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 3, "utc_offset",
-						   INTERVALOID, -1, 0);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 4, "is_dst",
-						   BOOLOID, -1, 0);
+	rsinfo->returnMode = SFRM_Materialize;
+	rsinfo->setResult = tupstore;
+	rsinfo->setDesc = tupdesc;
 
-		funcctx->tuple_desc = BlessTupleDesc(tupdesc);
-		MemoryContextSwitchTo(oldcontext);
-	}
+	MemoryContextSwitchTo(oldcontext);
 
-	/* stuff done on every call of the function */
-	funcctx = SRF_PERCALL_SETUP();
-	tzenum = (pg_tzenum *) funcctx->user_fctx;
+	/* initialize timezone scanning code */
+	tzenum = pg_tzenumerate_start();
 
 	/* search for another zone to display */
 	for (;;)
 	{
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
 		tz = pg_tzenumerate_next(tzenum);
-		MemoryContextSwitchTo(oldcontext);
-
 		if (!tz)
-		{
-			pg_tzenumerate_end(tzenum);
-			funcctx->user_fctx = NULL;
-			SRF_RETURN_DONE(funcctx);
-		}
+			break;
 
 		/* Convert now() to local time in this zone */
 		if (timestamp2tm(GetCurrentTransactionStartTimestamp(),
@@ -4840,25 +4823,23 @@ pg_timezone_names(PG_FUNCTION_ARGS)
 		if (tzn && strlen(tzn) > 31)
 			continue;
 
-		/* Found a displayable zone */
-		break;
-	}
-
-	MemSet(nulls, 0, sizeof(nulls));
 
-	values[0] = CStringGetTextDatum(pg_get_timezone_name(tz));
-	values[1] = CStringGetTextDatum(tzn ? tzn : "");
+		MemSet(nulls, 0, sizeof(nulls));
+		values[0] = CStringGetTextDatum(pg_get_timezone_name(tz));
+		values[1] = CStringGetTextDatum(tzn ? tzn : "");
 
-	MemSet(&itm, 0, sizeof(struct pg_tm));
-	itm.tm_sec = -tzoff;
-	resInterval = (Interval *) palloc(sizeof(Interval));
-	tm2interval(&itm, 0, resInterval);
-	values[2] = IntervalPGetDatum(resInterval);
+		MemSet(&itm, 0, sizeof(struct pg_tm));
+		itm.tm_sec = -tzoff;
+		resInterval = (Interval *) palloc(sizeof(Interval));
+		tm2interval(&itm, 0, resInterval);
+		values[2] = IntervalPGetDatum(resInterval);
 
-	values[3] = BoolGetDatum(tm.tm_isdst > 0);
+		values[3] = BoolGetDatum(tm.tm_isdst > 0);
 
-	tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls);
-	result = HeapTupleGetDatum(tuple);
+		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+	}
 
-	SRF_RETURN_NEXT(funcctx, result);
+	pg_tzenumerate_end(tzenum);
+	tuplestore_donestoring(tupstore);
+	return (Datum) 0;
 }
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index bcf9bd1b97..39a54d532f 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -36,13 +36,6 @@
 #include "utils/syscache.h"
 #include "utils/timestamp.h"
 
-typedef struct
-{
-	char	   *location;
-	DIR		   *dirdesc;
-	bool		include_dot_dirs;
-} directory_fctx;
-
 
 /*
  * Convert a "text" filename argument to C string, and check it's allowable.
@@ -447,67 +440,81 @@ pg_stat_file_1arg(PG_FUNCTION_ARGS)
 Datum
 pg_ls_dir(PG_FUNCTION_ARGS)
 {
-	FuncCallContext *funcctx;
-	struct dirent *de;
-	directory_fctx *fctx;
-	MemoryContext oldcontext;
+	ReturnSetInfo	*rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	MemoryContext	oldcontext;
+	TupleDesc		tupdesc;
+	Tuplestorestate *tupstore;
+	bool			randomAccess;
+
+	bool			missing_ok = false;
+	bool			include_dot_dirs = false;
+	char			*location;
+	DIR				*dirdesc;
+	struct dirent	*de;
+
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsinfo->allowedModes & SFRM_Materialize))
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
-	if (SRF_IS_FIRSTCALL())
+	/* check the optional arguments */
+	if (PG_NARGS() == 3)
 	{
-		bool		missing_ok = false;
-		bool		include_dot_dirs = false;
+		if (!PG_ARGISNULL(1))
+			missing_ok = PG_GETARG_BOOL(1);
+		if (!PG_ARGISNULL(2))
+			include_dot_dirs = PG_GETARG_BOOL(2);
+	}
 
-		/* check the optional arguments */
-		if (PG_NARGS() == 3)
-		{
-			if (!PG_ARGISNULL(1))
-				missing_ok = PG_GETARG_BOOL(1);
-			if (!PG_ARGISNULL(2))
-				include_dot_dirs = PG_GETARG_BOOL(2);
-		}
+	/* The Tuplestore and TupleDesc should be created in ecxt_per_query_memory */
+	oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+	randomAccess = (rsinfo->allowedModes&SFRM_Materialize_Random) != 0;
+	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
 
-		funcctx = SRF_FIRSTCALL_INIT();
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+	tupdesc = CreateTemplateTupleDesc(1);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 1, "column", TEXTOID, -1, 0);
 
-		fctx = palloc(sizeof(directory_fctx));
-		fctx->location = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
+	rsinfo->returnMode = SFRM_Materialize;
+	rsinfo->setResult = tupstore;
+	rsinfo->setDesc = tupdesc;
+	MemoryContextSwitchTo(oldcontext);
 
-		fctx->include_dot_dirs = include_dot_dirs;
-		fctx->dirdesc = AllocateDir(fctx->location);
+	location = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
+	dirdesc = AllocateDir(location);
 
-		if (!fctx->dirdesc)
-		{
-			if (missing_ok && errno == ENOENT)
-			{
-				MemoryContextSwitchTo(oldcontext);
-				SRF_RETURN_DONE(funcctx);
-			}
-			else
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not open directory \"%s\": %m",
-								fctx->location)));
-		}
-		funcctx->user_fctx = fctx;
-		MemoryContextSwitchTo(oldcontext);
+	if (!dirdesc)
+	{
+		if (missing_ok && errno == ENOENT)
+			return (Datum) 0;
+		else
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open directory \"%s\": %m",
+							location)));
 	}
 
-	funcctx = SRF_PERCALL_SETUP();
-	fctx = (directory_fctx *) funcctx->user_fctx;
-
-	while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
+	while ((de = ReadDir(dirdesc, location)) != NULL)
 	{
-		if (!fctx->include_dot_dirs &&
+		Datum		values[1];
+		bool		nulls[1] = {false};
+
+		if (!include_dot_dirs &&
 			(strcmp(de->d_name, ".") == 0 ||
 			 strcmp(de->d_name, "..") == 0))
 			continue;
 
-		SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(de->d_name));
+		values[0] = CStringGetTextDatum(de->d_name);
+		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
 
-	FreeDir(fctx->dirdesc);
-
-	SRF_RETURN_DONE(funcctx);
+	tuplestore_donestoring(tupstore);
+	FreeDir(dirdesc);
+	return (Datum) 0;
 }
 
 /*
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 323e36b81c..9919087335 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -194,72 +194,86 @@ current_query(PG_FUNCTION_ARGS)
 
 /* Function to find out which databases make use of a tablespace */
 
-typedef struct
-{
-	char	   *location;
-	DIR		   *dirdesc;
-} ts_db_fctx;
-
 Datum
 pg_tablespace_databases(PG_FUNCTION_ARGS)
 {
-	FuncCallContext *funcctx;
 	struct dirent *de;
-	ts_db_fctx *fctx;
 
-	if (SRF_IS_FIRSTCALL())
-	{
-		MemoryContext oldcontext;
-		Oid			tablespaceOid = PG_GETARG_OID(0);
+	MemoryContext oldcontext;
+	Oid			tablespaceOid = PG_GETARG_OID(0);
 
-		funcctx = SRF_FIRSTCALL_INIT();
-		oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+	ReturnSetInfo	*rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	TupleDesc		tupdesc;
+	Tuplestorestate *tupstore;
+	bool			randomAccess;
 
-		fctx = palloc(sizeof(ts_db_fctx));
+	DIR				*dirdesc;
+	char			*location;
 
-		if (tablespaceOid == GLOBALTABLESPACE_OID)
-		{
-			fctx->dirdesc = NULL;
-			ereport(WARNING,
-					(errmsg("global tablespace never has databases")));
-		}
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsinfo->allowedModes & SFRM_Materialize))
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
+
+	if (tablespaceOid == GLOBALTABLESPACE_OID)
+	{
+		dirdesc = NULL;
+		ereport(WARNING,
+				(errmsg("global tablespace never has databases")));
+	}
+	else
+	{
+		if (tablespaceOid == DEFAULTTABLESPACE_OID)
+			location = psprintf("base");
 		else
-		{
-			if (tablespaceOid == DEFAULTTABLESPACE_OID)
-				fctx->location = psprintf("base");
-			else
-				fctx->location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
-										  TABLESPACE_VERSION_DIRECTORY);
+			location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
+									  TABLESPACE_VERSION_DIRECTORY);
 
-			fctx->dirdesc = AllocateDir(fctx->location);
+		dirdesc = AllocateDir(location);
 
-			if (!fctx->dirdesc)
-			{
-				/* the only expected error is ENOENT */
-				if (errno != ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not open directory \"%s\": %m",
-									fctx->location)));
-				ereport(WARNING,
-						(errmsg("%u is not a tablespace OID", tablespaceOid)));
-			}
+		if (!dirdesc)
+		{
+			/* the only expected error is ENOENT */
+			if (errno != ENOENT)
+				ereport(ERROR,
+						(errcode_for_file_access(),
+						 errmsg("could not open directory \"%s\": %m",
+								location)));
+			ereport(WARNING,
+					(errmsg("%u is not a tablespace OID", tablespaceOid)));
 		}
-		funcctx->user_fctx = fctx;
-		MemoryContextSwitchTo(oldcontext);
 	}
 
-	funcctx = SRF_PERCALL_SETUP();
-	fctx = (ts_db_fctx *) funcctx->user_fctx;
+	/* The Tuplestore and TupleDesc should be created in ecxt_per_query_memory */
+	oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+
+	randomAccess = (rsinfo->allowedModes&SFRM_Materialize_Random) != 0;
+	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
 
-	if (!fctx->dirdesc)			/* not a tablespace */
-		SRF_RETURN_DONE(funcctx);
+	tupdesc = CreateTemplateTupleDesc(1);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 1, "column", OIDOID, -1, 0);
 
-	while ((de = ReadDir(fctx->dirdesc, fctx->location)) != NULL)
+	rsinfo->returnMode = SFRM_Materialize;
+	rsinfo->setResult = tupstore;
+	rsinfo->setDesc = tupdesc;
+
+	MemoryContextSwitchTo(oldcontext);
+
+	if (!dirdesc)			/* not a tablespace */
+		return 0;
+
+	while ((de = ReadDir(dirdesc, location)) != NULL)
 	{
 		Oid			datOid = atooid(de->d_name);
 		char	   *subdir;
 		bool		isempty;
+		Datum		values[1];
+		bool		nulls[1] = {false};
 
 		/* this test skips . and .., but is awfully weak */
 		if (!datOid)
@@ -267,18 +281,20 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 
 		/* if database subdir is empty, don't report tablespace as used */
 
-		subdir = psprintf("%s/%s", fctx->location, de->d_name);
+		subdir = psprintf("%s/%s", location, de->d_name);
 		isempty = directory_is_empty(subdir);
 		pfree(subdir);
 
 		if (isempty)
 			continue;			/* indeed, nothing in it */
 
-		SRF_RETURN_NEXT(funcctx, ObjectIdGetDatum(datOid));
+		values[0] = ObjectIdGetDatum(datOid);
+		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
 
-	FreeDir(fctx->dirdesc);
-	SRF_RETURN_DONE(funcctx);
+	tuplestore_donestoring(tupstore);
+	FreeDir(dirdesc);
+	return (Datum) 0;
 }
 
 
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index f9b75ae390..471100b7e5 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -234,7 +234,12 @@ extern Datum HeapTupleHeaderGetDatum(HeapTupleHeader tuple);
 /*----------
  *		Support for Set Returning Functions (SRFs)
  *
- * The basic API for SRFs looks something like:
+ * The basic API for SRFs using ValuePerCall mode looks something like this.
+ * Note that an SRF will possibly not be run to completion, due to a LIMIT or a
+ * cursor, and should avoid leaving resources like DIR*s or tablescans opened
+ * across calls.  To avoid leaking resources in those cases, instead of using
+ * ValuePerCall mode, the SRF should populate and return a tuplestore with
+ * SFRM_Materialize mode.  See fmgr/README.
  *
  * Datum
  * my_Set_Returning_Function(PG_FUNCTION_ARGS)
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index e217b678d7..b56ce74586 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -180,6 +180,24 @@ select count(*) >= 0 as ok from pg_ls_archive_statusdir();
  t
 (1 row)
 
+select * from (select pg_ls_dir('.')a)a where a='base' limit 1;
+  a   
+------
+ base
+(1 row)
+
+select * from (select (pg_timezone_names()).name)ptn where name='UTC' limit 1;
+ name 
+------
+ UTC
+(1 row)
+
+select datname from (select pg_tablespace_databases(b.oid) as pts from pg_tablespace b where b.spcname!='pg_global')pts join pg_database db on pts.pts=db.oid limit 1;
+  datname  
+-----------
+ template1
+(1 row)
+
 --
 -- Test adding a support function to a subject function
 --
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 1e11eb3554..5441b8b643 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -51,6 +51,12 @@ from (select pg_ls_waldir() w) ss where length((w).name) = 24 limit 1;
 
 select count(*) >= 0 as ok from pg_ls_archive_statusdir();
 
+select * from (select pg_ls_dir('.')a)a where a='base' limit 1;
+
+select * from (select (pg_timezone_names()).name)ptn where name='UTC' limit 1;
+
+select datname from (select pg_tablespace_databases(b.oid) as pts from pg_tablespace b where b.spcname!='pg_global')pts join pg_database db on pts.pts=db.oid limit 1;
+
 --
 -- Test adding a support function to a subject function
 --
-- 
2.17.0

  [text/x-diff] v2-0002-Clean-up-existing-anti-pattern-of-freeing-SRF-res.patch (8.2K, ../20200316155306.GM26184@telsasoft.com/3-v2-0002-Clean-up-existing-anti-pattern-of-freeing-SRF-res.patch)
  download | inline diff:
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

view thread (30+ messages)  latest in thread

Message-ID: <20200316155306.GM26184@telsasoft.com>
Permalink:  ../20200316155306.GM26184@telsasoft.com/
Also on:    postgresql.org/message-id/20200316155306.GM26184@telsasoft.com

copy link · copy postgr.es · copy git am

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: pryzby@telsasoft.com, tgl@sss.pgh.pa.us, coelho@cri.ensmp.fr, alvherre@2ndquadrant.com, david@pgmasters.net, bossartn@amazon.com, thomas.munro@gmail.com
  Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
  In-Reply-To: <20200316155306.GM26184@telsasoft.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox