public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
To: Lukas Fittl <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Zsolt Parragi <[email protected]>
Subject: Re: Avoid use of TopMemoryContext for resource owner cleanup in portals
Date: Sun, 22 Mar 2026 14:39:58 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAP53PkxfZu+WgeAC+1BybrS6asH+S_D1Nk5onq7Js5YDFZraJQ@mail.gmail.com>
References: <CAP53PkxfZu+WgeAC+1BybrS6asH+S_D1Nk5onq7Js5YDFZraJQ@mail.gmail.com>
On 21/03/2026 22:18, Lukas Fittl wrote:
> Hi,
>
> Whilst working on the stack-based instrumentation patch [0] which adds
> a new resource owner that gets set up during a query's execution, I
> encountered the following challenge:
>
> When allocating memory related to a resource, that is intended to be
> accessed during resource owner cleanup on abort, one cannot use a
> memory context that's below the active portal (e.g. the executor
> context), but must instead chose a longer-lived context, often
> TopMemoryContext.
Yeah, resources managed by resource owners have their own lifecycle
that's independent of memory contexts.
> This is caused by the fact that At(Sub)Abort_Portals currently frees
> all "subsidiary" memory of failed portals (i.e. failed portal memory
> child contexts), and will be called in Abort(Sub)Transaction before
> the ResourceOwnerRelease calls.
>
> There appears to be no clear reason why the freeing of subsidiary
> portal memory is being done before resource owner release - one could
> argue that freeing memory earlier allows a later allocation to re-use
> it, but the only relevant case I could find was
> RecordTransactionAbort, and that is already handled with the
> pre-allocated TransactionAbortContext to make sure we don't fail
> allocations in out-of-memory aborts.
>
> Other non-portal users of the ResOwner infrastructure don't suffer
> from this problem, as they typically have a memory context set up that
> survives the abort.
>
> If we separate out the freeing of this subsidiary portal memory to run
> separately, after resource owner cleanup is done (0001 patch), we can
> remove a handful of uses of TopMemoryContext from the tree in LLVM
> JIT, WaitEventSet and OpenSSL (0002 patch, passes CI), and make it
> much less likely that new resource owner code accidentally leaks
> because it uses the top memory context and missed a pfree.
It also makes it much more likely that you crash if you release the
resource owner only after deleting the memory context. I don't think
this is a good idea. I think it's a good rule that anything that's
needed for resource owner cleanup must be allocated in TopMemoryContext,
so that there's no hidden dependency between resource owners and memory
contexts and you can release them in any order. (I'm not 100% sure we
follow that rule everywhere today, but still)
> It also happens to make things significantly easier for the
> stack-based instrumentation patch, since we could rely on the executor
> context to free memory allocations that need to be accessed during
> abort (to propagate instrumentation data up the stack).
Hmm, I'm not sure I follow. Do you plan to rely on resource owner
cleanup to propagate the instrumentation data? That doesn't feel right
to me.
- Heikki
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Avoid use of TopMemoryContext for resource owner cleanup in portals
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox