From 6473d3e1636fb3fbb3f28d118b9b50cd11168d3f Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sat, 21 Mar 2026 11:02:37 -0700 Subject: [PATCH 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 aafc53e016467..3fd3532d452d3 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 493f9b0ee1912..d46c42762c844 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 a7bedb12c1817..41329e8eb24fa 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); From 29595315f01071bd505cf5d1c94855474f5facbe Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sat, 21 Mar 2026 11:30:20 -0700 Subject: [PATCH 2/2] Stop using TopMemoryContext for resource owner related allocations This removes the unnecessary use of TopMemoryContext for objects that need to be accessed during resource cleanup, instead allocating them in the current memory context. This is made possible thanks to the recent refactoring that delayed freeing portal subsidiary memory until after ResourceOwnerRelease has completed. The current memory context is assumed to be a child context of the currently active portal (if any), like the executor context, or another kind of local context that survives until after res owner cleanup. Author: Lukas Fittl Reviewed By: Discussion: --- contrib/pgcrypto/openssl.c | 6 ++---- src/backend/jit/llvm/llvmjit.c | 14 +++++++++----- src/backend/storage/ipc/waiteventset.c | 9 +++++++-- src/common/cryptohash_openssl.c | 7 +++---- src/common/hmac_openssl.c | 7 +++---- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c index d3c12e7fda36a..f01c105dddf1a 100644 --- a/contrib/pgcrypto/openssl.c +++ b/contrib/pgcrypto/openssl.c @@ -176,7 +176,7 @@ px_find_digest(const char *name, PX_MD **res) * The order is crucial, to make sure we don't leak anything on * out-of-memory or other error. */ - digest = MemoryContextAlloc(TopMemoryContext, sizeof(*digest)); + digest = palloc(sizeof(*digest)); ctx = EVP_MD_CTX_create(); if (!ctx) @@ -196,7 +196,6 @@ px_find_digest(const char *name, PX_MD **res) digest->owner = CurrentResourceOwner; ResourceOwnerRememberOSSLDigest(digest->owner, digest); - /* The PX_MD object is allocated in the current memory context. */ h = palloc_object(PX_MD); h->result_size = digest_result_size; h->block_size = digest_block_size; @@ -794,7 +793,7 @@ px_find_cipher(const char *name, PX_Cipher **res) * The order is crucial, to make sure we don't leak anything on * out-of-memory or other error. */ - od = MemoryContextAllocZero(TopMemoryContext, sizeof(*od)); + od = palloc0(sizeof(*od)); od->ciph = i->ciph; /* Allocate an EVP_CIPHER_CTX object. */ @@ -812,7 +811,6 @@ px_find_cipher(const char *name, PX_Cipher **res) if (i->ciph->cipher_func) od->evp_ciph = i->ciph->cipher_func(); - /* The PX_Cipher is allocated in current memory context */ c = palloc_object(PX_Cipher); c->block_size = gen_ossl_block_size; c->key_size = gen_ossl_key_size; diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 2e8aa4749dbc1..acffd7235bbad 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -233,8 +233,7 @@ llvm_create_context(int jitFlags) ResourceOwnerEnlarge(CurrentResourceOwner); - context = MemoryContextAllocZero(TopMemoryContext, - sizeof(LLVMJitContext)); + context = palloc0(sizeof(LLVMJitContext)); context->base.flags = jitFlags; /* ensure cleanup */ @@ -760,8 +759,13 @@ llvm_compile_module(LLVMJitContext *context) pfree(filename); } + /* + * Allocate handle in the same long-lived context as the LLVMJitContext, + * since this can be called during expression evaluation in a short-lived + * per-tuple context. + */ handle = (LLVMJitHandle *) - MemoryContextAlloc(TopMemoryContext, sizeof(LLVMJitHandle)); + MemoryContextAlloc(GetMemoryChunkContext(context), sizeof(LLVMJitHandle)); /* * Emit the code. Note that this can, depending on the optimization @@ -805,8 +809,8 @@ llvm_compile_module(LLVMJitContext *context) context->module = NULL; context->compiled = true; - /* remember emitted code for cleanup and lookups */ - oldcontext = MemoryContextSwitchTo(TopMemoryContext); + /* remember emitted code for cleanup and lookups. */ + oldcontext = MemoryContextSwitchTo(GetMemoryChunkContext(context)); context->handles = lappend(context->handles, handle); MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/storage/ipc/waiteventset.c b/src/backend/storage/ipc/waiteventset.c index 0f228e1e7b86a..414e9805eebad 100644 --- a/src/backend/storage/ipc/waiteventset.c +++ b/src/backend/storage/ipc/waiteventset.c @@ -389,9 +389,14 @@ CreateWaitEventSet(ResourceOwner resowner, int nevents) #endif if (resowner != NULL) + { ResourceOwnerEnlarge(resowner); - - data = (char *) MemoryContextAllocZero(TopMemoryContext, sz); + data = (char *) palloc0(sz); + } + else + { + data = (char *) MemoryContextAllocZero(TopMemoryContext, sz); + } set = (WaitEventSet *) data; data += MAXALIGN(sizeof(WaitEventSet)); diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c index 51b7e0409333d..2373699aa7552 100644 --- a/src/common/cryptohash_openssl.c +++ b/src/common/cryptohash_openssl.c @@ -34,12 +34,11 @@ #endif /* - * In the backend, use an allocation in TopMemoryContext to count for - * resowner cleanup handling. In the frontend, use malloc to be able - * to return a failure status back to the caller. + * In backends, use an allocation in the current memory context. In frontend, + * use malloc to be able to return a failure status back to the caller. */ #ifndef FRONTEND -#define ALLOC(size) MemoryContextAlloc(TopMemoryContext, size) +#define ALLOC(size) palloc(size) #define FREE(ptr) pfree(ptr) #else #define ALLOC(size) malloc(size) diff --git a/src/common/hmac_openssl.c b/src/common/hmac_openssl.c index 7990822854c2b..97a1b0756dd27 100644 --- a/src/common/hmac_openssl.c +++ b/src/common/hmac_openssl.c @@ -34,13 +34,12 @@ #endif /* - * In backend, use an allocation in TopMemoryContext to count for resowner - * cleanup handling if necessary. In frontend, use malloc to be able to return - * a failure status back to the caller. + * In backends, use an allocation in the current memory context. In frontend, + * use malloc to be able to return a failure status back to the caller. */ #ifndef FRONTEND #define USE_RESOWNER_FOR_HMAC -#define ALLOC(size) MemoryContextAlloc(TopMemoryContext, size) +#define ALLOC(size) palloc(size) #define FREE(ptr) pfree(ptr) #else /* FRONTEND */ #define ALLOC(size) malloc(size)