Subject: Fix signal handler in pgpool main

From: Tatsuo Ishii <[email-masked]>
Date: Mon, 08 Jun 2026 10:33:12 +0900 (JST)

I got a bug report regarding pgpool main process from Emond Papegaaij:
its exit/reload signal handler uses non signal safe functions, and it
could lead to a crash of the process.

Attached is the patch from him generated by Claude Code, with slight
modifications by me.

Basically it changes the signal handler to async-signal-safe calls
only: capture the signal number into a new volatile sig_atomic_t
main_exit_request, write one byte to the existing self-pipe to wake
the main loop, restore errno, and return.  The actual shutdown is
performed synchronously by a new do_shutdown() function called from
the pgpol main loop at the top of every iteration (via
check_requests()) and also right after the inner pool_pause() returns,
so a signal arriving during the 2-second select() sleep is acted on
without an extra tick of latency.

I think this patch is important and worth to apply to all supported
branches.

However, since it touches one of the most critical part of pgpool, I
would like to apply to master branch first and see how our build-farm
acts before back patching (it will take a few days).

Comments?
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
From 9f4022f16be5a482da95f608afe686803ebba0d4 Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <[email-masked]>
Date: Mon, 8 Jun 2026 09:18:23 +0900
Subject: [PATCH v1] Do not use signal unsafe functions in pgpool main process
 signal handler.

The pgpool main process SIGTERM/SIGINT/SIGQUIT handler did the full
shutdown work inline: ereport() (twice),
pool_semaphore_lock(MAIN_EXIT_HANDLER_SEM), terminate_all_childrens()
with a blocking waitpid(-1, ..., 0) and more ereport() calls, kill()
of the follow-child group, and finally exit(3) which runs atexit
handlers and stdio flush.  None of these are async-signal-safe (POSIX
2024 section 2.4.3 [1]).  In particular, SIGTERM arriving while pgpool
is mid-ereport() / mid-palloc() / mid-semop() could crash, hang, or
corrupt heap state.  Because the pgpool main process is the parent, a
crash here takes down every session and the pgpool cluster.

This commit restricts the handler to async-signal-safe calls only:
capture the signal number into a new volatile sig_atomic_t
main_exit_request, write one byte to the existing self-pipe to wake
the main loop, restore errno, and return.  The actual shutdown is
performed synchronously by a new do_shutdown() function called from
the pgpol main loop at the top of every iteration (via
check_requests()) and also right after the inner pool_pause() returns,
so a signal arriving during the 2-second select() sleep is acted on
without an extra tick of latency.

do_shutdown() carries the previous body verbatim - the
non-async-safe calls (ereport, pool_semaphore_lock,
terminate_all_childrens with its blocking waitpid, exit(3)) are now
invoked from normal context where they are safe.

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04_03

Reported-by: Emond Papegaaij <[email-masked]>
Reported-by: Claude code
Author: Tatsuo Ishii <[email-masked]>
Discussion:
Backpatch-through: v4.3
---
 src/main/pgpool_main.c | 111 +++++++++++++++++++++++++++++++++++------
 1 file changed, 95 insertions(+), 16 deletions(-)

diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c
index dbf1bfd14..5bb33e8de 100644
--- a/src/main/pgpool_main.c
+++ b/src/main/pgpool_main.c
@@ -171,6 +171,7 @@ static void kill_all_children(int sig);
 static pid_t fork_follow_child(int old_main_node, int new_primary, int old_primary);
 static int	read_status_file(bool discard_status);
 static RETSIGTYPE exit_handler(int sig);
+static void do_shutdown(int sig);
 static RETSIGTYPE reap_handler(int sig);
 static RETSIGTYPE sigusr1_handler(int sig);
 static void sigusr1_interrupt_processor(void);
@@ -253,6 +254,12 @@ volatile sig_atomic_t reload_config_request = 0;
 static volatile sig_atomic_t sigusr1_request = 0;
 static volatile sig_atomic_t sigchld_request = 0;
 static volatile sig_atomic_t wakeup_request = 0;
+static volatile sig_atomic_t main_exit_request = 0;	/* set by exit_handler;
+												 * carries the captured signal
+												 * number (SIGTERM/INT/QUIT)
+												 * so the main loop can
+												 * perform the (non-async-safe)
+												 * shutdown work itself. */
 
 static int	pipe_fds[2];		/* for delivering signals */
 
@@ -708,6 +715,13 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps)
 			r = pool_pause(&t);
 			POOL_SETMASK(&BlockSig);
 
+			/*
+			 * If a SIGTERM/SIGINT/SIGQUIT was queued by exit_handler,
+			 * service it immediately rather than waiting another tick.
+			 */
+			if (main_exit_request)
+				do_shutdown(main_exit_request);
+
 			if (pool_config->process_management == PM_DYNAMIC)
 				service_child_processes();
 
