v2-0001-plpython-Use-funccache.c-infrastructure-for-proce.patch

text/plain

Filename: v2-0001-plpython-Use-funccache.c-infrastructure-for-proce.patch
Type: text/plain
Part: 0
Message: Re: BUG #19480: PL/Python SRF crashes (SIGSEGV) when function is replaced mid-iteration: use-after-free in PLy_funct

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v2-0001
Subject: plpython: Use funccache.c infrastructure for procedure caching
File+
src/pl/plpython/expected/plpython_setof.out 23 0
src/pl/plpython/plpy_exec.c 96 109
src/pl/plpython/plpy_exec.h 1 1
src/pl/plpython/plpy_main.c 56 19
src/pl/plpython/plpy_procedure.c 228 111
src/pl/plpython/plpy_procedure.h 34 18
src/pl/plpython/sql/plpython_setof.sql 13 0
src/tools/pgindent/typedefs.list 1 2
From b17118b2178cca1855be784e20fb297be2b3c7e4 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Fri, 5 Jun 2026 10:51:53 -0300
Subject: [PATCH v2] plpython: Use funccache.c infrastructure for procedure
 caching

PL/Python set-returning functions can crash with a use-after-free when
CREATE OR REPLACE FUNCTION is executed while the SRF is mid-iteration.
The crash occurs because srfstate->savedargs is allocated in proc->mcxt,
which gets deleted when the procedure is invalidated, leaving a dangling
pointer that PLy_function_restore_args() then dereferences.

The fix is to use reference counting to prevent destroying the function
state while it's still in use, similar to what PL/pgSQL has done. This
commit converts PL/Python to use the funccache.c infrastructure
introduced in v18.

The main challenge is that PL/Python uses SFRM_ValuePerCall for SRFs,
where the handler is called multiple times with use_count potentially
returning to zero between calls. SQL functions face the same challenge,
so this commit follows the same approach used in functions.c: maintain
a per-call-site cache struct (PLyProcedureCache) in fn_extra that holds
both the pointer to the long-lived PLyProcedure and the SRF execution
state.

The use_count is incremented when we first obtain the procedure and is
decremented via a MemoryContextCallback registered on fn_mcxt, which runs
even during error aborts. Cleaning up the per-call SRF state needs more
care: an ExprContextCallback handles the in-query cases, since the
iterator is not guaranteed to run to completion (for example a LIMIT or a
rescan can abandon it early). But unlike SQL functions, whose resources
are released by transaction abort, PL/Python holds Python reference counts
on the iterator and saved arguments that abort will not release, and
ExprContextCallbacks are not invoked during an error abort. The
MemoryContextCallback on fn_mcxt therefore doubles as the backstop that
releases those references when a query errors out mid-iteration.

Since fn_extra is now used for PLyProcedureCache, this commit removes
the SRF macros (SRF_IS_FIRSTCALL, SRF_RETURN_NEXT, etc.) and switches to
direct isDone signaling via ReturnSetInfo, matching how SQL functions
handle ValuePerCall mode.

Author: Matheus Alcantara <mths.dev@pm.me>
Reported-by: Andrzej Doros <adoros@starfishstorage.com>
Suggested-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>

Discussion: https://www.postgresql.org/message-id/19480-f1f9fdce30462fc4%40postgresql.org
---
 src/pl/plpython/expected/plpython_setof.out |  23 ++
 src/pl/plpython/plpy_exec.c                 | 205 ++++++------
 src/pl/plpython/plpy_exec.h                 |   2 +-
 src/pl/plpython/plpy_main.c                 |  75 +++--
 src/pl/plpython/plpy_procedure.c            | 339 +++++++++++++-------
 src/pl/plpython/plpy_procedure.h            |  52 +--
 src/pl/plpython/sql/plpython_setof.sql      |  13 +
 src/tools/pgindent/typedefs.list            |   3 +-
 8 files changed, 452 insertions(+), 260 deletions(-)

diff --git a/src/pl/plpython/expected/plpython_setof.out b/src/pl/plpython/expected/plpython_setof.out
index c4461ac2762..68457c8a74b 100644
--- a/src/pl/plpython/expected/plpython_setof.out
+++ b/src/pl/plpython/expected/plpython_setof.out
@@ -228,3 +228,26 @@ SELECT * FROM get_user_records2();
  willem | doe   | w_doe    |      3
 (4 rows)
 
+-- A set-returning function that is invalidated mid-iteration must run to
+-- completion using its original definition (bug #19480).
+CREATE OR REPLACE FUNCTION self_invalidating_srf(x int) RETURNS SETOF int AS $$
+for i in range(3):
+    if i == 1:
+        plpy.execute("CREATE OR REPLACE FUNCTION self_invalidating_srf(x int) "
+                     "RETURNS SETOF int LANGUAGE plpython3u AS 'return [-1]'")
+    yield x + i
+$$ LANGUAGE plpython3u;
+SELECT self_invalidating_srf(10); -- expect 10,11,12 (original definition)
+ self_invalidating_srf 
+-----------------------
+                    10
+                    11
+                    12
+(3 rows)
+
+SELECT self_invalidating_srf(10); -- expect -1 (replacement now in effect)
+ self_invalidating_srf 
+-----------------------
+                    -1
+(1 row)
+
diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index de0dad1f533..e3a7497a2e4 100644
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -22,22 +22,13 @@
 #include "utils/fmgrprotos.h"
 #include "utils/rel.h"
 
-/* saved state for a set-returning function */
-typedef struct PLySRFState
-{
-	PyObject   *iter;			/* Python iterator producing results */
-	PLySavedArgs *savedargs;	/* function argument values */
-	MemoryContextCallback callback; /* for releasing refcounts when done */
-} PLySRFState;
-
 static PyObject *PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc);
