Hi, hackers!

I think I found an issue in two-phase commit code, I've attached patch with reproducing, pls take a look(master rev 9f4bd91a196).
I've got coredump with backtrace like this

#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:44 #1  __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78 #2  __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ./nptl/pthread_kill.c:89 #3  0x00007de7ef24527e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007de7ef2288ff in __GI_abort () at ./stdlib/abort.c:79
#5  0x0000610d848485bb in ExceptionalCondition (conditionName=0x610d84910812 "XLogRecPtrIsValid(RecPtr)", fileName=0x610d849107c2 "xlogreader.c", lineNumber=240) at assert.c:65 #6  0x0000610d840b0926 in XLogBeginRead (state=0x610d948b06a8, RecPtr=0) at xlogreader.c:240 #7  0x0000610d8408ba9a in XlogReadTwoPhaseData (lsn=0, buf=0x7fff8c941248, len=0x0) at twophase.c:1437 #8  0x0000610d8408be05 in FinishPreparedTransaction (gid=0x610d947e6698 "test_transaction", isCommit=true) at twophase.c:1544 #9  0x0000610d8462b70e in standard_ProcessUtility (pstmt=0x610d947e6798, queryString=0x610d947e5cb0 "COMMIT PREPARED 'test_transaction';", readOnlyTree=false,     context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x610d947e6b58, qc=0x7fff8c941730) at utility.c:655 #10 0x0000610d8462ade6 in ProcessUtility (pstmt=0x610d947e6798, queryString=0x610d947e5cb0 "COMMIT PREPARED 'test_transaction';", readOnlyTree=false,     context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x610d947e6b58, qc=0x7fff8c941730) at utility.c:525 #11 0x0000610d846293ee in PortalRunUtility (portal=0x610d94864a20, pstmt=0x610d947e6798, isTopLevel=true, setHoldSnapshot=false, dest=0x610d947e6b58, qc=0x7fff8c941730)
    at pquery.c:1149
#12 0x0000610d84629668 in PortalRunMulti (portal=0x610d94864a20, isTopLevel=true, setHoldSnapshot=false, dest=0x610d947e6b58, altdest=0x610d947e6b58, qc=0x7fff8c941730)
    at pquery.c:1307
#13 0x0000610d84628ac0 in PortalRun (portal=0x610d94864a20, count=9223372036854775807, isTopLevel=true, dest=0x610d947e6b58, altdest=0x610d947e6b58, qc=0x7fff8c941730) at pquery.c:784 #14 0x0000610d84620b10 in exec_simple_query (query_string=0x610d947e5cb0 "COMMIT PREPARED 'test_transaction';") at postgres.c:1297
....


It seems we have race between prepared transaction commit and checkpointer and it is old problem. We need TwoPhaseStateLock wrap the reading state file in FinishPreparedTransaction code, the draft patch is attached too, hope it will be helpful.


Best regards,
Maksim Melnikov
From 69a925c3a04d8b3dcab109c3dcfbecabec96004e Mon Sep 17 00:00:00 2001
From: Maksim Melnikov <[email protected]>
Date: Thu, 10 Sep 2026 19:37:16 +0300
Subject: [PATCH] Fixing race between prepared transaction commit and
 checkpointer.

