Fix OldestMemberMXactId and OldestVisibleMXactId array usage

Commit ab355e3a88 changed how the OldestMemberMXactId array is
indexed. It's no longer indexed by synthetic dummyBackendId, but with
ProcNumber. The PGPROC entries for prepared xacts come after auxiliary
processes in the allProcs array, which rendered the calculation for
MaxOldestSlot and the indexes into the array incorrect.  (The
OldestVisibleMXactId array is not used for prepared xacts, and thus
never accessed with ProcNumber's greater than MaxBackends, so this
only affects the OldestMemberMXactId array.)

As a result, a prepared xact would store its value past the end of the
OldestMemberMXactId array, overflowing into the OldestVisibleMXactId
array. That could cause a transaction's row lock to appear invisible
to other backends, or other such visibility issues. With a very small
max_connections setting, the store could even go beyond the
OldestVisibleMXactId array, stomping over the first element in the
BufferDescriptor array.

To fix, calculate the array sizes more precisely, and introduce helper
functions to calculate the array indexes correctly.

Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/7acc94b0-ea82-4657-b1b0-77842cb7a60c@postgrespro.ru
Backpatch-through: 17
This commit is contained in:
Heikki Linnakangas 2026-03-02 19:19:22 +02:00
parent 344b572e3e
commit ccae90abdb
4 changed files with 99 additions and 44 deletions

View file

