From b6e34d7d89d92637a92842dc7d16d43fb21cfe8b Mon Sep 17 00:00:00 2001 From: Taiki Koshino Date: Thu, 20 Aug 2026 08:31:21 +0900 Subject: [PATCH v1] Convert pcp_exit_handler and wakeup_handler_parent to flag-only. The signal handlers in the PCP parent process directly accessed the pcp_worker_children list and performed child-process cleanup. These operations are not async-signal-safe and can modify or traverse the list while the main loop is updating it. In particular, pcp_exit_handler() can run while the main loop is adding or removing worker entries. This can cause the handler to traverse a partially modified list, crash, or send a signal to a reused PID. wakeup_handler_parent() has the same risk when forwarding SIGUSR2 to worker processes. The handlers also call operations such as exit(), waitpid(), close(), and Pgpool List functions from signal context. Changes: Flag-only Signal Handlers: Make pcp_exit_handler() and wakeup_handler_parent() only set volatile sig_atomic_t flags and return. Main-loop Processing: Process pending exit and wakeup requests from the main loop, where the worker list can be accessed safely. Direct Restart Cleanup: Update the PCP restart path to invoke the cleanup function directly from normal process context. Reported-by: Emond Papegaaij Reported-by: Claude Code Author: Taiki Koshino Discussion: Backpatch-through: v4.3 --- src/pcp_con/pcp_child.c | 58 ++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/src/pcp_con/pcp_child.c b/src/pcp_con/pcp_child.c index 23d233146..687b7ec63 100644 --- a/src/pcp_con/pcp_child.c +++ b/src/pcp_con/pcp_child.c @@ -74,6 +74,9 @@ static volatile sig_atomic_t pcp_got_sighup = 0; static volatile sig_atomic_t pcp_restart_request = 0; List *pcp_worker_children = NULL; static volatile sig_atomic_t sigchld_request = 0; +static volatile sig_atomic_t pcp_exit_request = 0; +static volatile sig_atomic_t pcp_exit_request_sig = 0; +static volatile sig_atomic_t pcp_wakeup_request = 0; static RETSIGTYPE pcp_exit_handler(int sig); static RETSIGTYPE wakeup_handler_parent(int sig); @@ -86,6 +89,7 @@ static void start_pcp_command_processor_process(int port, int *fds); static void pcp_child_will_die(int code, Datum arg); static void pcp_kill_all_children(int sig); static void reaper(void); +static void process_pcp_exit_request(void); static bool pcp_unix_fds_not_isset(int *fds, int num_pcp_fds, fd_set *opt); @@ -95,10 +99,21 @@ static bool pcp_unix_fds_not_isset(int *fds, int num_pcp_fds, fd_set *opt); { \ reaper(); \ } \ + if (pcp_wakeup_request) \ + { \ + pcp_wakeup_request = 0; \ + pcp_kill_all_children(SIGUSR2); \ + } \ + if (pcp_exit_request) \ + { \ + process_pcp_exit_request(); \ + } \ if (pcp_restart_request) \ { \ ereport(LOG,(errmsg("restart request received in pcp child process"))); \ - pcp_exit_handler(SIGTERM); \ + pcp_exit_request = 1; \ + pcp_exit_request_sig = SIGTERM; \ + process_pcp_exit_request(); \ } \ } while (0) @@ -446,14 +461,47 @@ reaper(void) } } +/* + * Async-signal-safe exit handler: just record the request and the + * triggering signal number. The pcp_worker_children list is mutated by + * non-handler code (lappend_int, list_delete_int) without blocking these + * signals, so traversal must happen at a safe point in the main loop via + * process_pcp_exit_request(). + */ static RETSIGTYPE pcp_exit_handler(int sig) { + pcp_exit_request_sig = sig; + pcp_exit_request = 1; +} + +/* + * Wakeup signal handler for pcp parent process. Async-signal-safe: just + * record the request; the main loop forwards SIGUSR2 to workers at a safe + * point where the pcp_worker_children list is not being mutated. + */ +static RETSIGTYPE +wakeup_handler_parent(int sig) +{ + pcp_wakeup_request = 1; +} + +/* + * Process a pending exit request set by pcp_exit_handler(). Runs in the + * main loop, where it is safe to walk pcp_worker_children. + */ +static void +process_pcp_exit_request(void) +{ + int sig; pid_t wpid; ListCell *lc; POOL_SETMASK(&AuthBlockSig); + sig = pcp_exit_request_sig; + pcp_exit_request = 0; + pcp_kill_all_children(sig); if (sig == SIGTERM) /* smart shutdown */ @@ -481,14 +529,6 @@ pcp_exit_handler(int sig) exit(0); } -/* Wakeup signal handler for pcp parent process */ -static RETSIGTYPE -wakeup_handler_parent(int sig) -{ - /* forward wakeup signal to all children */ - pcp_kill_all_children(SIGUSR2); -} - static RETSIGTYPE restart_handler(int sig) { -- 2.52.0