---
 src/backend/access/transam/twophase.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 48e478a4ecb..d6e52f6d814 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -561,15 +561,7 @@ LockGXact(const char *gid, Oid user)
 {
 	int			i;
 
-	/* on first call, register the exit hook */
-	if (!twophaseExitRegistered)
-	{
-		before_shmem_exit(AtProcExit_Twophase, 0);
-		twophaseExitRegistered = true;
-	}
-
-	LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
-
+	Assert(LWLockHeldByMeInMode(TwoPhaseStateLock, LW_EXCLUSIVE));
 	for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
 	{
 		GlobalTransaction gxact = TwoPhaseState->prepXacts[i];
@@ -610,11 +602,10 @@ LockGXact(const char *gid, Oid user)
 		gxact->locking_backend = MyProcNumber;
 		MyLockedGxact = gxact;
 
-		LWLockRelease(TwoPhaseStateLock);
-
 		return gxact;
 	}
 
+	/* Release acquired lock before error, lock is held check placed on method header */
 	LWLockRelease(TwoPhaseStateLock);
 
 	ereport(ERROR,
@@ -1522,6 +1513,15 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	xl_xact_stats_item *abortstats;
 	SharedInvalidationMessage *invalmsgs;
 
+	/* on first call, register the exit hook */
+	if (!twophaseExitRegistered)
+	{
+		before_shmem_exit(AtProcExit_Twophase, 0);
+		twophaseExitRegistered = true;
+	}
+
+	LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE);
+
 	/*
 	 * Validate the GID, and lock the GXACT to ensure that two backends do not
 	 * try to commit the same GID at once.
@@ -1541,6 +1541,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	else
 		XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, NULL);
 
+	LWLockRelease(TwoPhaseStateLock);
 
 	/*
 	 * Disassemble the header area
-- 
2.43.0

diff --git a/src/backend/access/transam/twophase.c 
b/src/backend/access/transam/twophase.c
index 48e478a4ecb..0ec274d887e 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1539,7 +1539,10 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
        if (gxact->ondisk)
                buf = ReadTwoPhaseFile(fxid, false);
        else
+       {
+               INJECTION_POINT("wait-checkpointer-deals", NULL);
                XlogReadTwoPhaseData(gxact->prepare_start_lsn, &buf, NULL);
+       }
 
 
        /*
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 71a2e65ad70..79e0630c9af 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -54,6 +54,7 @@ SUBDIRS = \
                  test_shm_mq \
                  test_slru \
                  test_tidstore \
+                 test_two_phase \
                  test_wait_lsn \
                  unsafe_tests \
                  worker_spi \
diff --git a/src/test/modules/test_two_phase/Makefile 
b/src/test/modules/test_two_phase/Makefile
new file mode 100644
index 00000000000..f2757b4e9a0
--- /dev/null
+++ b/src/test/modules/test_two_phase/Makefile
@@ -0,0 +1,17 @@
+# src/test/modules/test_two_phase/Makefile
+
+TAP_TESTS = 1
+
+EXTRA_INSTALL=src/test/modules/injection_points
+export enable_injection_points
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_two_phase
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_two_phase/t/001_checkpoint.pl 
b/src/test/modules/test_two_phase/t/001_checkpoint.pl
new file mode 100644
index 00000000000..741c8f8e154
--- /dev/null
+++ b/src/test/modules/test_two_phase/t/001_checkpoint.pl
@@ -0,0 +1,64 @@
+# Simple tests for the reproducing race between prepared transaction commit 
and checkpointer.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+       plan skip_all => 'Injection points not supported by this build';
+}
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf('postgresql.conf', qq(
+       max_prepared_transactions = 10
+       checkpoint_timeout = 1d
+       log_min_messages = debug2
+));
+
+$node_primary->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node_primary->check_extension('injection_points'))
+{
+       plan skip_all => 'Extension injection_points not installed';
+}
+$node_primary->safe_psql('postgres', qq{ CREATE EXTENSION injection_points });
+
+# Create table that we will experiment with
+$node_primary->safe_psql('postgres', qq{ CREATE TABLE test_tab (id INT); });
+
+$node_primary->safe_psql('postgres',
+       "SELECT injection_points_attach('wait-checkpointer-deals', 'wait');"
+);
+
+my $psql = $node_primary->background_psql('postgres', on_error_stop => 1);
+
+$psql->query_until(
+       qr/start/, q{
+       \echo start
+       BEGIN;
+       INSERT INTO test_tab VALUES(1);
+       PREPARE TRANSACTION 'test_transaction';
+       COMMIT PREPARED 'test_transaction';
+});
+
+$node_primary->wait_for_event('client backend', 'wait-checkpointer-deals');
+
+$node_primary->safe_psql('postgres',"CHECKPOINT;");
+
+$node_primary->safe_psql('postgres',
+       "SELECT injection_points_wakeup('wait-checkpointer-deals');"
+);
+
+sleep(5);
+
+# expect coredump here
+$psql->quit;
+$node_primary->stop;
+done_testing();

Reply via email to