From 5e544af8ecdd00cf5cfe4714a16b7188b16b2217 Mon Sep 17 00:00:00 2001 From: Vlad Lesin Date: Tue, 29 Sep 2026 07:59:43 -0700 Subject: [PATCH v2 2/2] Assert that a PGPROC is in the proc array before writing its entry The dense arrays in ProcGlobal are indexed by PGPROC->pgxactoff, which is only meaningful while the PGPROC is in the proc array. An auxiliary process never enters it, so its pgxactoff stays zero, and a PGPROC that has been removed keeps the offset it had, which by then belongs to another PGPROC or is out of range. A store through such an offset silently overwrites another process's entry, as the previous commit fixed for ReplicationSlotRelease(). Add ProcArrayHasProc(), which checks that the PGPROC really occupies the entry its pgxactoff points to, and assert it wherever a process changes the flags in its own entry of ProcGlobal->statusFlags[]. The other writers of the array do not need it: - ProcArrayAdd() is what puts the PGPROC into the array. - ProcArrayRemove() is only called for a PGPROC that is in the array: by RemoveProcFromArray(), registered right after ProcArrayAdd(), and by FinishPreparedTransaction(), which only takes a prepared transaction whose PGPROC MarkAsPrepared() has added. - ProcArrayEndTransactionInternal() already asserts that the entry holds the PGPROC's XID, and a running transaction's XID identifies its entry. - ProcArrayEndTransaction() writes only to clear PROC_VACUUM_STATE_MASK flags. Those are set only where the new assertion is made, and a process does not leave the proc array before its transaction ends, since ShutdownPostgres() aborts it first. Checking that the caller is not an auxiliary process would cover only half of the problem: a regular backend that has left the proc array has a stale pgxactoff too. Suggested-by: Hayato Kuroda Discussion: https://postgr.es/m/889a06dd-45fc-423b-9dd2-87d5b5dc60c6@gmail.com --- src/backend/commands/indexcmds.c | 1 + src/backend/commands/vacuum.c | 1 + src/backend/replication/logical/logical.c | 1 + src/backend/replication/slot.c | 1 + src/backend/replication/walsender.c | 1 + src/backend/storage/ipc/procarray.c | 19 +++++++++++++++++++ src/include/storage/procarray.h | 1 + 7 files changed, 25 insertions(+) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5a0312fe772..3843c4d3013 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4810,6 +4810,7 @@ set_indexsafe_procflags(void) MyProc->xmin == InvalidTransactionId); LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); + Assert(ProcArrayHasProc(MyProc)); MyProc->statusFlags |= PROC_IN_SAFE_IC; ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; LWLockRelease(ProcArrayLock); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index d8c2f33c615..ee7bdf4698d 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2078,6 +2078,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, * xmin doesn't become visible ahead of setting the flag.) */ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); + Assert(ProcArrayHasProc(MyProc)); MyProc->statusFlags |= PROC_IN_VACUUM; if (params.is_wraparound) MyProc->statusFlags |= PROC_VACUUM_FOR_WRAPAROUND; diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 98e5f1dd8f9..abd36b98d5b 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -273,6 +273,7 @@ StartupDecodingContext(List *output_plugin_options, if (!IsTransactionOrTransactionBlock()) { LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); + Assert(ProcArrayHasProc(MyProc)); MyProc->statusFlags |= PROC_IN_LOGICAL_DECODING; ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; LWLockRelease(ProcArrayLock); diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index c84d021aeaf..dbc9aba82a5 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -840,6 +840,7 @@ ReplicationSlotRelease(void) if (MyProc->statusFlags & PROC_IN_LOGICAL_DECODING) { LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); + Assert(ProcArrayHasProc(MyProc)); MyProc->statusFlags &= ~PROC_IN_LOGICAL_DECODING; ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; LWLockRelease(ProcArrayLock); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index e9331de3df5..6c4d5359e6b 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -357,6 +357,7 @@ InitWalSender(void) { Assert(MyProc->xmin == InvalidTransactionId); LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); + Assert(ProcArrayHasProc(MyProc)); MyProc->statusFlags |= PROC_AFFECTS_ALL_HORIZONS; ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; LWLockRelease(ProcArrayLock); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index b7e03134ed8..1d4b25f85c4 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -645,6 +645,24 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid) LWLockRelease(ProcArrayLock); } +/* + * ProcArrayHasProc -- is proc in the proc array? + * + * Returns true if proc occupies the proc array entry its pgxactoff points + * to. + */ +bool +ProcArrayHasProc(PGPROC *proc) +{ + int pgxactoff = proc->pgxactoff; + + Assert(LWLockHeldByMe(ProcArrayLock)); + + return pgxactoff >= 0 && + pgxactoff < procArray->numProcs && + procArray->pgprocnos[pgxactoff] == GetNumberFromPGProc(proc); +} + /* * ProcArrayEndTransaction -- mark a transaction as no longer running @@ -2593,6 +2611,7 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc) * Install xmin and propagate the statusFlags that affect how the * value is interpreted by vacuum. */ + Assert(ProcArrayHasProc(MyProc)); MyProc->xmin = TransactionXmin = xmin; MyProc->statusFlags = (MyProc->statusFlags & ~PROC_XMIN_FLAGS) | (proc->statusFlags & PROC_XMIN_FLAGS); diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h index d718a5b542f..1e0d59b1e62 100644 --- a/src/include/storage/procarray.h +++ b/src/include/storage/procarray.h @@ -21,6 +21,7 @@ extern void ProcArrayAdd(PGPROC *proc); extern void ProcArrayRemove(PGPROC *proc, TransactionId latestXid); +extern bool ProcArrayHasProc(PGPROC *proc); extern void ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid); extern void ProcArrayClearTransaction(PGPROC *proc); -- 2.52.0