From a67d4847a4319ddee8a8ee7d8945c07301ada66e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 20 Aug 2025 16:09:18 -0400 Subject: Fix re-execution of a failed SQLFunctionCache entry. If we error out during execution of a SQL-language function, we will often leave behind non-null pointers in its SQLFunctionCache's cplan and eslist fields. This is problematic if the SQLFunctionCache is re-used, because those pointers will point at resources that were released during error cleanup. This problem escaped detection so far because ordinarily we won't re-use an FmgrInfo+SQLFunctionCache struct after a query error. However, in the rather improbable case that someone implements an opclass support function in SQL language, there will be long-lived FmgrInfos for it in the relcache, and then the problem is reachable after the function throws an error. To fix, add a flag to SQLFunctionCache that tracks whether execution escapes out of fmgr_sql, and clear out the relevant fields during init_sql_fcache if so. (This is going to need more thought if we ever try to share FMgrInfos across threads; but it's very far from being the only problem such a project will encounter, since many functions regard fn_extra as being query-local state.) This broke at commit 0313c5dc6; before that we did not try to re-use SQLFunctionCache state across calls. Hence, back-patch to v18. Bug: #19026 Reported-by: Alexander Lakhin Author: Tom Lane Discussion: https://postgr.es/m/19026-90aed5e71d0c8af3@postgresql.org Backpatch-through: 18 --- src/backend/executor/functions.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'src/backend/executor/functions.c') diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 359aafea681..97455b1ed4a 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -143,6 +143,7 @@ typedef struct SQLFunctionCache { SQLFunctionHashEntry *func; /* associated SQLFunctionHashEntry */ + bool active; /* are we executing this cache entry? */ bool lazyEvalOK; /* true if lazyEval is safe */ bool shutdown_reg; /* true if registered shutdown callback */ bool lazyEval; /* true if using lazyEval for result query */ @@ -556,6 +557,28 @@ init_sql_fcache(FunctionCallInfo fcinfo, bool lazyEvalOK) finfo->fn_extra = fcache; } + /* + * If the SQLFunctionCache is marked as active, we must have errored out + * of a prior execution. Reset state. (It might seem that we could also + * reach this during recursive invocation of a SQL function, but we won't + * because that case won't involve re-use of the same FmgrInfo.) + */ + if (fcache->active) + { + /* + * In general, this stanza should clear all the same fields that + * ShutdownSQLFunction would. Note we must clear fcache->cplan + * without doing ReleaseCachedPlan, because error cleanup from the + * prior execution would have taken care of releasing that plan. + * Likewise, if tstore is still set then it is pointing at garbage. + */ + fcache->cplan = NULL; + fcache->eslist = NULL; + fcache->tstore = NULL; + fcache->shutdown_reg = false; + fcache->active = false; + } + /* * 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 @@ -1597,6 +1620,9 @@ fmgr_sql(PG_FUNCTION_ARGS) */ fcache = init_sql_fcache(fcinfo, lazyEvalOK); + /* Mark fcache as active */ + fcache->active = true; + /* Remember info that we might need later to construct tuplestore */ fcache->tscontext = tscontext; fcache->randomAccess = randomAccess; @@ -1853,6 +1879,9 @@ fmgr_sql(PG_FUNCTION_ARGS) if (es == NULL) fcache->eslist = NULL; + /* Mark fcache as inactive */ + fcache->active = false; + error_context_stack = sqlerrcontext.previous; return result; -- cgit v1.2.3