Fix a wal_debug crash with the new shmem allocation API

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.

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.

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.

- Heikki

On Thu, Oct 8, 2026 at 3:37 PM Heikki Linnakangas <hlinnaka@iki.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

Hi,

On Thu, Oct 8, 2026 at 3:07 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

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.

Thanks. Yes, this fix is better than mine. Since the wal_debug memory
context is only used while inserting WAL records, moving it to the WAL
insert working buffers initialization code is correct. The patch LGTM.

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.

Moving the per-backend file flush context to the buffer pool access
init function looks good to me. Do we need to keep
BackendWritebackContext as extern? A quick check on
https://sourcegraph.com says no, but just in case any external module
needs it.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On 08/10/2026 15:52, Ashutosh Bapat wrote:

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().

Yeah, InitXLogInsert() isn't perfect. If there was another direct caller
of XLogInsertRecord() that from somewhere else than XLogInsert(), and
that caller didn't call InitXLogInsert(), it'd not work. But
XLogInsertRecord() is not really intended to be used directly like that,
and even if it was, it'd be reasonable to assume that you'd call
InitXLogInsert() in the backend first anyway. So yeah, I also think it's OK.

We don't have a universal mechanism or convention for per-backend
initialization functions. There are many functions like InitXLogInsert()
and InitBufferManagerAccess(), but how they're all called is a little ad
hoc. And no such facility for extensions at all.

On 08/10/2026 21:53, Bharath Rupireddy wrote:

Hi,

On Thu, Oct 8, 2026 at 3:07 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

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.

Thanks. Yes, this fix is better than mine. Since the wal_debug memory
context is only used while inserting WAL records, moving it to the WAL
insert working buffers initialization code is correct. The patch LGTM.

Ok, committed, thanks!

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.

Moving the per-backend file flush context to the buffer pool access
init function looks good to me. Do we need to keep
BackendWritebackContext as extern? A quick check on
https://sourcegraph.com says no, but just in case any external module
needs it.

I don't think there's any need to keep it as an extern. An extension
might want to use their own WritebackContext, maybe, but I don't think
they should be messing with BackendWritebackContext.

Committed this too.

- Heikki