Re: Unexpected changes of CurrentResourceOwner and CurrentMemoryContext - Mailing list pgsql-hackers

From Antonin Houska
Subject Re: Unexpected changes of CurrentResourceOwner and CurrentMemoryContext
Date
Msg-id 6242.1757659578@localhost
Whole thread Raw
In response to Re: Unexpected changes of CurrentResourceOwner and CurrentMemoryContext  ("Euler Taveira" <euler@eulerto.com>)
List pgsql-hackers
Euler Taveira <euler@eulerto.com> wrote:

> On Thu, Sep 11, 2025, at 3:05 PM, Álvaro Herrera wrote:
> > On 2025-Sep-03, Antonin Houska wrote:
> >
> >> When working on the REPACK command, we see an ERROR caused by unexpected
> >> change of CurrentResourceOwner [1]. I think the problem is that
> >> reorderbuffer.c does not restore the original value after calling
> >> RollbackAndReleaseCurrentSubTransaction(). The attached patch tries to handle
> >> the call like other callers throughout the tree do.
> >
>
> Interesting. I'm wondering that if this patch is applied we could remove the
> following code
>
>         /*
>          * Logical decoding could have clobbered CurrentResourceOwner during
>          * transaction management, so restore the executor's value.  (This is
>          * a kluge, but it's not worth cleaning up right now.)
>          */
>         CurrentResourceOwner = old_resowner;
>
> from pg_logical_slot_get_changes_guts and LogicalSlotAdvanceAndCheckSnapState
> functions too. IIUC the referred code is a band-aid that will be improved
> someday.

Even though we're fixing the likely reason of this problem, we cannot be 100%
sure that no other problem like this still exists. So I'd not remove this
assignment. Maybe add Assert(CurrentResourceOwner == old_resowner) in front of
that, and adjust the comment?

> > I have registered this as
> > https://commitfest.postgresql.org/patch/6051/
> >
> > I've been wondering whether this should be backpatched.  In principle
> > this is a bugfix, so it should, but I don't offhand recall any cases
> > where failure to set the current context/resowner in the other
> > reorderbuffer.c users causes a live bug, so ... maybe master only?  I'm
> > wondering if it's possible where anybody _depends_ on the current
> > behavior, but I suppose that's quite unlikely.
> >
>
> I would say apply it to master only. If/when we have a bug report we can
> backpatch it.

+1

> Per the crash description, I'm not sure we can create a
> reproducible test case with the current supported commands. Am I wrong?

It seems so, at least with he "CurrentResourceOwner = old_resowner" assignment
in place. REPACK CONCURRENTLY exposes the problem a bit more because it has at
least one kind of resource open during logical decoding: relation.

--
Antonin Houska
Web: https://www.cybertec-postgresql.com



pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: [PATCH] jit: fix build with LLVM-21
Next
From: Laurenz Albe
Date:
Subject: Re: Set log_lock_waits=on by default