From 6473d3e1636fb3fbb3f28d118b9b50cd11168d3f Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sat, 21 Mar 2026 11:02:37 -0700 Subject: [PATCH v1 1/2] Allow resource owners to access subsidiary portal memory during cleanup During AbortTransaction/AbortSubTransaction we free portal child memory contexts, aka subsidiary contexts, ahead of freeing the portal memory itself, in order to not keep short-lived contexts like the executor context around longer than needed, e.g. in the case of holdable cursors. However, we currently free such memory when we first mark the portal as aborted, and before calling ResourceOwnerRelease. This presents a challenge for any resource owner that wants to access memory that was allocated inside the portal (e.g. with the executor context) and is registered to a resource owner that wants to use it during clean up. The current workaround is to allocate such memory in the TopMemoryContext, and use precise pfree(..) calls in both abort and success cases to avoid leaks. Instead, separate marking portals as aborted (and calling the cleanup hook) from releasing subsidiary memory contexts. Delay releasing the memory contexts until ResourceOwnerRelease has been called for all phases. For example, this allows allocating memory in the per-resource context and then accessing it when cleaning up a resource. Since abort handling is not supposed to allocate new memory (the pre-allocated TransactionAbortContext exists for that purpose), this should have limited performance impact in practice, and will allow both potential refactorings of current resource owners, and avoid complexity in future patches. Author: Lukas Fittl Reviewed By: Discussion: --- src/backend/access/transam/xact.c | 24 ++++++++++++ src/backend/utils/mmgr/portalmem.c | 60 ++++++++++++++++++++++++++++-- src/include/utils/portal.h | 2 + 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index aafc53e0164..3fd3532d452 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3000,6 +3000,15 @@ AbortTransaction(void) ResourceOwnerRelease(TopTransactionResourceOwner, RESOURCE_RELEASE_AFTER_LOCKS, false, true); + + /* + * Now that resource owner cleanup has finished, we can release the + * memory resource owners might have still accessed during cleanup. + * + * Note the portal context itself is freed later in PortalDrop. + */ + AtAbort_Portals_ReleaseMemory(); + smgrDoPendingDeletes(false); AtEOXact_GUC(false, 1); @@ -3017,6 +3026,10 @@ AbortTransaction(void) AtEOXact_LogicalCtl(); pgstat_report_xact_timestamp(0); } + else + { + AtAbort_Portals_ReleaseMemory(); + } /* * State remains TRANS_ABORT until CleanupTransaction(). @@ -4939,6 +4952,7 @@ AbortOutOfAnyTransaction(void) * we need to shut down before doing CleanupTransaction. */ AtAbort_Portals(); + AtAbort_Portals_ReleaseMemory(); CleanupTransaction(); s->blockState = TBLOCK_DEFAULT; break; @@ -4968,6 +4982,7 @@ AbortOutOfAnyTransaction(void) s->parent->subTransactionId, s->curTransactionOwner, s->parent->curTransactionOwner); + AtSubAbort_Portals_ReleaseMemory(s->subTransactionId); } CleanupSubTransaction(); s = CurrentTransactionState; /* changed by pop */ @@ -5371,6 +5386,15 @@ AbortSubTransaction(void) ResourceOwnerRelease(s->curTransactionOwner, RESOURCE_RELEASE_AFTER_LOCKS, false, false); + + /* + * Now that resource owner cleanup has finished, we can release the + * memory resource owners might have still accessed during cleanup. + * + * Note the portal context itself is freed later in PortalDrop. + */ + AtSubAbort_Portals_ReleaseMemory(s->subTransactionId); + AtSubAbort_smgr(); AtEOXact_GUC(false, s->gucNestLevel); diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index 493f9b0ee19..d46c42762c8 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -839,12 +839,40 @@ AtAbort_Portals(void) * PortalDrop. */ portal->resowner = NULL; + } +} + +/* + * Release subsidiary memory for aborted portals. + * + * This is called after ResourceOwnerRelease, so that any resources still + * referencing portal memory have been cleaned up first. + */ +void +AtAbort_Portals_ReleaseMemory(void) +{ + HASH_SEQ_STATUS status; + PortalHashEnt *hentry; + + hash_seq_init(&status, PortalHashTable); + + while ((hentry = (PortalHashEnt *) hash_seq_search(&status)) != NULL) + { + Portal portal = hentry->portal; + + /* Do nothing to cursors held over from a previous transaction. */ + if (portal->createSubid == InvalidSubTransactionId) + continue; + + /* Do nothing to auto-held cursors. */ + if (portal->autoHeld) + continue; /* * Although we can't delete the portal data structure proper, we can * release any memory in subsidiary contexts, such as executor state. - * The cleanup hook was the last thing that might have needed data - * there. But leave active portals alone. + * The portal cleanup hook or ResourceOwnerRelease were the last thing + * that might have needed data there. But leave active portals alone. */ if (portal->status != PORTAL_ACTIVE) MemoryContextDeleteChildren(portal->portalContext); @@ -1074,11 +1102,35 @@ AtSubAbort_Portals(SubTransactionId mySubid, */ portal->resowner = NULL; + } +} + +/* + * Release subsidiary memory for portals aborted in a subtransaction. + * + * This is called after ResourceOwnerRelease, so that any resources still + * referencing portal memory have been cleaned up first. + */ +void +AtSubAbort_Portals_ReleaseMemory(SubTransactionId mySubid) +{ + HASH_SEQ_STATUS status; + PortalHashEnt *hentry; + + hash_seq_init(&status, PortalHashTable); + + while ((hentry = (PortalHashEnt *) hash_seq_search(&status)) != NULL) + { + Portal portal = hentry->portal; + + if (portal->createSubid != mySubid) + continue; + /* * Although we can't delete the portal data structure proper, we can * release any memory in subsidiary contexts, such as executor state. - * The cleanup hook was the last thing that might have needed data - * there. + * The portal cleanup hook or ResourceOwnerRelease were the last thing + * that might have needed data there. */ MemoryContextDeleteChildren(portal->portalContext); } diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h index a7bedb12c18..41329e8eb24 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -215,6 +215,7 @@ typedef struct PortalData extern void EnablePortalManager(void); extern bool PreCommit_Portals(bool isPrepare); extern void AtAbort_Portals(void); +extern void AtAbort_Portals_ReleaseMemory(void); extern void AtCleanup_Portals(void); extern void PortalErrorCleanup(void); extern void AtSubCommit_Portals(SubTransactionId mySubid, @@ -225,6 +226,7 @@ extern void AtSubAbort_Portals(SubTransactionId mySubid, SubTransactionId parentSubid, ResourceOwner myXactOwner, ResourceOwner parentXactOwner); +extern void AtSubAbort_Portals_ReleaseMemory(SubTransactionId mySubid); extern void AtSubCleanup_Portals(SubTransactionId mySubid); extern Portal CreatePortal(const char *name, bool allowDup, bool dupSilent); extern Portal CreateNewPortal(void); -- 2.47.1