From ad0e418e3140407aea5bffd499eb27312818edd4 Mon Sep 17 00:00:00 2001 From: Emond Papegaaij Date: Sat, 25 Apr 2026 07:56:21 +0200 Subject: [PATCH] Convert pcp_worker die handler to flag-only. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PCP worker installed `die()` for SIGTERM/SIGINT/SIGQUIT via bare `signal(2)` in `pcp_worker_main()` and the handler body called `ereport(DEBUG1, ...)` and `exit(0)` / `exit(1)` directly. Both operations are non-async-signal-safe: `ereport` walks the elog state machine and may allocate, while `exit(3)` runs libc atexit handlers and stdio flushing. If a SIGTERM arrives while the worker is already inside `ereport` for another message — exactly what happens when an admin issues `pcp_stop_pgpool` mid-log — the handler enters elog recursion with inconsistent state, risking crash or double-free. The bare `signal()` install also has SysV one-shot semantics on strict implementations, so the handler may be reset to SIG_DFL after the first delivery. Convert `die()` to a flag-only handler that records the signal in `pcp_worker_shutdown_request` (a `volatile sig_atomic_t`) and returns, preserving `errno`. Add a tiny helper `check_pcp_worker_shutdown_request()` that the main loop runs at safe points (before each PCP packet read and once the read returns) to log the shutdown event and call `exit(0)` / `exit(1)` from non-signal context. Replace the bare `signal()` install sites with `pool_signal()` so the handlers are registered via `sigaction(2)` with a deliberate `sa_mask`/`sa_flags`, which also gives stable multi-shot semantics. Behaviour is preserved: SIGTERM still logs and waits for the worker to terminate naturally on client disconnect ("smart shutdown"); SIGINT and SIGQUIT still cause an immediate clean exit. --- src/pcp_con/pcp_worker.c | 52 +++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/src/pcp_con/pcp_worker.c b/src/pcp_con/pcp_worker.c index 72bf68d84..313fe89b9 100644 --- a/src/pcp_con/pcp_worker.c +++ b/src/pcp_con/pcp_worker.c @@ -64,10 +64,12 @@ extern char *pcp_conf_file; /* global variable defined in main.c holds the * path for pcp.conf */ volatile sig_atomic_t pcp_worker_wakeup_request = 0; +static volatile sig_atomic_t pcp_worker_shutdown_request = 0; PCP_CONNECTION *volatile pcp_frontend = NULL; static RETSIGTYPE die(int sig); static RETSIGTYPE wakeup_handler_child(int sig); +static void check_pcp_worker_shutdown_request(void); static void unset_nonblock(int fd); static int user_authenticate(char *buf, char *passwd_file, char *salt, int salt_len); @@ -122,15 +124,15 @@ pcp_worker_main(int port) init_ps_display("", "", "", ""); /* set up signal handlers */ - signal(SIGTERM, die); - signal(SIGINT, die); - signal(SIGQUIT, die); - signal(SIGCHLD, SIG_DFL); - signal(SIGUSR2, wakeup_handler_child); - signal(SIGUSR1, SIG_IGN); - signal(SIGHUP, SIG_IGN); - signal(SIGPIPE, SIG_IGN); - signal(SIGALRM, SIG_IGN); + pool_signal(SIGTERM, die); + pool_signal(SIGINT, die); + pool_signal(SIGQUIT, die); + pool_signal(SIGCHLD, SIG_DFL); + pool_signal(SIGUSR2, wakeup_handler_child); + pool_signal(SIGUSR1, SIG_IGN); + pool_signal(SIGHUP, SIG_IGN); + pool_signal(SIGPIPE, SIG_IGN); + pool_signal(SIGALRM, SIG_IGN); /* Create per loop iteration memory context */ PCPMemoryContext = AllocSetContextCreate(TopMemoryContext, "PCP_worker_main_loop", @@ -174,10 +176,14 @@ pcp_worker_main(int port) errno = 0; + check_pcp_worker_shutdown_request(); + /* read a PCP packet */ do_pcp_read(pcp_frontend, &tos, 1); do_pcp_read(pcp_frontend, &rsize, sizeof(int)); + check_pcp_worker_shutdown_request(); + rsize = ntohl(rsize); if (rsize <= 0 || rsize >= MAX_PCP_PACKET_LENGTH) @@ -368,11 +374,39 @@ pcp_process_command(char tos, char *buf, int buf_len) } } +/* + * Signal handler for SIGTERM/SIGINT/SIGQUIT. Async-signal-safe: only + * record the request; the main loop runs check_pcp_worker_shutdown_request() + * to log the event and exit cleanly. + */ static RETSIGTYPE die(int sig) { + int save_errno = errno; + + pcp_worker_shutdown_request = sig; + errno = save_errno; +} + +/* + * Called from the main loop at safe points. Acts on a pending SIGTERM, + * SIGINT or SIGQUIT recorded by die(). SIGTERM ("smart shutdown") only + * logs and lets the worker terminate when the client disconnects, matching + * the historical behaviour. SIGINT/SIGQUIT trigger an immediate clean exit. + */ +static void +check_pcp_worker_shutdown_request(void) +{ + int sig = pcp_worker_shutdown_request; + + if (sig == 0) + return; + + pcp_worker_shutdown_request = 0; + ereport(DEBUG1, (errmsg("PCP worker child receives shutdown request signal %d", sig))); + if (sig == SIGTERM) { ereport(DEBUG1, -- 2.51.0