Re: Fix bug in multixact Oldest*MXactId initialization and access - Mailing list pgsql-hackers

From Chao Li
Subject Re: Fix bug in multixact Oldest*MXactId initialization and access
Date
Msg-id 93F0BD0A-0B8B-4592-9D4B-53F405A86F6A@gmail.com
Whole thread Raw
In response to Fix bug in multixact Oldest*MXactId initialization and access  (Yura Sokolov <y.sokolov@postgrespro.ru>)
Responses Re: Fix bug in multixact Oldest*MXactId initialization and access
List pgsql-hackers

> On Feb 25, 2026, at 01:35, Yura Sokolov <y.sokolov@postgrespro.ru> 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
isunchanged from the original v00. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/



Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgsql: libpq: Grease the protocol by default
Next
From: Andrew Dunstan
Date:
Subject: Re: pgsql: libpq: Grease the protocol by default