Subject: [PATCH v1 0/2] Fix exported snapshot xmin handoff race

From: "chee.wooson" <[email-masked]>
Date: Wed, 29 Jul 2026 18:10:43 +0800

Hi,

ProcArrayInstallImportedXmin() holds ProcArrayLock in shared mode while it
checks that the exporting transaction is still running and installs the
imported xmin.  That normally prevents transaction end from removing the
source from the proc array.  However, a read-only exporting transaction has
no assigned XID, so ProcArrayEndTransaction() clears its xmin without
ProcArrayLock.

This permits a horizon scan holding ProcArrayLock shared to pass the importer
before the imported xmin is installed.  The importer can then validate the
source under another shared lock, while the source concurrently clears its
xmin lock-free.  When the horizon scan reaches the source, it can therefore
miss both xmins and allow VACUUM to remove a tuple that is still visible to
the imported snapshot.

Tom Lane described the same general hazard in 2016:

https://postgr.es/m/[email-masked]

That discussion suspected it might not be a live bug because an exported
snapshot keeps the source xmin until transaction end.  The missing case is
that an XID-less source can clear xmin at transaction end even while another
backend holds ProcArrayLock shared.

This series adds a dedicated atomic three-state field to PGPROC.  Snapshot
export publishes EXPORTED before the snapshot file is made visible.  An
importer changes EXPORTED to REFERENCED before installing the xmin.  At
transaction end, an unreferenced export retains the existing lock-free
cleanup path, while a referenced export acquires ProcArrayLock exclusively
before clearing xmin.  If transaction end changes EXPORTED to ENDING first,
the importer fails safely.

Patch 1 adds deterministic injection points and a TAP test.  With patch 1
alone, the test reproduces the bug by failing both the expected lock wait and
the final visibility check.  Patch 2 implements the atomic handoff; all five
subtests then pass.  The test also covers the source-wins case, repeated
exports in one transaction, and the lock-free path for an export that was
never imported.

The series is based on PostgreSQL master at
c12c101b0846b1e6488f2dc986a852fbc6bf2e3b.  I also reproduced the issue on
REL_17_STABLE.

Validation on current master:

- ninja -C build
- meson test -C build test_misc/015_export_snapshot
- meson test -C build --suite regress --suite isolation
  --suite injection_points --suite test_misc
- pgindent --check on all modified C and header files

All tests pass.

Performance impact is limited to one four-byte atomic field per PGPROC, an
atomic read when an XID-less transaction ends, and state transitions during
snapshot export/import.  ProcArrayLock exclusive is added only when an
imported snapshot references the ending XID-less source.  I have not run a
dedicated performance benchmark.

chee.wooson (2):
  Add test for exported snapshot xmin race
  Fix exported snapshot xmin handoff race

 src/backend/access/transam/twophase.c         |   1 +
 src/backend/storage/ipc/procarray.c           |  99 ++++-
 src/backend/storage/lmgr/proc.c               |   4 +
 src/backend/utils/time/snapmgr.c              |  13 +
 src/include/storage/proc.h                    |  21 +
 src/test/modules/test_misc/meson.build        |   1 +
 .../test_misc/t/015_export_snapshot.pl        | 394 ++++++++++++++++++
 7 files changed, 525 insertions(+), 8 deletions(-)
 create mode 100644 src/test/modules/test_misc/t/015_export_snapshot.pl

-- 
2.43.0

---

From: "chee.wooson" <[email-masked]>
Date: Wed, 29 Jul 2026 18:10:45 +0800

A transaction that has not assigned an XID clears its advertised xmin at
transaction end without ProcArrayLock.  ProcArrayInstallImportedXmin() can
therefore verify the source under a shared lock, then lose the source xmin
before it installs the importer xmin.  A concurrent horizon scan can miss
both.