-static PLySavedArgs *PLy_function_save_args(PLyProcedure *proc);
+static PLySavedArgs *PLy_function_save_args(MemoryContext mctx, PLyProcedure *proc);
 static void PLy_function_restore_args(PLyProcedure *proc, PLySavedArgs *savedargs);
-static void PLy_function_drop_args(PLySavedArgs *savedargs);
 static void PLy_global_args_push(PLyProcedure *proc);
 static void PLy_global_args_pop(PLyProcedure *proc);
-static void plpython_srf_cleanup_callback(void *arg);
 static void plpython_return_error_callback(void *arg);
+static void ShutdownPLyFunction(Datum arg);
 
 static PyObject *PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc,
 										HeapTuple *rv);
@@ -51,14 +42,15 @@ static void PLy_abort_open_subtransactions(int save_subxact_level);
 
 /* function subhandler */
 Datum
-PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
+PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedureCache *pcache)
 {
+	PLyProcedure *proc = pcache->proc;
 	bool		is_setof = proc->is_setof;
 	Datum		rv;
 	PyObject   *volatile plargs = NULL;
 	PyObject   *volatile plrv = NULL;
-	FuncCallContext *volatile funcctx = NULL;
 	PLySRFState *volatile srfstate = NULL;
+	ReturnSetInfo *rsi = NULL;
 	ErrorContextCallback plerrcontext;
 
 	/*
@@ -72,25 +64,42 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
 	{
 		if (is_setof)
 		{
-			/* First Call setup */
-			if (SRF_IS_FIRSTCALL())
+			rsi = (ReturnSetInfo *) fcinfo->resultinfo;
+
+			/*
+			 * PL/Python returns a set in ValuePerCall mode, so the handler is
+			 * invoked once per result row.  Across those calls we keep the
+			 * iterator and saved arguments in the per-call-site cache
+			 * (pcache->srfstate); a NULL srfstate means this is the first
+			 * call of a new iteration, so we set that state up here.
+			 */
+			if (pcache->srfstate == NULL)
 			{
-				funcctx = SRF_FIRSTCALL_INIT();
-				srfstate = (PLySRFState *)
-					MemoryContextAllocZero(funcctx->multi_call_memory_ctx,
-										   sizeof(PLySRFState));
-				/* Immediately register cleanup callback */
-				srfstate->callback.func = plpython_srf_cleanup_callback;
-				srfstate->callback.arg = srfstate;
-				MemoryContextRegisterResetCallback(funcctx->multi_call_memory_ctx,
-												   &srfstate->callback);
-				funcctx->user_fctx = srfstate;
+				if (!rsi || !IsA(rsi, ReturnSetInfo) ||
+					(rsi->allowedModes & SFRM_ValuePerCall) == 0)
+				{
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("unsupported set function return mode"),
+							 errdetail("PL/Python set-returning functions only support returning one value per call.")));
+				}
+				rsi->returnMode = SFRM_ValuePerCall;
+
+				pcache->srfstate = (PLySRFState *)
+					MemoryContextAllocZero(pcache->fcontext, sizeof(PLySRFState));
+
+				/*
+				 * Register a shutdown callback so that the iterator state is
+				 * released if execution is abandoned before the iterator is
+				 * exhausted. We unregister it again on normal completion.
+				 */
+				RegisterExprContextCallback(rsi->econtext,
+											ShutdownPLyFunction,
+											PointerGetDatum(pcache));
+				pcache->shutdown_reg = true;
 			}
-			/* Every call setup */
-			funcctx = SRF_PERCALL_SETUP();
-			Assert(funcctx != NULL);
-			srfstate = (PLySRFState *) funcctx->user_fctx;
-			Assert(srfstate != NULL);
+
+			srfstate = pcache->srfstate;
 		}
 
 		if (srfstate == NULL || srfstate->iter == NULL)
@@ -127,20 +136,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
 		{
 			if (srfstate->iter == NULL)
 			{
-				/* first time -- do checks and setup */
-				ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo;
-
-				if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-					(rsi->allowedModes & SFRM_ValuePerCall) == 0)
-				{
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("unsupported set function return mode"),
-							 errdetail("PL/Python set-returning functions only support returning one value per call.")));
-				}
-				rsi->returnMode = SFRM_ValuePerCall;
-
-				/* Make iterator out of returned object */
+				/* first time -- make iterator out of returned object */
 				srfstate->iter = PyObject_GetIter(plrv);
 
 				Py_DECREF(plrv);
@@ -177,7 +173,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
 				 * this again each time in case the iterator is changing those
 				 * values.
 				 */
-				srfstate->savedargs = PLy_function_save_args(proc);
+				srfstate->savedargs = PLy_function_save_args(pcache->fcontext, proc);
 			}
 		}
 
@@ -260,21 +256,16 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
 		Py_XDECREF(plrv);
 
 		/*
-		 * If there was an error within a SRF, the iterator might not have
-		 * been exhausted yet.  Clear it so the next invocation of the
-		 * function will start the iteration again.  (This code is probably
-		 * unnecessary now; plpython_srf_cleanup_callback should take care of
-		 * cleanup.  But it doesn't hurt anything to do it here.)
+		 * If the error was thrown within a SRF, clean up its state here. This
+		 * is the only cleanup hook that runs for an error thrown during the
+		 * function's own execution: ShutdownPLyFunction is not called on
+		 * abort, and the memory-context callback only fires once the
+		 * FmgrInfo's context is torn down.  Releasing the Python references
+		 * promptly avoids leaking them if teardown is delayed, and clearing
+		 * pcache->srfstate ensures a reused cache won't mistake this for an
+		 * iteration still in progress.
 		 */
-		if (srfstate)
-		{
-			Py_XDECREF(srfstate->iter);
-			srfstate->iter = NULL;
-			/* And drop any saved args; we won't need them */
-			if (srfstate->savedargs)
-				PLy_function_drop_args(srfstate->savedargs);
-			srfstate->savedargs = NULL;
-		}
+		PLy_function_cleanup_srfstate(pcache);
 
 		PG_RE_THROW();
 	}
@@ -290,22 +281,59 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc)
 
 	if (srfstate)
 	{
-		/* We're in a SRF, exit appropriately */
+		/* We're in a SRF, signal via rsi->isDone */
 		if (srfstate->iter == NULL)
 		{
-			/* Iterator exhausted, so we're done */
-			SRF_RETURN_DONE(funcctx);
+			/*
+			 * Iterator exhausted.  Unregister the shutdown callback since
+			 * we're done normally, then clean up srfstate.  (srfstate->iter
+			 * is already NULL here, so the cleanup just frees the struct.)
+			 */
+			if (pcache->shutdown_reg)
+			{
+				UnregisterExprContextCallback(rsi->econtext,
+											  ShutdownPLyFunction,
+											  PointerGetDatum(pcache));
+				pcache->shutdown_reg = false;
+			}
+			PLy_function_cleanup_srfstate(pcache);
+
+			rsi->isDone = ExprEndResult;
+			fcinfo->isnull = true;
+			return (Datum) 0;
 		}
-		else if (fcinfo->isnull)
-			SRF_RETURN_NEXT_NULL(funcctx);
 		else
-			SRF_RETURN_NEXT(funcctx, rv);
+		{
+			rsi->isDone = ExprMultipleResult;
+			return rv;
+		}
 	}
 
 	/* Plain function, just return the Datum value (possibly null) */
 	return rv;
 }
 
+/*
+ * ExprContext shutdown callback, invoked when the expression context that ran
+ * a SRF is rescanned or freed at end of query.  This handles in-query
+ * cancellation, e.g. a LIMIT that stops fetching before the iterator is
+ * exhausted, or a rescan of the owning plan node.
+ *
+ * NB: this is not called during an error abort (see ShutdownExprContext()).
+ * The companion memory-context callback RemovePLyProcedureCache() covers that
+ * case, since memory-context reset/delete callbacks do run during abort.
+ */
+static void
+ShutdownPLyFunction(Datum arg)
+{
+	PLyProcedureCache *pcache = (PLyProcedureCache *) DatumGetPointer(arg);
+
+	/* execUtils.c will deregister the callback after we return */
+	pcache->shutdown_reg = false;
+
+	PLy_function_cleanup_srfstate(pcache);
+}
+
 /*
  * trigger subhandler
  *
@@ -536,13 +564,13 @@ PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc)
  * available via the proc's globals :-( ... but we're stuck with that now.
  */
 static PLySavedArgs *
-PLy_function_save_args(PLyProcedure *proc)
+PLy_function_save_args(MemoryContext mctx, PLyProcedure *proc)
 {
 	PLySavedArgs *result;
 
 	/* saved args are always allocated in procedure's context */
 	result = (PLySavedArgs *)
-		MemoryContextAllocZero(proc->mcxt,
+		MemoryContextAllocZero(mctx,
 							   offsetof(PLySavedArgs, namedargs) +
 							   proc->nargs * sizeof(PyObject *));
 	result->nargs = proc->nargs;
@@ -618,28 +646,6 @@ PLy_function_restore_args(PLyProcedure *proc, PLySavedArgs *savedargs)
 	pfree(savedargs);
 }
 
-/*
- * Free a PLySavedArgs struct without restoring the values.
- */
-static void
-PLy_function_drop_args(PLySavedArgs *savedargs)
-{
-	int			i;
-
-	/* Drop references for named args */
-	for (i = 0; i < savedargs->nargs; i++)
-	{
-		Py_XDECREF(savedargs->namedargs[i]);
-	}
-
-	/* Drop refs to the "args" and "TD" objects, too */
-	Py_XDECREF(savedargs->args);
-	Py_XDECREF(savedargs->td);
-
-	/* And free the PLySavedArgs struct */
-	pfree(savedargs);
-}
-
 /*
  * Save away any existing arguments for the given procedure, so that we can
  * install new values for a recursive call.  This should be invoked before
@@ -659,7 +665,7 @@ PLy_global_args_push(PLyProcedure *proc)
 		PLySavedArgs *node;
 
 		/* Build a struct containing current argument values */
-		node = PLy_function_save_args(proc);
+		node = PLy_function_save_args(proc->mcxt, proc);
 
 		/*
 		 * Push the saved argument values into the procedure's stack.  Once we
@@ -713,25 +719,6 @@ PLy_global_args_pop(PLyProcedure *proc)
 	}
 }
 
-/*
- * Memory context deletion callback for cleaning up a PLySRFState.
- * We need this in case execution of the SRF is terminated early,
- * due to error or the caller simply not running it to completion.
- */
-static void
-plpython_srf_cleanup_callback(void *arg)
-{
-	PLySRFState *srfstate = (PLySRFState *) arg;
-
-	/* Release refcount on the iter, if we still have one */
-	Py_XDECREF(srfstate->iter);
-	srfstate->iter = NULL;
-	/* And drop any saved args; we won't need them */
-	if (srfstate->savedargs)
-		PLy_function_drop_args(srfstate->savedargs);
-	srfstate->savedargs = NULL;
-}
-
 static void
 plpython_return_error_callback(void *arg)
 {
diff --git a/src/pl/plpython/plpy_exec.h b/src/pl/plpython/plpy_exec.h
index f35eabbd8ee..1ade1bae151 100644
--- a/src/pl/plpython/plpy_exec.h
+++ b/src/pl/plpython/plpy_exec.h
@@ -7,7 +7,7 @@
 
 #include "plpy_procedure.h"
 
-extern Datum PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc);
+extern Datum PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedureCache *pcache);
 extern HeapTuple PLy_exec_trigger(FunctionCallInfo fcinfo, PLyProcedure *proc);
 extern void PLy_exec_event_trigger(FunctionCallInfo fcinfo, PLyProcedure *proc);
 
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 9f07c115f80..98ec6a28a9b 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -103,8 +103,6 @@ _PG_init(void)
 
 	Py_DECREF(main_mod);
 
