public inbox for [email protected]
help / color / mirror / Atom feedFrom: Masahiko Sawada <[email protected]>
To: Andres Freund <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: Startup process deadlock: WaitForProcSignalBarriers vs aux process
Date: Fri, 24 Apr 2026 10:52:31 -0700
Message-ID: <CAD21AoBj+zKvgw_Q8gjr4YbKccW_uMe3OFQ5+KT246FHUuNXSQ@mail.gmail.com> (raw)
In-Reply-To: <ojnnvj553eber32njg5nphybcf6zvb3bzexfkh3obylsll67pc@jk7jmtzeb66k>
References: <CAEze2WgAJmWReDN7Chtba8Er2YBvKCoa0KVN25-1evnTrHsLyA@mail.gmail.com>
<ojnnvj553eber32njg5nphybcf6zvb3bzexfkh3obylsll67pc@jk7jmtzeb66k>
On Wed, Apr 22, 2026 at 12:05 PM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2026-04-22 13:21:02 +0200, Matthias van de Meent wrote:
> > If the PSB is emitted (and signaled to checkpointer) before the
> > checkpointer has registered its SIGUSR1 handler, then the checkpointer
> > won't receive the notice to check its procsignal slots, it won't
> > notice the updated procsignal flags, and it won't process the PSB; not
> > until it receives a new SIGUSR1.
> >
> > Signals are sent to all processes that have their procsignal pss_pid
> > set, which is true for every process which has called ProcSignalInit,
> > which for the checkpointer (like other aux processes) happens in
> > AuxiliaryProcessMainCommon. However, checkpointer (also like other aux
> > processes) calls AuxiliaryProcessMainCommon before registering its
> > signal handlers, creating a small window in time where signals are
> > sent, but not handled.
>
> Hm. Have we confirmed this happens?
>
> CheckpointerMain() is called with all signals masked, so it should be ok for
> the signal handler to only be set up after AuxiliaryProcessMainCommon(), as
> long as it happens before
>
> /*
> * Unblock signals (they were blocked when the postmaster forked us)
> */
> sigprocmask(SIG_SETMASK, &UnBlockSig, NULL);
>
> as the signal delivery should be held until after unblocking signals.
Right. The postmaster blocks all signals before starting child process
as the following comment explains:
/*
* We start postmaster children with signals blocked. This allows them to
* install their own handlers before unblocking, to avoid races where they
* might run the postmaster's handler and miss an important control
* signal. With more analysis this could potentially be relaxed.
*/
sigprocmask(SIG_SETMASK, &BlockSig, &save_mask);
Investigating the issue, I found there is a race condition between the
procsignal initialization and emitting signal barrier that could be
the cause of this issue. Imagine the following scenario:
1. In ProcSignalInit(), the checkpointer initializes its
slot->pss_barrierGeneration with the global generation.
2. In EmitProcSignalBarrier(), the startup checks the checkpointer's
procsignal slot but it skips emitting the signal as slot->pss_pid is
still 0. It can happen even though the checkpointer holds a spinlock
on its slot during the initialization because the first pid check is
done without a spinlock acquisition.
3. The checkpointer sets its pid to slot->pss_pid and releases the spin lock.
4. In WaitForProcSignalBarrier(), the startup checks the
checkpointer's procsignal slot that has already initialized the
pss_barrierGeneration, and waits for it to be updated. However, the
checkpointer never updates its barrier generation as it doesn't get
the signal.
Another similar issue I found would be that child processes could miss
the PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO signal during the
initialization and end up in an inconsistent state because
InitializeProcessXLogLogicalInfo() is called (in BaseInit()) before
ProcSignalInit(). If the startup emits the signal to a process who is
between two steps, the process would not reflect the latest
XLogLogicalInfo state. I think we should move
InitializeProcessXLogLogicalInfo() after ProcSignalInit() like we do
so for InitLocalDataChecksumState().
I've attached the patch for fixing the latter problem as the fix is
straightforward.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-patch] 0001-Fix-race-condition-in-XLogLogicalInfo-and-ProcSignal.patch (4.3K, 2-0001-Fix-race-condition-in-XLogLogicalInfo-and-ProcSignal.patch)
download | inline diff:
From 01370879bb0fe4065d06d92efb01582d1b1df996 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <[email protected]>
Date: Fri, 24 Apr 2026 10:36:55 -0700
Subject: [PATCH] Fix race condition in XLogLogicalInfo and ProcSignal
initialization
Previously, InitializeProcessXLogLogicalInfo() was called before
ProcSignalInit(). This created a window where a process could miss a
signal barrier if it was issued between these two calls. As a result,
the process could fail to update its local XLogLogicalInfo cache,
leading to an inconsistent logical decoding state.
This commit fixes this by moving InitializeProcessXLogLogicalInfo()
after ProcSignalInit(). This ensures that the process is registered to
participate in signal barriers before its state is initialized,
preventing it from missing any state change propagated during the
startup sequence.
Discussion: https://postgr.es/m/
---
src/backend/postmaster/auxprocess.c | 17 ++++++++++++-----
src/backend/utils/init/postinit.c | 20 ++++++++++++--------
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 8fdc518b3a1..01cced61492 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -71,12 +71,16 @@ AuxiliaryProcessMainCommon(void)
ProcSignalInit(NULL, 0);
/*
- * Initialize a local cache of the data_checksum_version, to be updated by
- * the procsignal-based barriers.
+ * Initialize local states, to be updated by the procsignal-based
+ * barriers.
*
- * This intentionally happens after initializing the procsignal, otherwise
- * we might miss a state change. This means we can get a barrier for the
- * state we've just initialized - but it can happen only once.
+ * These initialization intentionally happens afater initializing the
+ * procsignal, otherwise we might miss a state change. This means we can
+ * get a barrier for the state we've just initialized.
+ */
+
+ /*
+ * Initialize a local cache of the data_checksum_version.
*
* The postmaster (which is what gets forked into the new child process)
* does not handle barriers, therefore it may not have the current value
@@ -88,6 +92,9 @@ AuxiliaryProcessMainCommon(void)
*/
InitLocalDataChecksumState();
+ /* Initialize logical info WAL logging state */
+ InitializeProcessXLogLogicalInfo();
+
/*
* Auxiliary processes don't run transactions, but they may need a
* resource owner anyway to manage buffer pins acquired outside
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 6f074013aa9..96b06e444ec 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -662,9 +662,6 @@ BaseInit(void)
/* Initialize lock manager's local structs */
InitLockManagerAccess();
- /* Initialize logical info WAL logging state */
- InitializeProcessXLogLogicalInfo();
-
/*
* Initialize replication slots after pgstat. The exit hook might need to
* drop ephemeral slots, which in turn triggers stats reporting.
@@ -759,12 +756,16 @@ InitPostgres(const char *in_dbname, Oid dboid,
ProcSignalInit(MyCancelKey, MyCancelKeyLength);
/*
- * Initialize a local cache of the data_checksum_version, to be updated by
- * the procsignal-based barriers.
+ * Initialize local states, to be updated by the procsignal-based
+ * barriers.
*
- * This intentionally happens after initializing the procsignal, otherwise
- * we might miss a state change. This means we can get a barrier for the
- * state we've just initialized.
+ * These initialization intentionally happens afater initializing the
+ * procsignal, otherwise we might miss a state change. This means we can
+ * get a barrier for the state we've just initialized.
+ */
+
+ /*
+ * Initialize a local cache of the data_checksum_version.
*
* The postmaster (which is what gets forked into the new child process)
* does not handle barriers, therefore it may not have the current value
@@ -776,6 +777,9 @@ InitPostgres(const char *in_dbname, Oid dboid,
*/
InitLocalDataChecksumState();
+ /* Initialize logical info WAL logging state */
+ InitializeProcessXLogLogicalInfo();
+
/*
* Also set up timeout handlers needed for backend operation. We need
* these in every case except bootstrap.
--
2.53.0
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Startup process deadlock: WaitForProcSignalBarriers vs aux process
In-Reply-To: <CAD21AoBj+zKvgw_Q8gjr4YbKccW_uMe3OFQ5+KT246FHUuNXSQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox