Re: Fix a wal_debug crash with the new shmem allocation API

On Thu, Oct 8, 2026 at 3:37 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 21/09/2026 21:56, Bharath Rupireddy wrote:
> > I found a crash with WAL_DEBUG related code on HEAD. After commit
> > 9b5acad3f40f converted XLOGShmemInit() to the new shared memory
> > allocation API, the memory context used by wal_debug is created in the
> > initialization callback, which runs only in the postmaster or in a
> > standalone backend. In EXEC_BACKEND builds, child processes run the
> > attach callback instead, and that one was not taught to create the
> > context. Previously, XLOGShmemInit() itself ran in every child and
> > created the context before returning early.
> >

Sorry for missing this earlier.

> > As a result, with WAL_DEBUG compiled in and wal_debug turned on, a
> > child process has nowhere to allocate the description of the record it
> > is about to insert, and crashes on the first WAL insertion.
> > Reproduction steps at [1].
>
> Good catch, thanks.
>
> > I propose to fix this by creating the context from both callbacks, so
> > that every process ends up with one, as before. Please find the
> > attached patch. I think this fix needs to be back-patched through
> > PG19, where the above commit went in.
>
> Hmm, it's a bit ugly to initialize what is a purely backend-private
> thing in the ShmemInit/Attach() functions. It was expedient in the past,
> because the ShmemInit() functions happened to run at the right times,
> but initializing the memory context was never related to shared memory
> in any way. I propose the attached, which calls the InitWalDebug()
> function from InitXLogInsert() instead.

Untangling purely backend specific things from shared memory related
setup work is better. This approach is better than Bharat's.

xlog.c, which has WAL_DEBUG code, is about writing WAL to the disk and
xloginsert.c, which has InitXLogInsert() is about assembling WAL
records. InitWalDebug() does not seem to fit InitXLogInsert's charter.
Maybe the relation between InitXLogInsert() and InitWalDebug() is the
same as that of XLogInsert() and XLogInsertRecord(). So it's ok.
Further it doesn't look good to call two XLog related functions from
BaseInit().

In short, the patch looks good.

>
> Searching for similar cases where we do backend-private initialization
> that is not related to shared memory in the shmem callbacks, I found
> BufferManagerShmemAttach():
>
> > static void
> > BufferManagerShmemAttach(void *arg)
> > {
> > /* Initialize per-backend file flush context */
> > WritebackContextInit(&BackendWritebackContext,
> > &backend_flush_after);
> > }
>
> There's no bug here, but I propose that we also move that to
> InitBufferManagerAccess(), per the second attached patch.

In non-EXEC_BACKEND this is changing how BackendWritebackContext is
being set up in a backend. Before this patch, each backend would
inherit the state from Postmaster. With this patch it will not be
setup in Postmaster - which might be ok since Postmaster is not
expected to access any buffer. And also each backend will set it up on
its own. Since a pointer to the GUC variable is saved and the GUC
variable is inherited from the postmaster, I think the new code is
slightly better than the old arrangement. I think it's worth noting
this difference in the commit message.

The patch is ok to backpatch to PG 19 even though it removes a
PGDLLIMPORT variable since PG 19 is not released yet.

--
Best Wishes,
Ashutosh Bapat