-	init_procedure_caches();
-
 	explicit_subtransactions = NIL;
 
 	PLy_execution_contexts = NULL;
@@ -113,9 +111,13 @@ _PG_init(void)
 Datum
 plpython3_validator(PG_FUNCTION_ARGS)
 {
+	LOCAL_FCINFO(fake_fcinfo, 0);
 	Oid			funcoid = PG_GETARG_OID(0);
 	HeapTuple	tuple;
 	Form_pg_proc procStruct;
+	FmgrInfo	flinfo;
+	TriggerData trigdata;
+	EventTriggerData etrigdata;
 	PLyTrigType is_trigger;
 
 	if (!CheckFunctionValidatorAccess(fcinfo->flinfo->fn_oid, funcoid))
@@ -134,8 +136,33 @@ plpython3_validator(PG_FUNCTION_ARGS)
 
 	ReleaseSysCache(tuple);
 
+	/*
+	 * Set up a fake fcinfo with just enough info to satisfy
+	 * PLy_procedure_get().  That function derives the call context (plain
+	 * function, DML trigger, or event trigger) from the fcinfo, so we have to
+	 * fake up the matching context node here.
+	 */
+	MemSet(fake_fcinfo, 0, SizeForFunctionCallInfo(0));
+	MemSet(&flinfo, 0, sizeof(flinfo));
+	fake_fcinfo->flinfo = &flinfo;
+	flinfo.fn_oid = funcoid;
+	flinfo.fn_mcxt = CurrentMemoryContext;
+
+	if (is_trigger == PLPY_TRIGGER)
+	{
+		MemSet(&trigdata, 0, sizeof(trigdata));
+		trigdata.type = T_TriggerData;
+		fake_fcinfo->context = (Node *) &trigdata;
+	}
+	else if (is_trigger == PLPY_EVENT_TRIGGER)
+	{
+		MemSet(&etrigdata, 0, sizeof(etrigdata));
+		etrigdata.type = T_EventTriggerData;
+		fake_fcinfo->context = (Node *) &etrigdata;
+	}
+
 	/* We can't validate triggers against any particular table ... */
-	(void) PLy_procedure_get(funcoid, InvalidOid, is_trigger);
+	(void) PLy_procedure_get(fake_fcinfo, true);
 
 	PG_RETURN_VOID();
 }
