From 312fad1c36e064ab9e7dc1780575e8c07f300751 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 8 Apr 2026 18:17:40 +0900 Subject: [PATCH v3 2/4] Fix RI fast-path crash under nested C-level SPI When a C-language function uses SPI_connect/SPI_execute/SPI_finish to INSERT into a table with FK constraints, the FK AFTER triggers register ri_FastPathEndBatch as a batch callback and open PK relations under the SPI portal's resource owner. FireAfterTriggerBatchCallbacks was suppressed at that point by the query_depth > 0 guard, deferring teardown to the outer query's AfterTriggerEndQuery. By then the SPI portal's resource owner had been released, decrementing the cached relations' refcounts to zero. ri_FastPathTeardown then crashed attempting to close them: TRAP: failed Assert("rel->rd_refcnt > 0") Fix by tagging each AfterTriggerCallbackItem with the afterTriggerFiringDepth (added in 5c54c3ed1b9) at registration time and firing only callbacks whose depth matches the current depth. This replaces the query_depth > 0 suppression guard. Callbacks now fire at the same firing depth at which they were registered, while the resource owner that was active during registration is still alive, eliminating the mismatch. While at it, ensure callbacks are properly accounted for at all transaction boundaries, as cleanup of b7b27eb41a5c: assert on commit that no callbacks remain unfired, and discard any remaining callbacks on transaction abort. Also restructure FireAfterTriggerBatchCallbacks() to update afterTriggers.batch_callbacks before invoking any callbacks, so that if a callback throws an ERROR the list is already in a consistent state. Note that ri_PerformCheck() uses fire_triggers=false, which skips AfterTriggerBeginQuery/EndQuery and thus never increments afterTriggerFiringDepth; events queued there fire at the outer query's depth and are unaffected by this change. Reported-by: Evan Montgomery-Recht Author: Evan Montgomery-Recht Co-authored-by: Amit Langote Discussion: https://postgr.es/m/CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com --- src/backend/commands/trigger.c | 54 +++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index c41005ba44e..f59537fe86e 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3935,6 +3935,8 @@ struct AfterTriggersTableData typedef struct AfterTriggerCallbackItem { AfterTriggerBatchCallback callback; + int firing_depth; /* afterTriggerFiringDepth when registered; + * callback fires only at this depth */ void *arg; } AfterTriggerCallbackItem; @@ -5419,6 +5421,15 @@ AfterTriggerEndXact(bool isCommit) afterTriggers.query_depth = -1; afterTriggerFiringDepth = 0; + + Assert(afterTriggers.batch_callbacks == NIL || !isCommit); + + /* On abort, discard any pending callbacks without firing them. */ + if (!isCommit) + { + list_free_deep(afterTriggers.batch_callbacks); + afterTriggers.batch_callbacks = NIL; + } } /* @@ -6830,6 +6841,7 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, oldcxt = MemoryContextSwitchTo(TopTransactionContext); item = palloc(sizeof(AfterTriggerCallbackItem)); item->callback = callback; + item->firing_depth = afterTriggerFiringDepth; item->arg = arg; afterTriggers.batch_callbacks = lappend(afterTriggers.batch_callbacks, item); @@ -6838,31 +6850,51 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback, /* * FireAfterTriggerBatchCallbacks - * Invoke and clear all registered batch callbacks. + * Invoke callbacks registered at the current firing depth. + * + * Each callback is tagged with the afterTriggerFiringDepth at registration + * time. Only callbacks matching the current depth are invoked; the rest + * are retained for when their own depth fires. This ensures that nested + * trigger-firing contexts (e.g., SPI calls inside AFTER triggers) only + * fire the callbacks they registered, leaving outer-level callbacks intact + * until their firing depth is reached. * - * Only fires at the outermost query level (query_depth == 0) or from - * top-level operations (query_depth == -1, e.g. AfterTriggerFireDeferred - * at COMMIT). Nested queries from SPI inside AFTER triggers run at - * depth > 0 and must not tear down resources the outer batch still needs. + * The list is updated before any callbacks are invoked so that if a + * callback throws an ERROR the list is already in a consistent state. */ static void FireAfterTriggerBatchCallbacks(void) { + List *remaining = NIL; + List *to_fire = NIL; ListCell *lc; - if (afterTriggers.query_depth > 0) - return; + /* remaining and to_fire lists must survive until callbacks complete */ + MemoryContext oldcxt = MemoryContextSwitchTo(TopTransactionContext); - Assert(afterTriggerFiringDepth > 0); foreach(lc, afterTriggers.batch_callbacks) { AfterTriggerCallbackItem *item = lfirst(lc); - item->callback(item->arg); + if (item->firing_depth == afterTriggerFiringDepth) + to_fire = lappend(to_fire, item); + else + remaining = lappend(remaining, item); } - list_free_deep(afterTriggers.batch_callbacks); - afterTriggers.batch_callbacks = NIL; + list_free(afterTriggers.batch_callbacks); + afterTriggers.batch_callbacks = remaining; + MemoryContextSwitchTo(oldcxt); + + /* Now fire; if one throws, the list is already clean */ + foreach(lc, to_fire) + { + AfterTriggerCallbackItem *item = lfirst(lc); + + item->callback(item->arg); + pfree(item); + } + list_free(to_fire); } /* -- 2.47.3