@@ -1290,48 +1304,104 @@ terminate_all_childrens(int sig)

 /*
- * Pgpool main process exit handler
+ * Pgpool main process exit handler.
+ *
+ * This handler runs in async-signal context: it MUST only call
+ * async-signal-safe primitives (POSIX 2024 2.4.3).  ereport(),
+ * pool_semaphore_lock(), MemoryContext operations, blocking waitpid()
+ * and exit(3) (atexit chain + stdio flush) are all unsafe here.  In a
+ * previous version this body did the full shutdown inline, which could
+ * deadlock or corrupt heap state when SIGTERM/SIGINT/SIGQUIT arrived
+ * while PT_MAIN was inside ereport(), palloc(), or another semop.
+ *
+ * The handler now only records the requested signal number into a
+ * sig_atomic_t flag and writes one byte to the self-pipe so the main
+ * loop's select() returns.  The actual shutdown is performed
+ * synchronously by do_shutdown() from the main loop, where calling
+ * non-async-safe code is permitted.
+ *
+ * If the same signal is delivered to a forked child that has not yet
+ * reset this handler, fall through to proc_exit() unchanged.
  */
 static RETSIGTYPE exit_handler(int sig)
 {
-	int		   *walk;
 	int			save_errno = errno;
 
-	ereport(LOG,
-			(errmsg("exit handler called (signal: %d)", sig)));
-
-	POOL_SETMASK(&AuthBlockSig);
-
 	/*
 	 * this could happen in a child process if a signal has been sent before
-	 * resetting signal handler
+	 * resetting signal handler.  proc_exit() ultimately reduces to _exit()
+	 * for non-PT_MAIN processes; calling it from a handler in a child that
+	 * hasn't yet rewired its own handlers is the historical behaviour.
 	 */
 	if (getpid() != mypid)
 	{
-		POOL_SETMASK(&UnBlockSig);
 		proc_exit(0);
+		errno = save_errno;
+		return;
 	}
 
 	if (sig != SIGTERM && sig != SIGINT && sig != SIGQUIT)
 	{
-		POOL_SETMASK(&UnBlockSig);
 		errno = save_errno;
 		return;
 	}
 
 	/*
-	 * Check if another exit handler instance is already running.  It is
-	 * possible that exit_handler is interrupted in the middle by other
-	 * signal.
+	 * Record the signal for the main loop.  Repeated signals coalesce; the
+	 * first one wins, which matches the previous "exiting" guard.
+	 */
+	if (main_exit_request == 0)
+		main_exit_request = sig;
+
+	/* Wake up the main loop if the self-pipe is set up. */
+	if (pipe_fds[1])
+	{
+		/*
+		 * write() is async-signal-safe.  We deliberately ignore the
+		 * return value: if the pipe is full the main loop will pick the
+		 * flag up on its next select() wake-up anyway.
+		 *
+		 * We do not use "(void) write()" here because it warns "warning:
+		 * ignoring return value of ‘write’ declared with attribute
+		 * ‘warn_unused_result’ [-Wunused-result] on gcc -O2 -Wall and some
+		 * platforms enabling _FORTIFY_SOURCE.
+		 */
+		ssize_t		w = write(pipe_fds[1], "\0", 1);
+
+		(void) w;
+	}
+
+	errno = save_errno;
+}
+
+/*
+ * Synchronous shutdown body, invoked from the PT_MAIN main loop when
+ * main_exit_request has been set by exit_handler().  All the work that used
+ * to live inside the signal handler (ereport, pool_semaphore_lock,
+ * waitpid, kill of child group, exit(3)) lives here, in normal context
+ * where it is safe.
+ */
+static void
+do_shutdown(int sig)
+{
+	int		   *walk;
+
+	ereport(LOG,
+			(errmsg("exit handler called (signal: %d)", sig)));
+
+	POOL_SETMASK(&AuthBlockSig);
+
+	/*
+	 * Check if another exit handler instance is already running.  Since the
+	 * shutdown now runs serially from the main loop this is mostly a
+	 * belt-and-braces guard, but cheap and harmless to keep.
 	 */
 	if (exiting)
 	{
 		POOL_SETMASK(&UnBlockSig);
-		errno = save_errno;
 		return;
 	}
 
-	/* Check to make sure that other exit handler is not running */
 	pool_semaphore_lock(MAIN_EXIT_HANDLER_SEM);
 	if (exiting == 0)
 	{
@@ -1344,7 +1414,6 @@ static RETSIGTYPE exit_handler(int sig)
 		ereport(LOG,
 				(errmsg("exit handler (signal: %d) called. but exit handler is already in progress", sig)));
 		POOL_SETMASK(&UnBlockSig);
-		errno = save_errno;
 		return;
 	}
 
@@ -5031,6 +5100,16 @@ check_requests(void)
 {
 	sigset_t	sig;
 
+	/*
+	 * Shutdown request?  exit_handler() set main_exit_request to the captured
+	 * signal number; perform the actual (non-async-safe) shutdown here, in
+	 * normal context.  do_shutdown() does not return.
+	 */
+	if (main_exit_request)
+	{
+		do_shutdown(main_exit_request);
+	}
+
 	/*
 	 * Waking child request?
 	 */
-- 
2.43.0

---

From: Tatsuo Ishii <[email-masked]>
Date: Tue, 23 Jun 2026 21:54:23 +0900 (JST)

Hi Emond,

> acts before back patching (it will take a few days).

Patch pushed to v4.4.

Peng and Koshino reviewed the patch off-list. No change was necessary.
I just removed some non-ASCII characters in comments (probably I
mistakenly brought them in by copy-and-paste).  I tried to back-patch
to v4.3 but it seems it needs invasive changes and I gave up it.  So I
decided to back patch through v4.4.

Thank you!

---

From: Emond Papegaaij <[email-masked]>
Date: Wed, 24 Jun 2026 15:19:10 +0200

Thank you for taking care of this! I agree with your decision to not
patch 4.3 given that the code doesn't match. You have to weigh the
benefits against the risks in such cases.

Best regards,
Emond