@@ -143,6 +170,7 @@ plpython3_validator(PG_FUNCTION_ARGS)
 Datum
 plpython3_call_handler(PG_FUNCTION_ARGS)
 {
+	PLyProcedureCache *pcache;
 	bool		nonatomic;
 	Datum		retval;
 	PLyExecutionContext *exec_ctx;
@@ -164,9 +192,6 @@ plpython3_call_handler(PG_FUNCTION_ARGS)
 
 	PG_TRY();
 	{
-		Oid			funcoid = fcinfo->flinfo->fn_oid;
-		PLyProcedure *proc;
-
 		/*
 		 * Setup error traceback support for ereport().  Note that the PG_TRY
 		 * structure pops this for us again at exit, so we needn't do that
@@ -178,34 +203,35 @@ plpython3_call_handler(PG_FUNCTION_ARGS)
 		plerrcontext.previous = error_context_stack;
 		error_context_stack = &plerrcontext;
 
+		/*
+		 * Look up (and if necessary compile) the procedure.  This can throw
+		 * an error, so it must happen inside the PG_TRY so that the execution
+		 * context gets popped on the way out.
+		 */
+		pcache = PLy_procedure_get(fcinfo, false);
+		exec_ctx->curr_proc = pcache->proc;
+
 		if (CALLED_AS_TRIGGER(fcinfo))
 		{
-			Relation	tgrel = ((TriggerData *) fcinfo->context)->tg_relation;
 			HeapTuple	trv;
 
-			proc = PLy_procedure_get(funcoid, RelationGetRelid(tgrel), PLPY_TRIGGER);
-			exec_ctx->curr_proc = proc;
-			trv = PLy_exec_trigger(fcinfo, proc);
+			trv = PLy_exec_trigger(fcinfo, pcache->proc);
 			retval = PointerGetDatum(trv);
 		}
 		else if (CALLED_AS_EVENT_TRIGGER(fcinfo))
 		{
-			proc = PLy_procedure_get(funcoid, InvalidOid, PLPY_EVENT_TRIGGER);
-			exec_ctx->curr_proc = proc;
-			PLy_exec_event_trigger(fcinfo, proc);
+			PLy_exec_event_trigger(fcinfo, pcache->proc);
 			retval = (Datum) 0;
 		}
 		else
-		{
-			proc = PLy_procedure_get(funcoid, InvalidOid, PLPY_NOT_TRIGGER);
-			exec_ctx->curr_proc = proc;
-			retval = PLy_exec_function(fcinfo, proc);
-		}
+			retval = PLy_exec_function(fcinfo, pcache);
 	}
 	PG_CATCH();
 	{
+		/* Destroy the execution context */
 		PLy_pop_execution_context();
 		PyErr_Clear();
+
 		PG_RE_THROW();
 	}
 	PG_END_TRY();
@@ -223,6 +249,7 @@ plpython3_inline_handler(PG_FUNCTION_ARGS)
 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
 	FmgrInfo	flinfo;
 	PLyProcedure proc;
+	PLyProcedureCache pcache;
 	PLyExecutionContext *exec_ctx;
 	ErrorContextCallback plerrcontext;
 
@@ -248,6 +275,11 @@ plpython3_inline_handler(PG_FUNCTION_ARGS)
 	 */
 	proc.result.typoid = VOIDOID;
 
+	/* Set up a minimal PLyProcedureCache for the inline block */
+	MemSet(&pcache, 0, sizeof(PLyProcedureCache));
+	pcache.proc = &proc;
+	pcache.fcontext = CurrentMemoryContext;
+
 	/*
 	 * Push execution context onto stack.  It is important that this get
 	 * popped again, so avoid putting anything that could throw error between
@@ -269,7 +301,7 @@ plpython3_inline_handler(PG_FUNCTION_ARGS)
 
 		PLy_procedure_compile(&proc, codeblock->source_text);
 		exec_ctx->curr_proc = &proc;
-		PLy_exec_function(fake_fcinfo, &proc);
+		PLy_exec_function(fake_fcinfo, &pcache);
 	}
 	PG_CATCH();
 	{
@@ -289,6 +321,11 @@ plpython3_inline_handler(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * Determine whether a function is a (DML or event) trigger from its pg_proc
+ * result type.  This is used by the validator, which has no call context to
+ * inspect; the call handler instead relies on the fcinfo's call context.
+ */
 static PLyTrigType
 PLy_procedure_is_trigger(Form_pg_proc procStruct)
 {
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index 750ba586e0c..2a6a5c54809 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -9,33 +9,32 @@
 #include "access/htup_details.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
+#include "commands/event_trigger.h"
+#include "commands/trigger.h"
 #include "funcapi.h"
 #include "plpy_elog.h"
 #include "plpy_main.h"
 #include "plpy_procedure.h"
 #include "plpy_util.h"
 #include "utils/builtins.h"
-#include "utils/hsearch.h"
+#include "utils/funccache.h"
 #include "utils/memutils.h"
+#include "utils/rel.h"
 #include "utils/syscache.h"
 
-static HTAB *PLy_procedure_cache = NULL;
-
-static PLyProcedure *PLy_procedure_create(HeapTuple procTup, Oid fn_oid, PLyTrigType is_trigger);
-static bool PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup);
+static void PLy_procedure_create(PLyProcedure *proc,
+								 HeapTuple procTup,
+								 Oid fn_oid,
+								 PLyTrigType is_trigger);
 static char *PLy_procedure_munge_source(const char *name, const char *src);
-
-
-void
-init_procedure_caches(void)
-{
-	HASHCTL		hash_ctl;
-
-	hash_ctl.keysize = sizeof(PLyProcedureKey);
-	hash_ctl.entrysize = sizeof(PLyProcedureEntry);
-	PLy_procedure_cache = hash_create("PL/Python procedures", 32, &hash_ctl,
-									  HASH_ELEM | HASH_BLOBS);
-}
+static void PLy_compile_callback(FunctionCallInfo fcinfo,
+								 HeapTuple procTup,
+								 const CachedFunctionHashKey *hashkey,
+								 CachedFunction *cfunc,
+								 bool forValidator);
+static void PLy_delete_callback(CachedFunction *cfunc);
+static void RemovePLyProcedureCache(void *arg);
+static void PLy_function_drop_args(PLySavedArgs *savedargs);
 
 /*
  * PLy_procedure_name: get the name of the specified procedure.
@@ -51,103 +50,107 @@ PLy_procedure_name(PLyProcedure *proc)
 }
 
 /*
- * PLy_procedure_get: returns a cached PLyProcedure, or creates, stores and
- * returns a new PLyProcedure.
+ * PLy_procedure_get: returns a cached PLyProcedureCache for the function.
  *
- * fn_oid is the OID of the function requested
- * fn_rel is InvalidOid or the relation this function triggers on
- * is_trigger denotes whether the function is a trigger function
+ * The PLyProcedureCache contains a pointer to the long-lived PLyProcedure
+ * (managed by funccache.c) and execution-specific state like SRF state.
  *
- * The reason that both fn_rel and is_trigger need to be passed is that when
- * trigger functions get validated we don't know which relation(s) they'll
- * be used with, so no sensible fn_rel can be passed.  Also, in that case
- * we can't make a cache entry because we can't construct the right cache key.
- * To forestall leakage of the PLyProcedure in such cases, delete it after
- * construction and return NULL.  That's okay because the only caller that
- * would pass that set of values is plpython3_validator, which ignores our
- * result anyway.
+ * For SRFs, if we are resuming execution (srfstate->iter != NULL), we skip
+ * revalidation and continue using the same PLyProcedure to ensure consistent
+ * behavior throughout the SRF execution.
  */
-PLyProcedure *
-PLy_procedure_get(Oid fn_oid, Oid fn_rel, PLyTrigType is_trigger)
+PLyProcedureCache *
+PLy_procedure_get(FunctionCallInfo fcinfo, bool forValidator)
 {
-	bool		use_cache;
-	HeapTuple	procTup;
-	PLyProcedureKey key;
-	PLyProcedureEntry *volatile entry = NULL;
-	PLyProcedure *volatile proc = NULL;
-	bool		found = false;
-
-	if (is_trigger == PLPY_TRIGGER && fn_rel == InvalidOid)
-		use_cache = false;
-	else
-		use_cache = true;
+	PLyProcedure *proc;
+	PLyProcedureCache *pcache;
+	FmgrInfo   *finfo = fcinfo->flinfo;
+
+	/*
+	 * If this is the first execution for this FmgrInfo, set up a cache struct
+	 * (initially containing null pointers).  The cache must live as long as
+	 * the FmgrInfo, so it goes in fn_mcxt.  Also set up a memory context
+	 * callback that will be invoked when fn_mcxt is deleted.
+	 */
+	pcache = finfo->fn_extra;
+	if (pcache == NULL)
+	{
+		pcache = (PLyProcedureCache *)
+			MemoryContextAllocZero(finfo->fn_mcxt, sizeof(PLyProcedureCache));
 
-	procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid));
-	if (!HeapTupleIsValid(procTup))
-		elog(ERROR, "cache lookup failed for function %u", fn_oid);
+		pcache->fcontext = finfo->fn_mcxt;
+		pcache->mcb.func = RemovePLyProcedureCache;
+		pcache->mcb.arg = pcache;
+
+		MemoryContextRegisterResetCallback(finfo->fn_mcxt, &pcache->mcb);
+
+		finfo->fn_extra = pcache;
+	}
 
 	/*
-	 * Look for the function in the cache, unless we don't have the necessary
-	 * information (e.g. during validation). In that case we just don't cache
-	 * anything.
+	 * If we are resuming execution of a set-returning function, just keep
+	 * using the same cache.  We do not ask funccache.c to re-validate the
+	 * PLyProcedure: we want to run to completion using the function's initial
+	 * definition.
+	 *
+	 * A live iterator (srfstate->iter != NULL) reliably means a genuine
+	 * resume: when an iteration ends for any reason, srfstate->iter is reset
+	 * to NULL.  Normal exhaustion and in-query cancellation (LIMIT, rescan)
+	 * go through PLy_exec_function()/ShutdownPLyFunction(); an error thrown
+	 * during the function's own execution is handled by the PG_CATCH there;
+	 * and an abort that tears down the FmgrInfo runs
+	 * RemovePLyProcedureCache().  So we can't mistake leftover state from an
+	 * interrupted SRF for a resume here.
 	 */