Track each PGPROC exported xmin with an atomic three-state protocol.  An
importer marks the xmin referenced before installing it.  If the importer
wins the handoff, the source clears its xmin under ProcArrayLock exclusive;
if transaction end wins, the import fails.  Snapshots that were never
imported retain the existing lock-free cleanup path.
---
 src/backend/access/transam/twophase.c |  1 +
 src/backend/storage/ipc/procarray.c   | 86 ++++++++++++++++++++++++---
 src/backend/storage/lmgr/proc.c       |  4 ++
 src/backend/utils/time/snapmgr.c      | 13 ++++
 src/include/storage/proc.h            | 21 +++++++
 5 files changed, 117 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index fa3bc50ec48..a64b1a41d0c 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -479,6 +479,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, FullTransactionId fxid,
 	proc->waitLock = NULL;
 	dlist_node_init(&proc->waitLink);
 	proc->waitProcLock = NULL;
+	pg_atomic_init_u32(&proc->xminExportState, PROC_XMIN_EXPORT_ENDING);
 	pg_atomic_init_u64(&proc->waitStart, 0);
 	for (i = 0; i < NUM_LOCK_PARTITIONS; i++)
 		dlist_init(&proc->myProcLocks[i]);
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 66b394e2b4e..a2556314325 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -687,15 +687,39 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
 	}
 	else
 	{
+		uint32		xmin_export_state;
+		bool		clear_xmin_exclusively = false;
+
 		/*
-		 * If we have no XID, we don't need to lock, since we won't affect
-		 * anyone else's calculation of a snapshot.  We might change their
-		 * estimate of global xmin, but that's OK.
+		 * If we have no XID, we ordinarily don't need to lock, since we won't
+		 * affect anyone else's calculation of a snapshot.  Atomically close
+		 * an exported xmin to new importers.  If an importer acquired a
+		 * reference first, clear under an exclusive lock to serialize with
+		 * both that importer and horizon calculations.
 		 */
 		Assert(!TransactionIdIsValid(proc->xid));
 		Assert(proc->subxidStatus.count == 0);
 		Assert(!proc->subxidStatus.overflowed);
 
+		xmin_export_state = pg_atomic_read_u32(&proc->xminExportState);
+		Assert(xmin_export_state == PROC_XMIN_EXPORT_ENDING ||
+			   xmin_export_state == PROC_XMIN_EXPORT_EXPORTED ||
+			   xmin_export_state == PROC_XMIN_EXPORT_REFERENCED);
+
+		if (xmin_export_state != PROC_XMIN_EXPORT_ENDING)
+		{
+			xmin_export_state =
+				pg_atomic_exchange_u32(&proc->xminExportState,
+									   PROC_XMIN_EXPORT_ENDING);
+			Assert(xmin_export_state == PROC_XMIN_EXPORT_EXPORTED ||
+				   xmin_export_state == PROC_XMIN_EXPORT_REFERENCED);
+			clear_xmin_exclusively =
+				(xmin_export_state == PROC_XMIN_EXPORT_REFERENCED);
+		}
+
+		if (clear_xmin_exclusively)
+			LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+
 		proc->vxid.lxid = InvalidLocalTransactionId;
 		proc->xmin = InvalidTransactionId;
 
@@ -706,13 +730,20 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
 		/* avoid unnecessarily dirtying shared cachelines */
 		if (proc->statusFlags & PROC_VACUUM_STATE_MASK)
 		{
-			Assert(!LWLockHeldByMe(ProcArrayLock));
-			LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+			if (!clear_xmin_exclusively)
+			{
+				Assert(!LWLockHeldByMe(ProcArrayLock));
+				LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+			}
 			Assert(proc->statusFlags == ProcGlobal->statusFlags[proc->pgxactoff]);
 			proc->statusFlags &= ~PROC_VACUUM_STATE_MASK;
 			ProcGlobal->statusFlags[proc->pgxactoff] = proc->statusFlags;
-			LWLockRelease(ProcArrayLock);
+			if (!clear_xmin_exclusively)
+				LWLockRelease(ProcArrayLock);
 		}
+
+		if (clear_xmin_exclusively)
+			LWLockRelease(ProcArrayLock);
 	}
 }
 
