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

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/6078.1478985619@sss.pgh.pa.us

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