-	if (use_cache)
+	if (pcache->srfstate != NULL && pcache->srfstate->iter != NULL)
 	{
-		key.fn_oid = fn_oid;
-		key.fn_rel = fn_rel;
-		entry = hash_search(PLy_procedure_cache, &key, HASH_ENTER, &found);
-		proc = entry->proc;
+		Assert(pcache->proc != NULL);
+		return pcache;
 	}
 
-	PG_TRY();
+	/*
+	 * Look up, or re-validate, the long-lived hash entry.
+	 */
+	proc = (PLyProcedure *)
+		cached_function_compile(fcinfo,
+								(CachedFunction *) pcache->proc,
+								PLy_compile_callback,
+								PLy_delete_callback,
+								sizeof(PLyProcedure),
+								true,
+								forValidator);
+
+	/*
+	 * Install the hash pointer in the PLyProcedureCache, and increment its
+	 * use count to reflect that.  If cached_function_compile gave us back a
+	 * different hash entry than we were using before, we must decrement that
+	 * one's use count.
+	 */
+	if (proc != pcache->proc)
 	{
-		if (!found)
+		if (pcache->proc != NULL)
 		{
-			/* Haven't found it, create a new procedure */
-			proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
-			if (use_cache)
-				entry->proc = proc;
-			else
-			{
-				/* Delete the proc, otherwise it's a memory leak */
-				PLy_procedure_delete(proc);
-				proc = NULL;
-			}
-		}
-		else if (!PLy_procedure_valid(proc, procTup))
-		{
-			/* Found it, but it's invalid, free and reuse the cache entry */
-			entry->proc = NULL;
-			if (proc)
-				PLy_procedure_delete(proc);
-			proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
-			entry->proc = proc;
+			Assert(pcache->proc->cfunc.use_count > 0);
+			pcache->proc->cfunc.use_count--;
 		}
-		/* Found it and it's valid, it's fine to use it */
+		pcache->proc = proc;
+		proc->cfunc.use_count++;
 	}
-	PG_CATCH();
-	{
-		/* Do not leave an uninitialized entry in the cache */
-		if (use_cache)
-			hash_search(PLy_procedure_cache, &key, HASH_REMOVE, NULL);
-		PG_RE_THROW();
-	}
-	PG_END_TRY();
-
-	ReleaseSysCache(procTup);
 
-	return proc;
+	return pcache;
 }
 
 /*
  * Create a new PLyProcedure structure
  */
-static PLyProcedure *
-PLy_procedure_create(HeapTuple procTup, Oid fn_oid, PLyTrigType is_trigger)
+static void
+PLy_procedure_create(PLyProcedure *proc,
+					 HeapTuple procTup,
+					 Oid fn_oid,
+					 PLyTrigType is_trigger)
 {
 	char		procName[NAMEDATALEN + 256];
 	Form_pg_proc procStruct;
-	PLyProcedure *volatile proc;
 	MemoryContext cxt;
 	MemoryContext oldcxt;
 	int			rv;
@@ -177,7 +180,6 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, PLyTrigType is_trigger)
 
 	oldcxt = MemoryContextSwitchTo(cxt);
 
-	proc = palloc0_object(PLyProcedure);
 	proc->mcxt = cxt;
 
 	PG_TRY();
@@ -191,8 +193,6 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, PLyTrigType is_trigger)
 		proc->proname = pstrdup(NameStr(procStruct->proname));
 		MemoryContextSetIdentifier(cxt, proc->proname);
 		proc->pyname = pstrdup(procName);
-		proc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
-		proc->fn_tid = procTup->t_self;
 		proc->fn_readonly = (procStruct->provolatile != PROVOLATILE_VOLATILE);
 		proc->is_setof = procStruct->proretset;
 		proc->is_procedure = (procStruct->prokind == PROKIND_PROCEDURE);