@@ -738,6 +769,7 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
 	proc->xid = InvalidTransactionId;
 	proc->vxid.lxid = InvalidLocalTransactionId;
 	proc->xmin = InvalidTransactionId;
+	pg_atomic_write_u32(&proc->xminExportState, PROC_XMIN_EXPORT_ENDING);
 
 	/* be sure this is cleared in abort */
 	proc->delayChkptFlags = 0;
@@ -2498,7 +2530,10 @@ ProcArrayInstallImportedXmin(TransactionId xmin,
 	if (!sourcevxid)
 		return false;
 
-	/* Get lock so source xact can't end while we're doing this */
+	/*
+	 * Stabilize the proc array while locating the source.  A source without
+	 * an XID can still begin ending until we acquire an xmin reference below.
+	 */
 	LWLockAcquire(ProcArrayLock, LW_SHARED);
 
 	/*
@@ -2512,6 +2547,7 @@ ProcArrayInstallImportedXmin(TransactionId xmin,
 		PGPROC	   *proc = &allProcs[pgprocno];
 		int			statusFlags = ProcGlobal->statusFlags[index];
 		TransactionId xid;
+		uint32		xmin_export_state;
 
 		/* Ignore procs running LAZY VACUUM */
 		if (statusFlags & PROC_IN_VACUUM)
@@ -2535,8 +2571,42 @@ ProcArrayInstallImportedXmin(TransactionId xmin,
 		INJECTION_POINT("install-imported-xmin-before-reference", NULL);
 
 		/*
-		 * Likewise, let's just make real sure its xmin does cover us.
+		 * Acquire a reference to the exported xmin.  This atomic transition
+		 * arbitrates with lock-free transaction end: either we change
+		 * EXPORTED to REFERENCED first, forcing the source to clear under
+		 * ProcArrayLock exclusive, or the source changes it to ENDING first
+		 * and this import fails.
 		 */
+		xmin_export_state = pg_atomic_read_u32(&proc->xminExportState);
+		while (xmin_export_state == PROC_XMIN_EXPORT_EXPORTED)
+		{
+			if (pg_atomic_compare_exchange_u32(&proc->xminExportState,
+											   &xmin_export_state,
+											   PROC_XMIN_EXPORT_REFERENCED))
+			{
+				xmin_export_state = PROC_XMIN_EXPORT_REFERENCED;
+				break;
+			}
+		}
+
+		if (xmin_export_state != PROC_XMIN_EXPORT_REFERENCED)
+		{
+			Assert(xmin_export_state == PROC_XMIN_EXPORT_ENDING);
+			continue;
+		}
+
+		/*
+		 * The source could have ended and reused its PGPROC between the
+		 * initial VXID match and the state transition above.  Recheck its
+		 * identity after acquiring the reference.  We intentionally leave a
+		 * conservatively acquired reference in place on failure.
+		 */
+		if (proc->vxid.procNumber != sourcevxid->procNumber ||
+			proc->vxid.lxid != sourcevxid->localTransactionId ||
+			proc->databaseId != MyDatabaseId)
+			continue;
+
+		/* Lastly, make real sure its xmin does cover us. */
 		xid = UINT32_ACCESS_ONCE(proc->xmin);
 		if (!TransactionIdIsNormal(xid) ||
 			!TransactionIdPrecedesOrEquals(xid, xmin))
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 9d6e69175a5..0af8774d585 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -372,6 +372,8 @@ ProcGlobalShmemInit(void *arg)
 		 */
 		pg_atomic_init_u32(&(proc->procArrayGroupNext), INVALID_PROC_NUMBER);
 		pg_atomic_init_u32(&(proc->clogGroupNext), INVALID_PROC_NUMBER);
+		pg_atomic_init_u32(&(proc->xminExportState),
+						   PROC_XMIN_EXPORT_ENDING);
 		pg_atomic_init_u64(&(proc->waitStart), 0);
 	}
 
@@ -477,6 +479,7 @@ InitProcess(void)
 	MyProc->fpLocalTransactionId = InvalidLocalTransactionId;
 	MyProc->xid = InvalidTransactionId;
 	MyProc->xmin = InvalidTransactionId;
+	pg_atomic_write_u32(&MyProc->xminExportState, PROC_XMIN_EXPORT_ENDING);
 	MyProc->pid = MyProcPid;
 	MyProc->vxid.procNumber = MyProcNumber;
 	MyProc->vxid.lxid = InvalidLocalTransactionId;
@@ -682,6 +685,7 @@ InitAuxiliaryProcess(void)
 	MyProc->fpLocalTransactionId = InvalidLocalTransactionId;
 	MyProc->xid = InvalidTransactionId;
 	MyProc->xmin = InvalidTransactionId;
+	pg_atomic_write_u32(&MyProc->xminExportState, PROC_XMIN_EXPORT_ENDING);
 	MyProc->vxid.procNumber = INVALID_PROC_NUMBER;
 	MyProc->vxid.lxid = InvalidLocalTransactionId;
 	MyProc->databaseId = InvalidOid;
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index bc98a4361bf..3760dff7558 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1122,6 +1122,7 @@ ExportSnapshot(Snapshot snapshot)
 	StringInfoData buf;
 	FILE	   *f;
 	MemoryContext oldcxt;
+	uint32		xmin_export_state;
 	char		path[MAXPGPATH];
 	char		pathtmp[MAXPGPATH];
 
@@ -1264,6 +1265,18 @@ ExportSnapshot(Snapshot snapshot)
 				(errcode_for_file_access(),
 				 errmsg("could not write to file \"%s\": %m", pathtmp)));
 
