> On Feb 25, 2026, at 01:35, Yura Sokolov <[email protected]> wrote: > > Good day. > > multixact.c has bug in initialization and access of OldestMemberMXactId > (and partially OldestVisibleMXactId). > > Size of this arrays is defined as: > > #define MaxOldestSlot (MaxBackends + max_prepared_xacts) > > assuming there are only backends and prepared transactions could hold > multixacts. > > This assumption is correct. And in fact there were no bug when this formula > were introduced in 2009y [1], since these arrays were indexed but synthetic > dummy `dummyBackendId` field of `GlobalTransactionData` struct. > > But in 2024y [2] field `dummyBackendId` were removed and pgprocno were used > instead. > > But proc structs reserved for two phase commit are placed after auxiliary > procs, therefore writes to OldestMemberMXactId[dummy] starts to overwrites > slots of OldestVisibleMXactId. > > Then PostgreSQL 18 increased NUM_AUXILIARY_PROCS due to reserve of workers > for AIO. And it is possible to make such test postgresql.conf with so > extremely low MaxBackend so writes to OldestMemberMXactId[dummy] overwrites > first entry of BufferDescriptors, which are allocated next in shared memory. > > Patch in attach replaces direct accesses to this arrays with inline > functions which include asserts. And changes calculation of MaxOldestSlot > to include NUM_AUXILIARY_PROCS. > > Certainly, it is not clearest patch possible: > - may be you will decide to not introduce inline functions, > - or will introduce separate inline function for each array, > - or will fix slot calculation to remove aux procs from account, > - or will revert deletion of dummyBackendId. > > [1] > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=cd87b6f8a5084c070c3e56b07794be8fea33647d > [2] > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ab355e3a88de745607f6dd4c21f0119b5c68f2ad > > -- > regards > Yura Sokolov aka funny-falcon > <v00-0001-Fix-multixacts-OldestMemberMXactId-and-OldestVis.patch>
I think this is a good catch. I read through the related code paths:
In InitProcGlobal()
```
PreparedXactProcs = &procs[MaxBackends + NUM_AUXILIARY_PROCS];
```
Then in TwoPhaseShmemInit()
```
gxacts[i].pgprocno = GetNumberFromPGProc(&PreparedXactProcs[i]);
```
This shows prepared xacts are placed after MaxBackends + NUM_AUXILIARY_PROCS.
Also, in InitializeMaxBackends():
```
/* Note that this does not include "auxiliary" processes */
MaxBackends = MaxConnections + autovacuum_worker_slots +
max_worker_processes + max_wal_senders + NUM_SPECIAL_WORKER_PROCS;
```
So MaxBackends does not include NUM_AUXILIARY_PROCS.
Overall, the change looks correct to me.
One minor thought on the new helper asserts:
```
+ Assert(procno < MaxOldestSlot);
```
It may be worth also checking procno >= 0, since INVALID_PROC_NUMBER is -1.
While reading, I also noticed two stale comments related to this area. I added
those fixes in 0002 (attached v2); 0001 is unchanged from the original v00.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v2-0001-Fix-multixacts-OldestMemberMXactId-and-OldestVisi.patch
Description: Binary data
v2-0002-Fix-stale-comments-about-prepared-xact-proc-numbe.patch
Description: Binary data