@@ -355,7 +355,6 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, PLyTrigType is_trigger)
 	PG_END_TRY();
 
 	MemoryContextSwitchTo(oldcxt);
-	return proc;
 }
 
 /*
@@ -424,23 +423,6 @@ PLy_procedure_delete(PLyProcedure *proc)
 	MemoryContextDelete(proc->mcxt);
 }
 
-/*
- * Decide whether a cached PLyProcedure struct is still valid
- */
-static bool
-PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
-{
-	if (proc == NULL)
-		return false;
-
-	/* If the pg_proc tuple has changed, it's not valid */
-	if (!(proc->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
-		  ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
-		return false;
-
-	return true;
-}
-
 static char *
 PLy_procedure_munge_source(const char *name, const char *src)
 {
@@ -485,3 +467,138 @@ PLy_procedure_munge_source(const char *name, const char *src)
 
 	return mrc;
 }
+
+/*
+ * Compile callback for funccache.c.
+ *
+ * cached_function_compile() calls this when it needs to (re)compile the
+ * long-lived PLyProcedure for a function.  The CachedFunction handed to us is
+ * pre-zeroed workspace of size sizeof(PLyProcedure); we just have to fill in
+ * the PL/Python-specific fields.  We derive the trigger type from the call
+ * context, matching what plpython3_call_handler dispatches on.
+ */
+static void
+PLy_compile_callback(FunctionCallInfo fcinfo,
+					 HeapTuple procTup,
+					 const CachedFunctionHashKey *hashkey,
+					 CachedFunction *cfunc,
+					 bool forValidator)
+{
+	PLyProcedure *proc = (PLyProcedure *) cfunc;
+	PLyTrigType is_trigger;
+	Oid			fn_oid = fcinfo->flinfo->fn_oid;
+
+	if (CALLED_AS_TRIGGER(fcinfo))
+		is_trigger = PLPY_TRIGGER;
+	else if (CALLED_AS_EVENT_TRIGGER(fcinfo))
+		is_trigger = PLPY_EVENT_TRIGGER;
+	else
+		is_trigger = PLPY_NOT_TRIGGER;
+
+	PLy_procedure_create(proc, procTup, fn_oid, is_trigger);
+}
+
+/*
+ * Deletion callback for funccache.c.
+ *
+ * cached_function_compile() calls this when it discards a cache entry, which
+ * only happens once the entry's use count has dropped to zero.  We must free
+ * the subsidiary data but not the CachedFunction struct itself.
+ */
+static void
+PLy_delete_callback(CachedFunction *cfunc)
+{
+	PLyProcedure *proc = (PLyProcedure *) cfunc;
+
+	Assert(proc->cfunc.use_count == 0);
+	Assert(proc->calldepth == 0);
+
+	PLy_procedure_delete(proc);
+}
+
+/*
+ * MemoryContext callback function
+ *
+ * We register this in the memory context that contains a PLyProcedureCache
+ * struct (that is, the FmgrInfo's fn_mcxt).  When the memory context is reset
+ * or deleted, we release the reference count (if any) that the cache holds on
+ * the long-lived hash entry.  Note that this will happen even during error
+ * aborts.
+ *
+ * This is also our opportunity to release the Python references held by an
+ * interrupted set-returning function.  ShutdownPLyFunction() handles that for
+ * normal completion and in-query cancellation, but it does not run during an
+ * error abort; this callback does, so it is the backstop that prevents leaking
+ * the SRF's iterator and saved arguments when a query errors out mid-iteration.
+ */
+static void
+RemovePLyProcedureCache(void *arg)
+{
+	PLyProcedureCache *pcache = (PLyProcedureCache *) arg;
+
+	/* Release any Python state left behind by an interrupted SRF */
+	PLy_function_cleanup_srfstate(pcache);
+
+	/* Release reference count on PLyProcedure */
+	if (pcache->proc != NULL)
+	{
+		Assert(pcache->proc->cfunc.use_count > 0);
+		pcache->proc->cfunc.use_count--;
+		pcache->proc = NULL;
+	}
+}
+
+/*
+ * Free a PLySavedArgs struct without restoring the values.
+ */
+static void
+PLy_function_drop_args(PLySavedArgs *savedargs)
+{
+	int			i;
+
+	/* Drop references for named args */
+	for (i = 0; i < savedargs->nargs; i++)
+	{
+		Py_XDECREF(savedargs->namedargs[i]);
+	}
+
+	/* Drop refs to the "args" and "TD" objects, too */
+	Py_XDECREF(savedargs->args);
+	Py_XDECREF(savedargs->td);
+
+	/* And free the PLySavedArgs struct */
+	pfree(savedargs);
+}
+
+/*
+ * Release the Python references held by an in-progress set-returning
+ * function, and free the SRF state.  This is a no-op if there is no active
+ * SRF state, so it's safe to call more than once.
+ *
+ * The Python iterator and the saved argument values own reference counts on
+ * Python objects, which are not released by transaction abort the way backend
+ * memory is.  We must therefore make sure this runs on every exit path: it is
+ * called from the SRF paths in PLy_exec_function() and from its companion
+ * ShutdownPLyFunction() for normal completion and in-query cancellation, and
+ * from RemovePLyProcedureCache() above as the backstop for error aborts.
+ */
+void
+PLy_function_cleanup_srfstate(PLyProcedureCache *pcache)
+{
+	PLySRFState *srfstate = pcache->srfstate;
+
+	if (srfstate != NULL)
+	{
+		/* Release refcount on the iter, if we still have one */
+		Py_XDECREF(srfstate->iter);
+		srfstate->iter = NULL;
+
+		/* And drop any saved args; we won't need them */
+		if (srfstate->savedargs)
+			PLy_function_drop_args(srfstate->savedargs);
+		srfstate->savedargs = NULL;
+
+		pfree(srfstate);
+		pcache->srfstate = NULL;
+	}
+}
diff --git a/src/pl/plpython/plpy_procedure.h b/src/pl/plpython/plpy_procedure.h
index 3ef22844a9b..6cd61a981fa 100644
--- a/src/pl/plpython/plpy_procedure.h
+++ b/src/pl/plpython/plpy_procedure.h
@@ -6,9 +6,7 @@
 #define PLPY_PROCEDURE_H
 
 #include "plpy_typeio.h"
-
-
-extern void init_procedure_caches(void);
+#include "utils/funccache.h"
 
 
 /*
@@ -31,15 +29,28 @@ typedef struct PLySavedArgs
 	PyObject   *namedargs[FLEXIBLE_ARRAY_MEMBER];	/* named args */
 } PLySavedArgs;
 
-/* cached procedure data */
+/* saved state for a set-returning function */
+typedef struct PLySRFState
+{
+	PyObject   *iter;			/* Python iterator producing results */
+	PLySavedArgs *savedargs;	/* function argument values */
+} PLySRFState;
+
+/*
+ * Long-lived data for a PL/Python function.
+ *
+ * This struct is managed by funccache.c and can be shared across multiple
+ * executions of the same function.  It must contain no execution-specific
+ * state.  The CachedFunction struct must be first so we can cast between them.
+ */
 typedef struct PLyProcedure
 {
+	CachedFunction cfunc;		/* fields managed by funccache.c */
+
 	MemoryContext mcxt;			/* context holding this PLyProcedure and its
 								 * subsidiary data */
 	char	   *proname;		/* SQL name of procedure */
 	char	   *pyname;			/* Python name of procedure */
-	TransactionId fn_xmin;
-	ItemPointerData fn_tid;
 	bool		fn_readonly;
 	bool		is_setof;		/* true, if function returns result set */
 	bool		is_procedure;
@@ -59,24 +70,29 @@ typedef struct PLyProcedure
 	PLySavedArgs *argstack;		/* stack of outer-level call arguments */
 } PLyProcedure;
 
-/* the procedure cache key */
-typedef struct PLyProcedureKey
+/*
+ * Per-call-site cache for a PL/Python function.
+ *
+ * This struct is stored in fn_extra and holds execution-specific state,
+ * including a pointer to the long-lived PLyProcedure.  The use_count in
+ * the PLyProcedure is incremented while we hold a reference.
+ */
+typedef struct PLyProcedureCache
 {
-	Oid			fn_oid;			/* function OID */
-	Oid			fn_rel;			/* triggered-on relation or InvalidOid */
-} PLyProcedureKey;
+	PLyProcedure *proc;			/* long-lived hash entry */
+	MemoryContext fcontext;		/* fn_mcxt - context holding this struct */
+	PLySRFState *srfstate;		/* SRF execution state, NULL if not in SRF */
+	bool		shutdown_reg;	/* true if registered shutdown callback */
 
-/* the procedure cache entry */
-typedef struct PLyProcedureEntry
-{
-	PLyProcedureKey key;		/* hash key */
-	PLyProcedure *proc;
-} PLyProcedureEntry;
+	/* Callback to release use-count when fcontext is deleted */
+	MemoryContextCallback mcb;
+} PLyProcedureCache;
 
 /* PLyProcedure manipulation */
 extern char *PLy_procedure_name(PLyProcedure *proc);