+	/*
+	 * Make our xmin available for import before publishing the file. Preserve
+	 * REFERENCED if an earlier snapshot from this transaction was already
+	 * imported.
+	 */
+	xmin_export_state = PROC_XMIN_EXPORT_ENDING;
+	if (!pg_atomic_compare_exchange_u32(&MyProc->xminExportState,
+										&xmin_export_state,
+										PROC_XMIN_EXPORT_EXPORTED))
+		Assert(xmin_export_state == PROC_XMIN_EXPORT_EXPORTED ||
+			   xmin_export_state == PROC_XMIN_EXPORT_REFERENCED);
+
 	/*
 	 * Now that we have written everything into a .tmp file, rename the file
 	 * to remove the .tmp suffix.
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 03a1a466fa8..ee8ee3862a4 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -81,6 +81,25 @@ struct XidCache
  */
 #define		PROC_XMIN_FLAGS (PROC_IN_VACUUM | PROC_IN_SAFE_IC)
 
+/*
+ * States for PGPROC.xminExportState:
+ *
+ * ENDING --ExportSnapshot()--> EXPORTED
+ * EXPORTED --snapshot import--> REFERENCED
+ * EXPORTED/REFERENCED --transaction end--> ENDING
+ *
+ * ENDING is also the quiescent state before the current transaction exports
+ * a snapshot.  Transaction end may clear xmin lock-free when it changes
+ * EXPORTED to ENDING, but must use ProcArrayLock exclusive when it changes
+ * REFERENCED to ENDING.
+ */
+typedef enum ProcXminExportState
+{
+	PROC_XMIN_EXPORT_ENDING = 0,
+	PROC_XMIN_EXPORT_EXPORTED,
+	PROC_XMIN_EXPORT_REFERENCED
+}			ProcXminExportState;
+
 /*
  * We allow a limited number of "weak" relation locks (AccessShareLock,
  * RowShareLock, RowExclusiveLock) to be recorded in the PGPROC structure
@@ -250,6 +269,8 @@ typedef struct PGPROC
 								 * vacuum must not remove tuples deleted by
 								 * xid >= xmin ! */
 
+	pg_atomic_uint32 xminExportState;	/* one of ProcXminExportState */
+
 	XidCacheStatus subxidStatus;	/* mirrored with
 									 * ProcGlobal->subxidStates[i] */
 	struct XidCache subxids;	/* cache for subtransaction XIDs */
-- 
2.43.0