[PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

Started by Bryan Green15 days ago7 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253356
psql -h localhost -U postgres

Built from patchset v6 (message #6), August 23, 2026 at 05:27 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253356_6 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253356_6 && git checkout t253356_6

Patchset v6 (message #6) is on t253356_6

Jump to latest
#1Bryan Green
dbryan.green@gmail.com

Greetings,

The subtransaction commit path can corrupt pg_xact, and the offending code
is all stock.  A subtransaction that has already subcommitted can still be
forced to abort, and when that happens its XID is left in the parent's list
of committed children while pg_xact records it as aborted.  When the parent
commits, TransactionIdSetTreeStatus() tries to mark that aborted XID
committed.  With assertions on, that's a TRAP in clog.c; with them off, it
writes the wrong status, and since the parent's commit record carries the
same child list, replay hits it too and recovery never finishes.

The window is in CommitSubTransaction():

    s->state = TRANS_COMMIT;
    ...
    if (FullTransactionIdIsValid(s->fullTransactionId))
        AtSubCommit_childXids();
    AfterTriggerEndSubXact(true);
    AtSubCommit_Portals(...);
    ...

By the time AfterTriggerEndSubXact() runs, AtSubCommit_childXids() has
already copied our XID and any committed grandchildren into the parent's
array.  If one of the later steps throws (OOM being the obvious case),
control longjmps into AbortSubTransaction() with the subtransaction still in
TRANS_COMMIT state.  We record the XID aborted and leave it sitting in the
parent's list; AtSubAbort_childXids() only frees our own array, not the
parent's.

An error thrown at a subtransaction's commit inside a PL/pgSQL EXCEPTION 
block is caught right there, so the subtransaction aborts while the 
surrounding transaction goes on to commit.  That is the shape that bites.  
A subtransaction-commit callback that raises on 
SUBXACT_EVENT_COMMIT_SUB reproduces it with no core changes and no 
injection points; an injection point just after AtSubCommit_childXids() 
does too:

    BEGIN;
    DO $$
    BEGIN
        BEGIN
            INSERT INTO t VALUES (1);   -- subtransaction acquires an XID
        EXCEPTION WHEN OTHERS THEN
            NULL;                       -- swallow the commit-time error
        END;
    END $$;
    COMMIT;                             -- crashes here

    TRAP: failed Assert("curval == 0 || ... || curval == status"),
          File: "clog.c", Line: 702
        TransactionIdSetStatusBit
        TransactionIdSetTreeStatus
        TransactionIdCommitTree
        RecordTransactionCommit

The fix is small and stays in xact.c.  AtSubCommit_childXids() records the
parent's child count before it appends, and AbortSubTransaction() restores
that count when the subtransaction aborts after the transfer.  The entries
we added are the tail of the parent's array, so restoring the length drops
exactly them; the grandchildren revert to aborted, which is correct because
the whole subtree is rolling back.

I confirmed on current master that the reproduction crashes without the
patch and commits cleanly with it, that the aborted row is gone, that a
committed sibling savepoint survives, and that the regression tests pass.

--
Bryan Green
EDB: https://www.enterprisedb.com

Attachments:

t253356_1
0001-Fix-pg_xact-corruption-from-subtransaction-abort-aft.patchtext/plain; charset=UTF-8; name=0001-Fix-pg_xact-corruption-from-subtransaction-abort-aft.patchDownload+28-1
#2Andrey Borodin
amborodin@acm.org
In reply to: Bryan Green (#1)
Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

Hi Bryan,

Thank you for finding this.

On 9 Aug 2026, at 08:39, Bryan Green <dbryan.green@gmail.com> wrote:

path can corrupt pg_xact

That's a very interesting corruption case. We observed something very much like
this with "failed to find parent tuple for heap-only tuple".

But so far I can't build a test with an error between

AtSubCommit_childXids();
and
PopTransaction();

I meant, yeah, extensions can do this, but that's a weak argument. Maybe some
allocations inside AtSubCommit_Notify() can cause catachable error... IDK.

This actually looks like a corruption that bite us. But how an error can happen
in this path? If it's an extension, which extension is thorwin it? In-tree
extensions seems to avoid erroring out in callbacks. Maybe something in
AtEOXact_GUC()?

Best regards, Andrey Borodin.

#3Andrey Rachitskiy
pl0h0yp1@gmail.com
In reply to: Andrey Borodin (#2)
Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

Hi, Andrey!

On whether a catchable ERROR can happen after AtSubCommit_childXids()
without an extension callback: yes. Allocations inside stock
AtSubCommit_Notify() are enough to raise one. That reaches Bryan's
steps (A) and (B) below. It does not by itself give a neat clog TRAP on
the later parent COMMIT. Under this OOM setup that COMMIT usually dies
in SIGSEGV first. The childXids / pg_xact split is already present
before COMMIT, which is the corruption window Bryan described.

Bryan's sequence is:

(A) AtSubCommit_childXids() publishes the subxid into parent->childXids.
(B) A later catchable ERROR aborts the subxact and marks that XID aborted
in pg_xact, without removing it from parent->childXids.
(C) Parent COMMIT then calls TransactionIdSetTreeStatus() / CommitTree
over that mixed list (assert TRAP, or silent clog corruption).

(A)+(B) are visible under gdb with no extension. For a clean (C) with
the clog assert, an extension COMMIT_SUB callback that raises ERROR
still works as Bryan sketched.

Setup in short: parent and subxact both queue NOTIFYs, then force
memory pressure so the pending-notify merge in AtSubCommit_Notify()
hits a catchable out-of-memory ERROR; PL/pgSQL EXCEPTION WHEN OTHERS
swallows it.

Observed under gdb:

1) (A) Subxid published into the parent list, still before Notify merge:

AtSubCommit_childXids ENTER xid=696 parent->nChildXids=0
after AtSubCommit_childXids: parent->nChildXids=1 childXids[0]=696
AtSubCommit_Notify ENTER parent->nChildXids=1 childXids[0]=696

2) Catchable OOM later in the same CommitSubTransaction():

#0 MemoryContextAllocationFailure (...) at mcxt.c:1203
#6 AddEventToPendingNotifies (...) at async.c:3228
#7 AtSubCommit_Notify () at async.c:2498
#8 CommitSubTransaction () at xact.c:5207
#9 ReleaseCurrentSubTransaction () at xact.c:4837
#10 exec_stmt_block (...) at pl_exec.c:1859
...

3) (B) Abort marks the same XID aborted in pg_xact while it remains in
parent->childXids (AbortSubTransaction -> RecordTransactionAbort ->
TransactionIdAbortTree; AtSubAbort_childXids only frees *this* subxact's
own child list, not the parent's entry):

TransactionIdAbortTree xid=696 nchildren=0

==== BUG STATE after CleanupSubTransaction ====
parent->nChildXids = 1
parent->childXids[0] = 696 DidAbort=1 DidCommit=0
==== end BUG STATE ====

вс, 9 авг. 2026 г. в 22:57, Andrey Borodin <x4mmm@yandex-team.ru>:

Hi Bryan,

Thank you for finding this.

On 9 Aug 2026, at 08:39, Bryan Green <dbryan.green@gmail.com> wrote:

path can corrupt pg_xact

That's a very interesting corruption case. We observed something very much
like
this with "failed to find parent tuple for heap-only tuple".

But so far I can't build a test with an error between

AtSubCommit_childXids();
and
PopTransaction();

I meant, yeah, extensions can do this, but that's a weak argument. Maybe
some
allocations inside AtSubCommit_Notify() can cause catachable error... IDK.

This actually looks like a corruption that bite us. But how an error can
happen
in this path? If it's an extension, which extension is thorwin it? In-tree
extensions seems to avoid erroring out in callbacks. Maybe something in
AtEOXact_GUC()?

Best regards, Andrey Borodin.

--
Regards,
Rachitskiy Andrey

#4Jonathan Gonzalez V.
jonathan.abdiel@gmail.com
In reply to: Bryan Green (#1)
Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

Hello!

Bryan Green <dbryan.green@gmail.com> writes:

...
    BEGIN;
    DO $$
    BEGIN
        BEGIN
            INSERT INTO t VALUES (1);   -- subtransaction acquires an XID
        EXCEPTION WHEN OTHERS THEN
            NULL;                       -- swallow the commit-time error
        END;
    END $$;
    COMMIT;                             -- crashes here

    TRAP: failed Assert("curval == 0 || ... || curval == status"),
          File: "clog.c", Line: 702
        TransactionIdSetStatusBit
        TransactionIdSetTreeStatus
        TransactionIdCommitTree
        RecordTransactionCommit

The fix is small and stays in xact.c.  AtSubCommit_childXids() records the
parent's child count before it appends, and AbortSubTransaction() restores
that count when the subtransaction aborts after the transfer.  The entries
we added are the tail of the parent's array, so restoring the length drops
exactly them; the grandchildren revert to aborted, which is correct because
the whole subtree is rolling back.

I confirmed on current master that the reproduction crashes without the
patch and commits cleanly with it, that the aborted row is gone, that a
committed sibling savepoint survives, and that the regression tests pass.

Probably this requires to have this test in the patch, reproducing this
issue it's not easy. Using a test that use the callback or an injection
point, don't know if this it's expects to be backported, if that's not
the case probably an injection point plus a regression will be enough.

Related to the code, you added this:

@@ -250,6 +252,7 @@ static TransactionStateData TopTransactionStateData = {
.state = TRANS_DEFAULT,
.blockState = TBLOCK_DEFAULT,
.topXidLogged = false,
+ .savedParentNChildXids = -1,
};

Why not initialize in StartTransaction() alongside with the other
transaction states ?

I was able to confirm this error, but it requires a test that targets
this specific case, that's why I think that the test should be included
here.

Regards,
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com

#5Andrey Borodin
amborodin@acm.org
In reply to: Jonathan Gonzalez V. (#4)
Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

On 12 Aug 2026, at 16:36, Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> wrote:

Probably this requires to have this test in the patch, reproducing this
issue it's not easy. Using a test that use the callback or an injection
point, don't know if this it's expects to be backported, if that's not
the case probably an injection point plus a regression will be enough.

It's relatively easy with injection point generating an error in the window. PFA.
However it was not clear to me that error in this window is possible.
I did not dig into Andrey Rachitskiy's analysis yet.

Best regards, Andrey Borodin.

Attachments:

t253356_5
0001-Demonstrate-the-data-corruption-from-a-subtransactio.patchapplication/octet-stream; name=0001-Demonstrate-the-data-corruption-from-a-subtransactio.patch; x-unix-mode=0644Download+162-1
#6Andrey Rachitskiy
pl0h0yp1@gmail.com
In reply to: Andrey Borodin (#5)
Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

Hi, Andrey, Jonathan, Bryan!

v2 addresses the comments: savedParentNChildXids is initialized in
StartTransaction() alongside the other childXids fields (and still in
PushTransaction()).

The attached test uses a SUBXACT_EVENT_COMMIT_SUB callback so it does not
need --enable-injection-points and can be backported. The TAP case adapts
Andrey Borodin's demonstration to that callback.

ср, 12 авг. 2026 г. в 19:39, Andrey Borodin <x4mmm@yandex-team.ru>:

On 12 Aug 2026, at 16:36, Jonathan Gonzalez V. <

jonathan.abdiel@gmail.com> wrote:

Probably this requires to have this test in the patch, reproducing this
issue it's not easy. Using a test that use the callback or an injection
point, don't know if this it's expects to be backported, if that's not
the case probably an injection point plus a regression will be enough.

It's relatively easy with injection point generating an error in the
window. PFA.
However it was not clear to me that error in this window is possible.
I did not dig into Andrey Rachitskiy's analysis yet.

Best regards, Andrey Borodin.

--
Regards,
Rachitskiy Andrey

Attachments:

t253356_6
v2-callback-0001-Fix-pg_xact-corruption-from-subtransaction-abort-after-subcommit.patchtext/x-patch; charset=US-ASCII; name=v2-callback-0001-Fix-pg_xact-corruption-from-subtransaction-abort-after-subcommit.patchDownload+321-1
#7Andrey Borodin
amborodin@acm.org
In reply to: Andrey Rachitskiy (#6)
Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit

On 12 Aug 2026, at 19:23, Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:

The TAP case adapts
Andrey Borodin's demonstration to that callback.

The injection-point test was not intended as a committable test. I
wanted to make the resulting corruption directly observable. On its
own it was not entirely convincing, because it injected an ERROR
without showing that stock code could raise one there.

As I understand your OOM analysis, AtSubCommit_Notify() provides a real,
if probably rare, path to a catchable ERROR after
AtSubCommit_childXids(). A one-off allocation failure does not
necessarily imply that the subsequent abort and parent commit must also
fail.

Taken together, your analysis establishes that the window is reachable,
while the injection-point test demonstrates the corruption resulting
from an ERROR in that window. So yes, this appears to be a real
corruption scenario.

Thank you!

Best regards, Andrey Borodin.