-extern PLyProcedure *PLy_procedure_get(Oid fn_oid, Oid fn_rel, PLyTrigType is_trigger);
+extern PLyProcedureCache *PLy_procedure_get(FunctionCallInfo fcinfo, bool forValidator);
 extern void PLy_procedure_compile(PLyProcedure *proc, const char *src);
 extern void PLy_procedure_delete(PLyProcedure *proc);
+extern void PLy_function_cleanup_srfstate(PLyProcedureCache *pcache);
 
 #endif							/* PLPY_PROCEDURE_H */
diff --git a/src/pl/plpython/sql/plpython_setof.sql b/src/pl/plpython/sql/plpython_setof.sql
index 1a0472b7a3b..af73155a713 100644
--- a/src/pl/plpython/sql/plpython_setof.sql
+++ b/src/pl/plpython/sql/plpython_setof.sql
@@ -107,3 +107,16 @@ $$ LANGUAGE plpython3u;
 
 SELECT get_user_records2();
 SELECT * FROM get_user_records2();
+
+-- A set-returning function that is invalidated mid-iteration must run to
+-- completion using its original definition (bug #19480).
+CREATE OR REPLACE FUNCTION self_invalidating_srf(x int) RETURNS SETOF int AS $$
+for i in range(3):
+    if i == 1:
+        plpy.execute("CREATE OR REPLACE FUNCTION self_invalidating_srf(x int) "
+                     "RETURNS SETOF int LANGUAGE plpython3u AS 'return [-1]'")
+    yield x + i
+$$ LANGUAGE plpython3u;
+
+SELECT self_invalidating_srf(10); -- expect 10,11,12 (original definition)
+SELECT self_invalidating_srf(10); -- expect -1 (replacement now in effect)
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f9eb23e52c9..6975aaa25c5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2075,8 +2075,7 @@ PLyObToTuple
 PLyObject_AsString_t
 PLyPlanObject
 PLyProcedure
-PLyProcedureEntry
-PLyProcedureKey
+PLyProcedureCache
 PLyResultObject
 PLySRFState
 PLySavedArgs
-- 
2.50.1 (Apple Git-155)