@ -159,12 +159,7 @@ typedef struct MultiXactStateData
/*
* Per-backend data starts here. We have two arrays stored in the area
* immediately following the MultiXactStateData struct. Each is indexed by
* ProcNumber.
*
* In both arrays, there's a slot for all normal backends
* (0..MaxBackends-1) followed by a slot for max_prepared_xacts prepared
* transactions.
* immediately following the MultiXactStateData struct:
*
* OldestMemberMXactId[k] is the oldest MultiXactId each backend's current
* transaction(s) could possibly be a member of, or InvalidMultiXactId
@ -176,6 +171,10 @@ typedef struct MultiXactStateData
* member of a MultiXact, and that MultiXact would have to be created
* during or after the lock acquisition.)
*
* In the OldestMemberMXactId array, there's a slot for all normal
* backends (0..MaxBackends-1) followed by a slot for max_prepared_xacts
* prepared transactions.
*
* OldestVisibleMXactId[k] is the oldest MultiXactId each backend's
* current transaction(s) think is potentially live, or InvalidMultiXactId
* when not in a transaction or not in a transaction that's paid any
@ -187,6 +186,9 @@ typedef struct MultiXactStateData
* than its own OldestVisibleMXactId[] setting; this is necessary because
* the relevant SLRU data can be concurrently truncated away.
*
* In the OldestVisibleMXactId array, there's a slot for all normal
* backends (0..MaxBackends-1) only. No slots for prepared transactions.
*
* The oldest valid value among all of the OldestMemberMXactId[] and
* OldestVisibleMXactId[] entries is considered by vacuum as the earliest
* possible value still having any live member transaction -- OldestMxact.
@ -208,9 +210,10 @@ typedef struct MultiXactStateData
} MultiXactStateData;
/*
* Size of OldestMemberMXactId and OldestVisibleMXactId arrays.
* Sizes of OldestMemberMXactId and OldestVisibleMXactId arrays.
*/
#define MaxOldestSlot (MaxBackends + max_prepared_xacts)
#define NumMemberSlots (MaxBackends + max_prepared_xacts)
#define NumVisibleSlots MaxBackends
/* Pointers to the state data in shared memory */
static MultiXactStateData *MultiXactState;
@ -218,6 +221,41 @@ static MultiXactId *OldestMemberMXactId;
static MultiXactId *OldestVisibleMXactId;
static inline MultiXactId *
MyOldestMemberMXactIdSlot(void)
{
/*
* The first MaxBackends entries in the OldestMemberMXactId array are
* reserved for regular backends. MyProcNumber should index into one of
* them.
*/
Assert(MyProcNumber >= 0 && MyProcNumber < MaxBackends);
return &OldestMemberMXactId[MyProcNumber];
}
static inline MultiXactId *
PreparedXactOldestMemberMXactIdSlot(ProcNumber procno)
{
int prepared_xact_idx;
Assert(procno >= FIRST_PREPARED_XACT_PROC_NUMBER);
prepared_xact_idx = procno - FIRST_PREPARED_XACT_PROC_NUMBER;
/*
* The first MaxBackends entries in the OldestMemberMXactId array are
* reserved for regular backends. Prepared xacts come after them.
*/
Assert(MaxBackends + prepared_xact_idx < NumMemberSlots);
return &OldestMemberMXactId[MaxBackends + prepared_xact_idx];
}
static inline MultiXactId *
MyOldestVisibleMXactIdSlot(void)
{
Assert(MyProcNumber >= 0 && MyProcNumber < NumVisibleSlots);
return &OldestVisibleMXactId[MyProcNumber];
}
/*
* Definitions for the backend-local MultiXactId cache.
*
@ -308,7 +346,7 @@ MultiXactIdCreate(TransactionId xid1, MultiXactStatus status1,
Assert(!TransactionIdEquals(xid1, xid2) || (status1 != status2));
/* MultiXactIdSetOldestMember() must have been called already. */
Assert(MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber]));
Assert(MultiXactIdIsValid(*MyOldestMemberMXactIdSlot()));
/*
* Note: unlike MultiXactIdExpand, we don't bother to check that both XIDs
@ -362,7 +400,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
Assert(TransactionIdIsValid(xid));
/* MultiXactIdSetOldestMember() must have been called already. */
Assert(MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber]));
Assert(MultiXactIdIsValid(*MyOldestMemberMXactIdSlot()));
debug_elog5(DEBUG2, "Expand: received multi %u, xid %u status %s",
multi, xid, mxstatus_to_string(status));
@ -536,7 +574,7 @@ MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly)
void
MultiXactIdSetOldestMember(void)
{
if (!MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber]))
if (!MultiXactIdIsValid(*MyOldestMemberMXactIdSlot()))
{
MultiXactId nextMXact;
@ -558,7 +596,7 @@ MultiXactIdSetOldestMember(void)
nextMXact = MultiXactState->nextMXact;
OldestMemberMXactId[MyProcNumber] = nextMXact;
*MyOldestMemberMXactIdSlot() = nextMXact;
LWLockRelease(MultiXactGenLock);
@ -586,7 +624,7 @@ MultiXactIdSetOldestMember(void)
static void
MultiXactIdSetOldestVisible(void)
{
if (!MultiXactIdIsValid(OldestVisibleMXactId[MyProcNumber]))
if (!MultiXactIdIsValid(*MyOldestVisibleMXactIdSlot()))
{
MultiXactId oldestMXact;
int i;
@ -594,7 +632,7 @@ MultiXactIdSetOldestVisible(void)
LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE);
oldestMXact = MultiXactState->nextMXact;
for (i = 0; i < MaxOldestSlot; i++)
for (i = 0; i < NumMemberSlots; i++)
{
MultiXactId thisoldest = OldestMemberMXactId[i];
@ -603,7 +641,7 @@ MultiXactIdSetOldestVisible(void)
oldestMXact = thisoldest;
}
OldestVisibleMXactId[MyProcNumber] = oldestMXact;
*MyOldestVisibleMXactIdSlot() = oldestMXact;
LWLockRelease(MultiXactGenLock);
@ -1152,7 +1190,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
* multi. It cannot possibly still be running.
*/
if (isLockOnly &&
MultiXactIdPrecedes(multi, OldestVisibleMXactId[MyProcNumber]))
MultiXactIdPrecedes(multi, *MyOldestVisibleMXactIdSlot()))
{
debug_elog2(DEBUG2, "GetMembers: a locker-only multi is too old");
*members = NULL;
@ -1574,8 +1612,8 @@ AtEOXact_MultiXact(void)
* We assume that storing a MultiXactId is atomic and so we need not take
* MultiXactGenLock to do this.
*/
OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId;
OldestVisibleMXactId[MyProcNumber] = InvalidMultiXactId;
*MyOldestMemberMXactIdSlot() = InvalidMultiXactId;
*MyOldestVisibleMXactIdSlot() = InvalidMultiXactId;
/*
* Discard the local MultiXactId cache. Since MXactContext was created as
@ -1595,7 +1633,7 @@ AtEOXact_MultiXact(void)
void
AtPrepare_MultiXact(void)
{
MultiXactId myOldestMember = OldestMemberMXactId[MyProcNumber];
MultiXactId myOldestMember = *MyOldestMemberMXactIdSlot();
if (MultiXactIdIsValid(myOldestMember))
RegisterTwoPhaseRecord(TWOPHASE_RM_MULTIXACT_ID, 0,
@ -1615,7 +1653,7 @@ PostPrepare_MultiXact(FullTransactionId fxid)
* Transfer our OldestMemberMXactId value to the slot reserved for the
* prepared transaction.
*/
myOldestMember = OldestMemberMXactId[MyProcNumber];
myOldestMember = *MyOldestMemberMXactIdSlot();
if (MultiXactIdIsValid(myOldestMember))
{
ProcNumber dummyProcNumber = TwoPhaseGetDummyProcNumber(fxid, false);
@ -1628,8 +1666,8 @@ PostPrepare_MultiXact(FullTransactionId fxid)
*/
LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE);
OldestMemberMXactId[dummyProcNumber] = myOldestMember;
OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId;
*PreparedXactOldestMemberMXactIdSlot(dummyProcNumber) = myOldestMember;
*MyOldestMemberMXactIdSlot() = InvalidMultiXactId;
LWLockRelease(MultiXactGenLock);
}
@ -1642,7 +1680,7 @@ PostPrepare_MultiXact(FullTransactionId fxid)
* We assume that storing a MultiXactId is atomic and so we need not take
* MultiXactGenLock to do this.
*/
OldestVisibleMXactId[MyProcNumber] = InvalidMultiXactId;
*MyOldestVisibleMXactIdSlot() = InvalidMultiXactId;
/*
* Discard the local MultiXactId cache like in AtEOXact_MultiXact.
@ -1669,7 +1707,7 @@ multixact_twophase_recover(FullTransactionId fxid, uint16 info,
Assert(len == sizeof(MultiXactId));
oldestMember = *((MultiXactId *) recdata);
OldestMemberMXactId[dummyProcNumber] = oldestMember;
*PreparedXactOldestMemberMXactIdSlot(dummyProcNumber) = oldestMember;
}
/*
@ -1684,7 +1722,7 @@ multixact_twophase_postcommit(FullTransactionId fxid, uint16 info,
Assert(len == sizeof(MultiXactId));
OldestMemberMXactId[dummyProcNumber] = InvalidMultiXactId;
*PreparedXactOldestMemberMXactIdSlot(dummyProcNumber) = InvalidMultiXactId;
}
/*
@ -1699,21 +1737,32 @@ multixact_twophase_postabort(FullTransactionId fxid, uint16 info,
}
/*
* Initialization of shared memory for MultiXact. We use two SLRU areas,
* thus double memory. Also, reserve space for the shared MultiXactState
* struct and the per-backend MultiXactId arrays (two of those, too).
* Initialization of shared memory for MultiXact.
*
* MultiXactSharedStateShmemSize() calculates the size of the MultiXactState
* struct, and the two per-backend MultiXactId arrays. They are carved out of
* the same allocation. MultiXactShmemSize() additionally includes the memory
* needed for the two SLRU areas.
*/
static Size
MultiXactSharedStateShmemSize(void)
{
Size size;
size = offsetof(MultiXactStateData, perBackendXactIds);
size = add_size(size,
mul_size(sizeof(MultiXactId), NumMemberSlots));
size = add_size(size,
mul_size(sizeof(MultiXactId), NumVisibleSlots));
return size;
}
Size
MultiXactShmemSize(void)
{
Size size;
/* We need 2*MaxOldestSlot perBackendXactIds[] entries */
#define SHARED_MULTIXACT_STATE_SIZE \
add_size(offsetof(MultiXactStateData, perBackendXactIds), \
mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot))
size = SHARED_MULTIXACT_STATE_SIZE;
size = MultiXactSharedStateShmemSize();
size = add_size(size, SimpleLruShmemSize(multixact_offset_buffers, 0));
size = add_size(size, SimpleLruShmemSize(multixact_member_buffers, 0));
@ -1747,14 +1796,14 @@ MultiXactShmemInit(void)
/* Initialize our shared state struct */
MultiXactState = ShmemInitStruct("Shared MultiXact State",
SHARED_MULTIXACT_STATE_SIZE,
MultiXactSharedStateShmemSize(),
&found);
if (!IsUnderPostmaster)
{
Assert(!found);
/* Make sure we zero out the per-backend state */
MemSet(MultiXactState, 0, SHARED_MULTIXACT_STATE_SIZE);
MemSet(MultiXactState, 0, MultiXactSharedStateShmemSize());
}
else
Assert(found);
@ -1763,7 +1812,7 @@ MultiXactShmemInit(void)
* Set up array pointers.
*/
OldestMemberMXactId = MultiXactState->perBackendXactIds;
OldestVisibleMXactId = OldestMemberMXactId + MaxOldestSlot;
OldestVisibleMXactId = OldestMemberMXactId + NumMemberSlots;
}
/*
@ -2303,7 +2352,6 @@ MultiXactId
GetOldestMultiXactId(void)
{
MultiXactId oldestMXact;
int i;
/*
* This is the oldest valid value among all the OldestMemberMXactId[] and
@ -2311,7 +2359,7 @@ GetOldestMultiXactId(void)
*/
LWLockAcquire(MultiXactGenLock, LW_SHARED);
oldestMXact = MultiXactState->nextMXact;
for (i = 0; i < MaxOldestSlot; i++)
for (int i = 0; i < NumMemberSlots; i++)
{
MultiXactId thisoldest;
@ -2319,6 +2367,11 @@ GetOldestMultiXactId(void)
if (MultiXactIdIsValid(thisoldest) &&
MultiXactIdPrecedes(thisoldest, oldestMXact))
oldestMXact = thisoldest;
}
for (int i = 0; i < NumVisibleSlots; i++)
{
MultiXactId thisoldest;
thisoldest = OldestVisibleMXactId[i];
if (MultiXactIdIsValid(thisoldest) &&
MultiXactIdPrecedes(thisoldest, oldestMXact))

View file

@ -900,9 +900,10 @@ TwoPhaseGetXidByVirtualXID(VirtualTransactionId vxid,
* Get the dummy proc number for prepared transaction
*
* Dummy proc numbers are similar to proc numbers of real backends. They
* start at MaxBackends, and are unique across all currently active real
* backends and prepared transactions. If lock_held is set to true,
* TwoPhaseStateLock will not be taken, so the caller had better hold it.
* start at FIRST_PREPARED_XACT_PROC_NUMBER, and are unique across all
* currently active real backends and prepared transactions. If lock_held is
* set to true, TwoPhaseStateLock will not be taken, so the caller had better
* hold it.
*/
ProcNumber
TwoPhaseGetDummyProcNumber(FullTransactionId fxid, bool lock_held)

View file

@ -304,7 +304,7 @@ InitProcGlobal(void)
* dummy PGPROCs don't need these though - they're never associated
* with a real process
*/
if (i < MaxBackends + NUM_AUXILIARY_PROCS)
if (i < FIRST_PREPARED_XACT_PROC_NUMBER)
{
proc->sem = PGSemaphoreCreate();
InitSharedLatch(&(proc->procLatch));
@ -369,7 +369,7 @@ InitProcGlobal(void)
* processes and prepared transactions.
*/
AuxiliaryProcs = &procs[MaxBackends];
PreparedXactProcs = &procs[MaxBackends + NUM_AUXILIARY_PROCS];
PreparedXactProcs = &procs[FIRST_PREPARED_XACT_PROC_NUMBER];
}
/*

View file

@ -532,6 +532,7 @@ extern PGDLLIMPORT PGPROC *PreparedXactProcs;
#define MAX_IO_WORKERS 32
#define NUM_AUXILIARY_PROCS (6 + MAX_IO_WORKERS)
#define FIRST_PREPARED_XACT_PROC_NUMBER (MaxBackends + NUM_AUXILIARY_PROCS)
/* configurable options */
extern PGDLLIMPORT int DeadlockTimeout;