Hi Andres, Andrew, Chao thanks for putting so much effort into enhancing the the previous implementation of this code. Earlier I was not aware of potential problems involved.
On Fri, Apr 10, 2026 at 9:41 AM Chao Li <[email protected]> wrote: > > > > > On Apr 9, 2026, at 18:59, Andrew Dunstan <[email protected]> wrote: > > > > > > On 2026-04-08 We 1:01 PM, Andres Freund wrote: > >> Hi, > >> > >> Attached is a very rough first draft for how I think this needs to look > >> like. > >> > >> Basically, SIGNAL_INFO always will pass both the signal number and extended > >> information along to the signal handler. The extended information is a > >> postgres specific struct. If the platform can't provide the extended > >> information, the values are instead set to some default value indicating > >> that > >> the information is not known. > >> > >> With that die() (and also StatementCancelHandler, ...) can just set > >> whatever > >> globals it wants, without pqsignal.c needing to know about it. > >> > >> It also allows us to extend the amount of information in the future. E.g. > >> I'd > >> like to log the reason for a segfault (could e.g. be an OOM kill or an > >> umapped > >> region) to stderr. > >> > >> The annoying thing about it is needing to change nearly all the existing > >> references to SIG_IGN/SIG_DFL, to avoid warnings due to mismatched types. > > > > I agree that's annoying. The only way around it I found was via some > > casting to/from void* that I suspect you would find a cure worse than the > > disease. > > I reworked your patch slightly. This version fixes the translatability > > issue you raised earlier, makes the TAP test from the original commit more > > robust, and tries to resolve your XXX issue by moving the assignment of > > ProcDieSenderPid/Uid inside the "if (!proc_exit_inprogress)" block. > > > > I reviewed this version. Besides the compile warning and uid 0 issues, I got > a few more comments, so I try to put them all together as below. > TL;DR; win/mingw is really unhappy with the state of rework patch right now. I've tried to enhance and fix it, and now it's green for default CI run and also for mingw too. Attached 0002-fixup-win32 patch does not incorporate Chao's all findings so far. [..] > 2 - uid 0 problem > ``` > +typedef struct pg_signal_info > +{ > + pid_t pid; /* pid of sending process or > 0 if unknown */ > + uid_t uid; /* uid of sending process or > 0 if unknown */ > +} pg_signal_info; > ``` > > I think we can mention that “uid” is only meaningful when pid is set. [..] > 5 > ``` > + pg_info.uid = 0; > ``` > > If you take comment 2, then when unavailable, we can just don’t assign > anything to pg_info.uid. Or "(uid_t)-1", maybe. > I. I've started from this and I vaguley remembered that I had terrible experience when trying to chose the proper types for those field types, but I couldn't remind myself why, so I've gave a try of the current rework patch and got this on Windows Server 2022, vs2019, cirrus-ci said: [08:40:22.848] c:\cirrus\src\include\c.h(1451): error C2061: syntax error: identifier 'pid_t' [08:40:22.848] c:\cirrus\src\include\c.h(1452): error C2061: syntax error: identifier 'uid' [08:40:22.848] c:\cirrus\src\include\c.h(1452): error C2059: syntax error: ';' [08:40:22.848] c:\cirrus\src\include\c.h(1453): error C2059: syntax error: '}' for c.h: 1449 typedef struct pg_signal_info 1450 { 1451 pid_t pid; /* pid of sending process or 0 if unknown */ 1452 uid_t uid; /* uid of sending process or 0 if unknown */ 1453 } pg_signal_info; so maybe we should just move that typedef with SIGNAL_ARGS to after "include of port.h" (line ~1471) in that c.h, because only then we'll have access to: src/include/port/win32_port.h:typedef int pid_t; src/include/port/win32_port.h:typedef int uid_t; but the problem is that port.h itself requires SIGNAL_ARGS to be defined and that seems to be like chicken and egg problem. I thought that just using native "ints" could be the way to go, but the problem is that now that uid_t can be bigger than pid_t as Linux kernel headers show this: x86_64-linux-gnu/bits/types.h:#define __S32_TYPE int x86_64-linux-gnu/bits/types.h:#define __U32_TYPE unsigned int x86_64-linux-gnu/bits/types.h:__STD_TYPE __UID_T_TYPE __uid_t; /* Type of user identifications. */ x86_64-linux-gnu/bits/types.h:__STD_TYPE __PID_T_TYPE __pid_t; /* Type of process identifications. */ x86_64-linux-gnu/bits/typesizes.h:#define __UID_T_TYPE __U32_TYPE x86_64-linux-gnu/bits/typesizes.h:#define __PID_T_TYPE __S32_TYPE so maybe do that typedef struct pg_signal_info with 2x uint32_t and that's good enough? (it covers the ranges necessary and makes it platform compatible) II. While we are tthis so with above uid_t of up to being u32, possibly `volatile int ProcDieSenderUid/Pid` should be also bigger like uint32_t? My fixup-patch does not incude it, because I don't know really. III. As Chao said I've used: +#define PG_SIG_DFL (pqsigfunc) (pg_funcptr_t) SIG_DFL +#define PG_SIG_IGN (pqsigfunc) (pg_funcptr_t) SIG_IGN IV. Then just got lots of warnings for use_wrapper/wrapp_handler and so on [09:27:33.602] ../src/port/pqsignal.c(206): error C2065: 'use_wrapper': undeclared identifier [09:27:33.602] ../src/port/pqsignal.c(206): warning C4113: 'pqsigfunc' differs in parameter lists from 'void (__cdecl *)(int)' that's for: 204 #else 205 /* Forward to Windows native signal system. */ 206 if (signal(signo, use_wrapper ? wrapper_handler : func) == SIG_ERR) 207 Assert(false); /* probably indicates coding error */ In the end, I've ended up using wrapper_handler for the windows path there as signal() requires function with single param. IV. Got some further issues, and VC complained that siginfo_t is used for USE_SIGACTION - isn't it impossible on win32? Anyway, that makes some sense, USE_SIGINFO is not defined and we use 'siginfo_t', so something like fixes it: @@ -90,7 +90,7 @@ static volatile pqsigfunc pqsignal_handlers[PG_NSIG]; * * This wrapper also handles restoring the value of errno. */ -#ifdef USE_SIGACTION +#if defined(USE_SIGACTION) && defined(USE_SIGINFO) static void wrapper_handler(int postgres_signal_arg, siginfo_t *info, void *context) #else V. Later I've stumbled on series of other problems related to src/backend/port/win32/signal.c (it also uses SIG_DFL but not PG_SIG_DFL and stil somewhat references pgsigfunc, so those changes seemed to impact it). Also there was: [10:37:02.824] ../src/backend/port/win32/signal.c(154): error C2198: 'sig': too few arguments for call so I've fixed with adding nodata struct there and: @@ -151,7 +154,7 @@ pgwin32_dispatch_queued_signals(void) block_mask |= sigmask(i); sigprocmask(SIG_BLOCK, &block_mask, &save_mask); - sig(i); + sig(i, &nodata); sigprocmask(SIG_SETMASK, &save_mask, NULL); VI. Possibly we could rename USE_SIGACTION define because at least to me it is confusing to me to reason and communicate about in terms of win32 context on win32 do we do have it or not? (it can be both ways): * win32 C API doesnt have it * PG does have sigaction win32 wrapper with override macro #define sigaction.. pqsigaction.. * however src/include/libpq/pqsignal.h says "sa_sigaction not yet implemented" for it (so with USE_SIGACTION are we talking about sa_sigaction field memeber that it's not used or about sigaction function?) VII. FWIW, I've also removed superflous "else" #ifdef USE_SIGINFO if (!(is_ign || is_dfl)) { act.sa_sigaction = wrapper_handler; act.sa_flags |= SA_SIGINFO; } - else #else -J.
From 6886e3c0f6ca692ca8c50bc1886d7e60b7c9508a Mon Sep 17 00:00:00 2001 From: Jakub Wartak <[email protected]> Date: Mon, 13 Apr 2026 11:24:20 +0200 Subject: [PATCH 2/2] fixup-win32 --- src/backend/port/win32/signal.c | 13 ++++++++----- src/include/c.h | 5 +++-- src/include/port.h | 4 ++-- src/port/pqsignal.c | 24 +++++++++++++++++------- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/backend/port/win32/signal.c b/src/backend/port/win32/signal.c index 465d69a1f28..d7cdb1cd1fe 100644 --- a/src/backend/port/win32/signal.c +++ b/src/backend/port/win32/signal.c @@ -88,7 +88,7 @@ pgwin32_signal_initialize(void) pg_signal_array[i].sa_handler = SIG_DFL; pg_signal_array[i].sa_mask = 0; pg_signal_array[i].sa_flags = 0; - pg_signal_defaults[i] = SIG_IGN; + pg_signal_defaults[i] = PG_SIG_IGN; } pg_signal_mask = 0; pg_signal_queue = 0; @@ -134,15 +134,18 @@ pgwin32_dispatch_queued_signals(void) { /* Execute this signal */ struct sigaction *act = &pg_signal_array[i]; - pqsigfunc sig = act->sa_handler; + pqsigfunc sig = (pqsigfunc)(pg_funcptr_t) act->sa_handler; - if (sig == SIG_DFL) + if (sig == PG_SIG_DFL) sig = pg_signal_defaults[i]; pg_signal_queue &= ~sigmask(i); - if (sig != SIG_ERR && sig != SIG_IGN && sig != SIG_DFL) + if (sig != (pqsigfunc)(pg_funcptr_t)SIG_ERR && sig != PG_SIG_IGN && sig != PG_SIG_DFL) { sigset_t block_mask; sigset_t save_mask; + struct pg_signal_info nodata; + nodata.pid = 0; + nodata.uid = 0; LeaveCriticalSection(&pg_signal_crit_sec); @@ -151,7 +154,7 @@ pgwin32_dispatch_queued_signals(void) block_mask |= sigmask(i); sigprocmask(SIG_BLOCK, &block_mask, &save_mask); - sig(i); + sig(i, &nodata); sigprocmask(SIG_SETMASK, &save_mask, NULL); EnterCriticalSection(&pg_signal_crit_sec); diff --git a/src/include/c.h b/src/include/c.h index 77ea73cc707..b43ac8e995d 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -1446,10 +1446,11 @@ extern int fdatasync(int fd); * or a field does not apply to the signal, the value is instead reset to the * documented default value. */ + typedef struct pg_signal_info { - pid_t pid; /* pid of sending process or 0 if unknown */ - uid_t uid; /* uid of sending process or 0 if unknown */ + uint32_t pid; /* pid of sending process or 0 if unknown */ + uint32_t uid; /* uid of sending process or 0 if unknown */ } pg_signal_info; /* diff --git a/src/include/port.h b/src/include/port.h index 7db476d7b01..c029878c6be 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -547,8 +547,8 @@ extern int pg_mkdir_p(char *path, int omode); #define pqsignal pqsignal_be #endif -#define PG_SIG_DFL (pqsigfunc) SIG_DFL -#define PG_SIG_IGN (pqsigfunc) SIG_IGN +#define PG_SIG_DFL (pqsigfunc) (pg_funcptr_t) SIG_DFL +#define PG_SIG_IGN (pqsigfunc) (pg_funcptr_t) SIG_IGN typedef void (*pqsigfunc) (SIGNAL_ARGS); extern void pqsignal(int signo, pqsigfunc func); diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c index 2b39be99f94..dfa5acbb139 100644 --- a/src/port/pqsignal.c +++ b/src/port/pqsignal.c @@ -90,10 +90,10 @@ static volatile pqsigfunc pqsignal_handlers[PG_NSIG]; * * This wrapper also handles restoring the value of errno. */ -#ifdef USE_SIGACTION +#if defined(USE_SIGACTION) && defined(USE_SIGINFO) static void wrapper_handler(int postgres_signal_arg, siginfo_t *info, void *context) -#else +#else /* no USE_SIGINFO */ static void wrapper_handler(int postgres_signal_arg) #endif @@ -157,6 +157,8 @@ pqsignal(int signo, pqsigfunc func) { #ifdef USE_SIGACTION struct sigaction act; +#else + void (*wrapper_func_ptr)(int); #endif bool is_ign = func == PG_SIG_IGN; bool is_dfl = func == PG_SIG_DFL; @@ -182,14 +184,12 @@ pqsignal(int signo, pqsigfunc func) act.sa_handler = SIG_IGN; else if (is_dfl) act.sa_handler = SIG_DFL; - #ifdef USE_SIGINFO if (!(is_ign || is_dfl)) { act.sa_sigaction = wrapper_handler; act.sa_flags |= SA_SIGINFO; } - else #else else act.sa_handler = wrapper_handler; @@ -201,9 +201,19 @@ pqsignal(int signo, pqsigfunc func) #endif if (sigaction(signo, &act, NULL) < 0) Assert(false); /* probably indicates coding error */ -#else - /* Forward to Windows native signal system. */ - if (signal(signo, use_wrapper ? wrapper_handler : func) == SIG_ERR) +#else /* no USE_SIGACTION */ + /* + * Forward to Windows native signal system, we need to send this though + * wrapper handler as it it needs to take single argument only. + */ + if(is_ign) + wrapper_func_ptr = SIG_IGN; + else if (is_dfl) + wrapper_func_ptr = SIG_DFL; + else + wrapper_func_ptr = wrapper_handler; + + if (signal(signo, wrapper_func_ptr) == SIG_ERR) Assert(false); /* probably indicates coding error */ #endif } -- 2.43.0
From a27f1abd54ac47e3356becc06f98355a9bbd22fc Mon Sep 17 00:00:00 2001 From: Andrew Dunstan <[email protected]> Date: Wed, 8 Apr 2026 16:09:35 -0400 Subject: [PATCH 1/2] Rework signal-sender errdetail to pass info via handler arguments. Commit 095c9d4cf06 added errdetail() reporting of the PID and UID of the process that sent a termination signal. However, as noted by Andres Freund, the implementation had architectural problems: 1. wrapper_handler() in pqsignal.c contained SIGTERM-specific logic (setting ProcDieSenderPid/Uid), violating its role as a generic signal dispatch wrapper. 2. Using globals to pass sender info between wrapper_handler and the real handler is unsafe when signals nest on some platforms. 3. The syncrep.c errdetail used psprintf() to conditionally embed text via %s, breaking translatability. Adopt the approach proposed by Andres Freund: introduce a pg_signal_info struct that is passed as an argument to all signal handlers via the SIGNAL_ARGS macro. wrapper_handler populates it from siginfo_t when SA_SIGINFO is available, or with zeros otherwise. This keeps wrapper_handler fully generic and avoids any globals for passing signal metadata. Since pqsigfunc now has a different signature from the system's signal handler type, SIG_IGN and SIG_DFL can no longer be passed directly to pqsignal(). Introduce PG_SIG_IGN and PG_SIG_DFL macros that cast to the new pqsigfunc type, and update all call sites. The legacy pqsignal() in libpq retains its original signature via a local typedef. Only die() reads pg_siginfo today, copying the sender PID/UID into ProcDieSenderPid/Uid for later use by ProcessInterrupts(). Only the first SIGTERM's sender info is recorded. Also fix the syncrep.c translatability issue by using separate ereport calls with complete, independently translatable errdetail strings. Also make the psql TAP test require the DETAIL line on platforms with SA_SIGINFO, rather than making it unconditionally optional. Author: Andres Freund <[email protected]> Reviewed-by: Andrew Dunstan <[email protected]> Discussion: https://postgr.es/m/cwyyryh2veejuxbj5ifzyaejw7jhhqc5mrdeq56xckknsdecn2@6hzfcxde2nm5 Discussion: https://postgr.es/m/jygesyr7mwg7ovdbxpmjvvbi3hccptpkcreqb645h7f56puwbz@hmkkwi3melfe --- src/backend/bootstrap/bootstrap.c | 8 +-- src/backend/postmaster/autovacuum.c | 10 +-- src/backend/postmaster/bgworker.c | 14 ++-- src/backend/postmaster/bgwriter.c | 10 +-- src/backend/postmaster/checkpointer.c | 8 +-- src/backend/postmaster/datachecksum_state.c | 2 +- src/backend/postmaster/pgarch.c | 8 +-- src/backend/postmaster/postmaster.c | 12 ++-- src/backend/postmaster/startup.c | 6 +- src/backend/postmaster/syslogger.c | 14 ++-- src/backend/postmaster/walsummarizer.c | 10 +-- src/backend/postmaster/walwriter.c | 10 +-- src/backend/replication/logical/slotsync.c | 6 +- src/backend/replication/syncrep.c | 28 ++++---- src/backend/replication/walreceiver.c | 10 +-- src/backend/replication/walsender.c | 4 +- src/backend/storage/aio/method_worker.c | 6 +- src/backend/storage/file/fd.c | 4 +- src/backend/storage/ipc/waiteventset.c | 2 +- src/backend/tcop/postgres.c | 19 ++++-- src/bin/initdb/initdb.c | 4 +- src/bin/pg_ctl/pg_ctl.c | 2 +- src/bin/pg_dump/parallel.c | 8 +-- src/bin/psql/t/001_basic.pl | 5 +- src/fe_utils/print.c | 4 +- src/include/c.h | 27 +++++--- src/include/port.h | 3 + src/interfaces/libpq/legacy-pqsignal.c | 8 ++- src/port/pqsignal.c | 75 ++++++++++++++------- src/test/regress/pg_regress.c | 2 +- src/tools/pgindent/typedefs.list | 1 + 31 files changed, 191 insertions(+), 139 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index a4af7bf8fad..b0dcd9876c5 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -463,10 +463,10 @@ bootstrap_signals(void) * mode; "curl up and die" is a sufficient response for all these cases. * Let's set that handling explicitly, as documentation if nothing else. */ - pqsignal(SIGHUP, SIG_DFL); - pqsignal(SIGINT, SIG_DFL); - pqsignal(SIGTERM, SIG_DFL); - pqsignal(SIGQUIT, SIG_DFL); + pqsignal(SIGHUP, PG_SIG_DFL); + pqsignal(SIGINT, PG_SIG_DFL); + pqsignal(SIGTERM, PG_SIG_DFL); + pqsignal(SIGQUIT, PG_SIG_DFL); } /* ---------------------------------------------------------------- diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 82061247988..680db664be4 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -445,11 +445,11 @@ AutoVacLauncherMain(const void *startup_data, size_t startup_data_len) InitializeTimeouts(); /* establishes SIGALRM handler */ - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, avl_sigusr2_handler); pqsignal(SIGFPE, FloatExceptionHandler); - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * Create a per-backend PGPROC struct in shared memory. We must do this @@ -1456,11 +1456,11 @@ AutoVacWorkerMain(const void *startup_data, size_t startup_data_len) InitializeTimeouts(); /* establishes SIGALRM handler */ - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); pqsignal(SIGFPE, FloatExceptionHandler); - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * Create a per-backend PGPROC struct in shared memory. We must do this diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 3914d22a514..2e4acad4f00 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -785,19 +785,19 @@ BackgroundWorkerMain(const void *startup_data, size_t startup_data_len) } else { - pqsignal(SIGINT, SIG_IGN); - pqsignal(SIGUSR1, SIG_IGN); - pqsignal(SIGFPE, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); + pqsignal(SIGUSR1, PG_SIG_IGN); + pqsignal(SIGFPE, PG_SIG_IGN); } pqsignal(SIGTERM, die); /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGHUP, SIG_IGN); + pqsignal(SIGHUP, PG_SIG_IGN); InitializeTimeouts(); /* establishes SIGALRM handler */ - pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR2, SIG_IGN); - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGPIPE, PG_SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * If an exception is encountered, processing resumes here. diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index a30de4262eb..cd1bf9d919c 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -101,18 +101,18 @@ BackgroundWriterMain(const void *startup_data, size_t startup_data_len) * Properly accept or ignore signals that might be sent to us. */ pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGINT, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); /* * Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * We just started, assume there has been either a shutdown or diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 6b424ee610f..087120db090 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -223,17 +223,17 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) */ pqsignal(SIGHUP, SignalHandlerForConfigReload); pqsignal(SIGINT, ReqShutdownXLOG); - pqsignal(SIGTERM, SIG_IGN); /* ignore SIGTERM */ + pqsignal(SIGTERM, PG_SIG_IGN); /* ignore SIGTERM */ /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, SignalHandlerForShutdownRequest); /* * Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * Initialize so that first time-driven event happens at the correct time. diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c index 1243949eacb..18797a8ee3d 100644 --- a/src/backend/postmaster/datachecksum_state.c +++ b/src/backend/postmaster/datachecksum_state.c @@ -1020,7 +1020,7 @@ DataChecksumsWorkerLauncherMain(Datum arg) pqsignal(SIGTERM, die); pqsignal(SIGINT, launcher_cancel_handler); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); BackgroundWorkerUnblockSignals(); diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 0a1a1149d78..0f207ac0356 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -229,16 +229,16 @@ PgArchiverMain(const void *startup_data, size_t startup_data_len) * except for SIGHUP, SIGTERM, SIGUSR1, SIGUSR2, and SIGQUIT. */ pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGINT, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, pgarch_waken_stop); /* Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* Unblock signals (they were blocked when the postmaster forked us) */ sigprocmask(SIG_SETMASK, &UnBlockSig, NULL); diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 6e0f41d2661..b6fd332f196 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -555,8 +555,8 @@ PostmasterMain(int argc, char *argv[]) pqsignal(SIGINT, handle_pm_shutdown_request_signal); pqsignal(SIGQUIT, handle_pm_shutdown_request_signal); pqsignal(SIGTERM, handle_pm_shutdown_request_signal); - pqsignal(SIGALRM, SIG_IGN); /* ignored */ - pqsignal(SIGPIPE, SIG_IGN); /* ignored */ + pqsignal(SIGALRM, PG_SIG_IGN); /* ignored */ + pqsignal(SIGPIPE, PG_SIG_IGN); /* ignored */ pqsignal(SIGUSR1, handle_pm_pmsignal_signal); pqsignal(SIGUSR2, dummy_handler); /* unused, reserve for children */ pqsignal(SIGCHLD, handle_pm_child_exit_signal); @@ -573,15 +573,15 @@ PostmasterMain(int argc, char *argv[]) * child processes should just allow the inherited settings to stand. */ #ifdef SIGTTIN - pqsignal(SIGTTIN, SIG_IGN); /* ignored */ + pqsignal(SIGTTIN, PG_SIG_IGN); /* ignored */ #endif #ifdef SIGTTOU - pqsignal(SIGTTOU, SIG_IGN); /* ignored */ + pqsignal(SIGTTOU, PG_SIG_IGN); /* ignored */ #endif /* ignore SIGXFSZ, so that ulimit violations work like disk full */ #ifdef SIGXFSZ - pqsignal(SIGXFSZ, SIG_IGN); /* ignored */ + pqsignal(SIGXFSZ, PG_SIG_IGN); /* ignored */ #endif /* Begin accepting signals. */ @@ -3967,7 +3967,7 @@ process_pm_pmsignal(void) * Dummy signal handler * * We use this for signals that we don't actually use in the postmaster, - * but we do use in backends. If we were to SIG_IGN such signals in the + * but we do use in backends. If we were to PG_SIG_IGN such signals in the * postmaster, then a newly started backend might drop a signal that arrives * before it's able to reconfigure its signal processing. (See notes in * tcop/postgres.c.) diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index cdbe53dd262..b46bac681fe 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -226,18 +226,18 @@ StartupProcessMain(const void *startup_data, size_t startup_data_len) * Properly accept or ignore signals the postmaster might send us. */ pqsignal(SIGHUP, StartupProcSigHupHandler); /* reload config file */ - pqsignal(SIGINT, SIG_IGN); /* ignore query cancel */ + pqsignal(SIGINT, PG_SIG_IGN); /* ignore query cancel */ pqsignal(SIGTERM, StartupProcShutdownHandler); /* request shutdown */ /* SIGQUIT handler was already set up by InitPostmasterChild */ InitializeTimeouts(); /* establishes SIGALRM handler */ - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, StartupProcTriggerHandler); /* * Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * Register timeouts needed for standby mode diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index 0c2a7bc8578..acfe0a01715 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -276,18 +276,18 @@ SysLoggerMain(const void *startup_data, size_t startup_data_len) pqsignal(SIGHUP, SignalHandlerForConfigReload); /* set flag to read config * file */ - pqsignal(SIGINT, SIG_IGN); - pqsignal(SIGTERM, SIG_IGN); - pqsignal(SIGQUIT, SIG_IGN); - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); + pqsignal(SIGTERM, PG_SIG_IGN); + pqsignal(SIGQUIT, PG_SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, sigUsr1Handler); /* request log rotation */ - pqsignal(SIGUSR2, SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); /* * Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); sigprocmask(SIG_SETMASK, &UnBlockSig, NULL); diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c index 20960f5b633..4f12eaf2c85 100644 --- a/src/backend/postmaster/walsummarizer.c +++ b/src/backend/postmaster/walsummarizer.c @@ -244,13 +244,13 @@ WalSummarizerMain(const void *startup_data, size_t startup_data_len) * Properly accept or ignore signals the postmaster might send us */ pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGINT, SIG_IGN); /* no query to cancel */ + pqsignal(SIGINT, PG_SIG_IGN); /* no query to cancel */ pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); /* not used */ + pqsignal(SIGUSR2, PG_SIG_IGN); /* not used */ /* Advertise ourselves. */ on_shmem_exit(WalSummarizerShutdown, (Datum) 0); @@ -267,7 +267,7 @@ WalSummarizerMain(const void *startup_data, size_t startup_data_len) /* * Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * If an exception is encountered, processing resumes here. diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 9cd86ad7022..af24d05c542 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -101,18 +101,18 @@ WalWriterMain(const void *startup_data, size_t startup_data_len) * Properly accept or ignore signals the postmaster might send us */ pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGINT, SIG_IGN); /* no query to cancel */ + pqsignal(SIGINT, PG_SIG_IGN); /* no query to cancel */ pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); /* not used */ + pqsignal(SIGUSR2, PG_SIG_IGN); /* not used */ /* * Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* * Create a memory context that we will do all our work in. We do this so diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index d01b401cd28..ad3747e598c 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -1620,9 +1620,9 @@ ReplSlotSyncWorkerMain(const void *startup_data, size_t startup_data_len) pqsignal(SIGTERM, die); pqsignal(SIGFPE, FloatExceptionHandler); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGUSR2, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); + pqsignal(SIGCHLD, PG_SIG_DFL); check_and_set_sync_info(MyProcPid); diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index 46a778f0917..896ba45412d 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -300,22 +300,18 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) */ if (ProcDiePending) { - /* - * ProcDieSenderPid/Uid are read directly from the globals here - * rather than copied to locals first; a second SIGTERM could - * change them between reads, but that is harmless because the - * process is about to die anyway. The signal sender detail is - * inlined rather than using a separate errdetail() call because - * it must be appended to the existing detail message. - */ - ereport(WARNING, - (errcode(ERRCODE_ADMIN_SHUTDOWN), - errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"), - errdetail("The transaction has already committed locally, but might not have been replicated to the standby.%s", - ProcDieSenderPid == 0 ? "" : - psprintf("\nSignal sent by PID %d, UID %d.", - (int) ProcDieSenderPid, - (int) ProcDieSenderUid)))); + if (ProcDieSenderPid != 0) + ereport(WARNING, + (errcode(ERRCODE_ADMIN_SHUTDOWN), + errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"), + errdetail("The transaction has already committed locally, but might not have been replicated to the standby. Signal sent by PID %d, UID %d.", + (int) ProcDieSenderPid, + (int) ProcDieSenderUid))); + else + ereport(WARNING, + (errcode(ERRCODE_ADMIN_SHUTDOWN), + errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"), + errdetail("The transaction has already committed locally, but might not have been replicated to the standby."))); whereToSendOutput = DestNone; SyncRepCancelWait(); break; diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 09fde92bfd7..6da5b86dbc5 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -248,16 +248,16 @@ WalReceiverMain(const void *startup_data, size_t startup_data_len) /* Properly accept or ignore signals the postmaster might send us */ pqsignal(SIGHUP, SignalHandlerForConfigReload); /* set flag to read config * file */ - pqsignal(SIGINT, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); pqsignal(SIGTERM, die); /* request shutdown */ /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); /* Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); /* Load the libpq-specific functions */ load_file("libpqwalreceiver", false); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index bad45adb004..3d4ab929f91 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -3897,13 +3897,13 @@ WalSndSignals(void) pqsignal(SIGTERM, die); /* request shutdown */ /* SIGQUIT handler was already set up by InitPostmasterChild */ InitializeTimeouts(); /* establishes SIGALRM handler */ - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, WalSndLastCycleHandler); /* request a last cycle and * shutdown */ /* Reset some signals that are accepted by postmaster but not here */ - pqsignal(SIGCHLD, SIG_DFL); + pqsignal(SIGCHLD, PG_SIG_DFL); } /* Register shared-memory space needed by walsender */ diff --git a/src/backend/storage/aio/method_worker.c b/src/backend/storage/aio/method_worker.c index a5ccd506d8c..061a93d90d4 100644 --- a/src/backend/storage/aio/method_worker.c +++ b/src/backend/storage/aio/method_worker.c @@ -684,10 +684,10 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len) * Ignore SIGTERM, will get explicit shutdown via SIGUSR2 later in the * shutdown sequence, similar to checkpointer. */ - pqsignal(SIGTERM, SIG_IGN); + pqsignal(SIGTERM, PG_SIG_IGN); /* SIGQUIT handler was already set up by InitPostmasterChild */ - pqsignal(SIGALRM, SIG_IGN); - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGALRM, PG_SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGUSR2, SignalHandlerForShutdownRequest); diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 01f1bd6e687..a8be066afe0 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -2748,11 +2748,11 @@ OpenPipeStream(const char *command, const char *mode) TryAgain: fflush(NULL); - pqsignal(SIGPIPE, SIG_DFL); + pqsignal(SIGPIPE, PG_SIG_DFL); errno = 0; file = popen(command, mode); save_errno = errno; - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); errno = save_errno; if (file != NULL) { diff --git a/src/backend/storage/ipc/waiteventset.c b/src/backend/storage/ipc/waiteventset.c index 0f228e1e7b8..627dba0a842 100644 --- a/src/backend/storage/ipc/waiteventset.c +++ b/src/backend/storage/ipc/waiteventset.c @@ -348,7 +348,7 @@ InitializeWaitEventSupport(void) #ifdef WAIT_USE_KQUEUE /* Ignore SIGURG, because we'll receive it via kqueue. */ - pqsignal(SIGURG, SIG_IGN); + pqsignal(SIGURG, PG_SIG_IGN); #endif } diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index aeaf1c6db8f..2c1f14b7889 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3027,6 +3027,17 @@ die(SIGNAL_ARGS) { InterruptPending = true; ProcDiePending = true; + + /* + * Record who sent the signal. Will be 0 on platforms without + * SA_SIGINFO, which is fine -- ProcessInterrupts() checks for that. + * Only set on the first SIGTERM so we report the original sender. + */ + if (ProcDieSenderPid == 0) + { + ProcDieSenderPid = pg_siginfo->pid; + ProcDieSenderUid = pg_siginfo->uid; + } } /* for the cumulative stats system */ @@ -4316,17 +4327,17 @@ PostgresMain(const char *dbname, const char *username) * returns to outer loop. This seems safer than forcing exit in the * midst of output during who-knows-what operation... */ - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SIG_IGN); + pqsignal(SIGUSR2, PG_SIG_IGN); pqsignal(SIGFPE, FloatExceptionHandler); /* * Reset some signals that are accepted by postmaster but not by * backend */ - pqsignal(SIGCHLD, SIG_DFL); /* system() requires this on some - * platforms */ + pqsignal(SIGCHLD, PG_SIG_DFL); /* system() requires this on some + * platforms */ } /* Early initialization */ diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 509f1114ef6..44a2c7a7c7f 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -2903,10 +2903,10 @@ setup_signals(void) pqsignal(SIGQUIT, trapsig); /* Ignore SIGPIPE when writing to backend, so we can clean up */ - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); /* Prevent SIGSYS so we can probe for kernel calls that might not work */ - pqsignal(SIGSYS, SIG_IGN); + pqsignal(SIGSYS, PG_SIG_IGN); #endif } diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 3cc61455dcb..5539eb8ebef 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -868,7 +868,7 @@ trap_sigint_during_startup(SIGNAL_ARGS) * Clear the signal handler, and send the signal again, to terminate the * process as normal. */ - pqsignal(postgres_signal_arg, SIG_DFL); + pqsignal(postgres_signal_arg, PG_SIG_DFL); raise(postgres_signal_arg); } diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c index a28561fbd84..a7bed5ecccf 100644 --- a/src/bin/pg_dump/parallel.c +++ b/src/bin/pg_dump/parallel.c @@ -568,9 +568,9 @@ sigTermHandler(SIGNAL_ARGS) * signal handler. That could muck up our attempt to send PQcancel, so * disable the signals that set_cancel_handler enabled. */ - pqsignal(SIGINT, SIG_IGN); - pqsignal(SIGTERM, SIG_IGN); - pqsignal(SIGQUIT, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); + pqsignal(SIGTERM, PG_SIG_IGN); + pqsignal(SIGQUIT, PG_SIG_IGN); /* * If we're in the leader, forward signal to all workers. (It seems best @@ -1049,7 +1049,7 @@ ParallelBackupStart(ArchiveHandle *AH) * the workers to inherit this setting, though. */ #ifndef WIN32 - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); #endif /* diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl index 7c21204c1f2..9d966c7bece 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -142,8 +142,11 @@ my ($ret, $out, $err) = $node->psql('postgres', is($ret, 2, 'server crash: psql exit code'); like($out, qr/before/, 'server crash: output before crash'); unlike($out, qr/AFTER/, 'server crash: no output after crash'); +my $detail_re = check_pg_config("#define HAVE_SA_SIGINFO 1") + ? qr/DETAIL: Signal sent by PID \d+, UID \d+\.\n/ + : qr//; like( $err, qr/psql:<stdin>:2: FATAL: terminating connection due to administrator command -(?:DETAIL: Signal sent by PID \d+, UID \d+\.\n)?psql:<stdin>:2: server closed the connection unexpectedly +${detail_re}psql:<stdin>:2: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. psql:<stdin>:2: error: connection to server was lost/, diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index 12d969e8666..f2dd52003c1 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -3024,7 +3024,7 @@ void disable_sigpipe_trap(void) { #ifndef WIN32 - pqsignal(SIGPIPE, SIG_IGN); + pqsignal(SIGPIPE, PG_SIG_IGN); #endif } @@ -3047,7 +3047,7 @@ void restore_sigpipe_trap(void) { #ifndef WIN32 - pqsignal(SIGPIPE, always_ignore_sigpipe ? SIG_IGN : SIG_DFL); + pqsignal(SIGPIPE, always_ignore_sigpipe ? PG_SIG_IGN : PG_SIG_DFL); #endif } diff --git a/src/include/c.h b/src/include/c.h index 88d13ec9993..77ea73cc707 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -1441,17 +1441,26 @@ extern int fdatasync(int fd); #endif /* - * The following is used as the arg list for signal handlers. Any ports - * that take something other than an int argument should override this in - * their pg_config_os.h file. Note that variable names are required - * because it is used in both the prototypes as well as the definitions. - * Note also the long name. We expect that this won't collide with - * other names causing compiler warnings. + * Platform independent struct representing additional information about the + * received signal. If the system does not support the extended information, + * or a field does not apply to the signal, the value is instead reset to the + * documented default value. */ +typedef struct pg_signal_info +{ + pid_t pid; /* pid of sending process or 0 if unknown */ + uid_t uid; /* uid of sending process or 0 if unknown */ +} pg_signal_info; -#ifndef SIGNAL_ARGS -#define SIGNAL_ARGS int postgres_signal_arg -#endif +/* + * The following is used as the arg list for signal handlers. These days we + * use the same argument to all signal handlers and hide the difference + * between platforms in wrapper functions. + * + * SIGNAL_ARGS just exists separately from the pqsignal() definition for + * historical reasons. + */ +#define SIGNAL_ARGS int postgres_signal_arg, pg_signal_info *pg_siginfo /* * When there is no sigsetjmp, its functionality is provided by plain diff --git a/src/include/port.h b/src/include/port.h index 51df9b80e7d..7db476d7b01 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -546,6 +546,9 @@ extern int pg_mkdir_p(char *path, int omode); #else #define pqsignal pqsignal_be #endif + +#define PG_SIG_DFL (pqsigfunc) SIG_DFL +#define PG_SIG_IGN (pqsigfunc) SIG_IGN typedef void (*pqsigfunc) (SIGNAL_ARGS); extern void pqsignal(int signo, pqsigfunc func); diff --git a/src/interfaces/libpq/legacy-pqsignal.c b/src/interfaces/libpq/legacy-pqsignal.c index 1285b033e1b..0735e4ee0d5 100644 --- a/src/interfaces/libpq/legacy-pqsignal.c +++ b/src/interfaces/libpq/legacy-pqsignal.c @@ -36,10 +36,12 @@ * is to ensure that no in-tree code accidentally calls this version.) */ #undef pqsignal -extern pqsigfunc pqsignal(int signo, pqsigfunc func); -pqsigfunc -pqsignal(int signo, pqsigfunc func) +typedef void (*pqsigfunc_legacy) (int postgres_signal_arg); +extern pqsigfunc_legacy pqsignal(int signo, pqsigfunc_legacy func); + +pqsigfunc_legacy +pqsignal(int signo, pqsigfunc_legacy func) { #ifndef WIN32 struct sigaction act, diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c index 8841464b5cb..2b39be99f94 100644 --- a/src/port/pqsignal.c +++ b/src/port/pqsignal.c @@ -63,6 +63,14 @@ #define PG_NSIG (64) /* XXX: wild guess */ #endif +#if !(defined(WIN32) && defined(FRONTEND)) +#define USE_SIGACTION +#endif + +#if defined(USE_SIGACTION) && defined(HAVE_SA_SIGINFO) +#define USE_SIGINFO +#endif + /* Check a couple of common signals to make sure PG_NSIG is accurate. */ StaticAssertDecl(SIGUSR2 < PG_NSIG, "SIGUSR2 >= PG_NSIG"); StaticAssertDecl(SIGHUP < PG_NSIG, "SIGHUP >= PG_NSIG"); @@ -82,19 +90,16 @@ static volatile pqsigfunc pqsignal_handlers[PG_NSIG]; * * This wrapper also handles restoring the value of errno. */ -#if !defined(FRONTEND) && defined(HAVE_SA_SIGINFO) +#ifdef USE_SIGACTION static void -wrapper_handler(int signo, siginfo_t * info, void *context) +wrapper_handler(int postgres_signal_arg, siginfo_t *info, void *context) #else static void -wrapper_handler(SIGNAL_ARGS) +wrapper_handler(int postgres_signal_arg) #endif { int save_errno = errno; -#if !defined(FRONTEND) && defined(HAVE_SA_SIGINFO) - /* SA_SIGINFO signature uses signo, not SIGNAL_ARGS macro */ - int postgres_signal_arg = signo; -#endif + pg_signal_info pg_info; Assert(postgres_signal_arg > 0); Assert(postgres_signal_arg < PG_NSIG); @@ -110,21 +115,32 @@ wrapper_handler(SIGNAL_ARGS) if (unlikely(MyProcPid != (int) getpid())) { - pqsignal(postgres_signal_arg, SIG_DFL); + pqsignal(postgres_signal_arg, PG_SIG_DFL); raise(postgres_signal_arg); return; } +#endif #ifdef HAVE_SA_SIGINFO - if (signo == SIGTERM && info) - { - ProcDieSenderPid = info->si_pid; - ProcDieSenderUid = info->si_uid; - } -#endif + + /* + * If supported by the system, forward interesting information from the + * system's extended signal information to our platform independent + * format. + */ + pg_info.pid = info->si_pid; + pg_info.uid = info->si_uid; +#else + + /* + * Otherwise forward values indicating that we do not have the + * information. + */ + pg_info.pid = 0; + pg_info.uid = 0; #endif - (*pqsignal_handlers[postgres_signal_arg]) (postgres_signal_arg); + (*pqsignal_handlers[postgres_signal_arg]) (postgres_signal_arg, &pg_info); errno = save_errno; } @@ -139,33 +155,44 @@ wrapper_handler(SIGNAL_ARGS) void pqsignal(int signo, pqsigfunc func) { -#if !(defined(WIN32) && defined(FRONTEND)) +#ifdef USE_SIGACTION struct sigaction act; #endif - bool use_wrapper = false; + bool is_ign = func == PG_SIG_IGN; + bool is_dfl = func == PG_SIG_DFL; Assert(signo > 0); Assert(signo < PG_NSIG); - if (func != SIG_IGN && func != SIG_DFL) + /* set up indirection handler */ + if (!(is_ign || is_dfl)) { pqsignal_handlers[signo] = func; /* assumed atomic */ - use_wrapper = true; } -#if !(defined(WIN32) && defined(FRONTEND)) + /* + * Configure system to either ignore/reset the signal handler, or to + * forward it to wrapper_handler. + */ +#ifdef USE_SIGACTION sigemptyset(&act.sa_mask); act.sa_flags = SA_RESTART; -#if !defined(FRONTEND) && defined(HAVE_SA_SIGINFO) - if (use_wrapper) + + if (is_ign) + act.sa_handler = SIG_IGN; + else if (is_dfl) + act.sa_handler = SIG_DFL; + +#ifdef USE_SIGINFO + if (!(is_ign || is_dfl)) { act.sa_sigaction = wrapper_handler; act.sa_flags |= SA_SIGINFO; } else - act.sa_handler = func; #else - act.sa_handler = use_wrapper ? wrapper_handler : func; + else + act.sa_handler = wrapper_handler; #endif #ifdef SA_NOCLDSTOP diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 0c062056982..c26efeba1ee 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -492,7 +492,7 @@ signal_remove_temp(SIGNAL_ARGS) { remove_temp(); - pqsignal(postgres_signal_arg, SIG_DFL); + pqsignal(postgres_signal_arg, PG_SIG_DFL); raise(postgres_signal_arg); } diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index ea95e7984bc..49dfb662abc 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4039,6 +4039,7 @@ pg_sha224_ctx pg_sha256_ctx pg_sha384_ctx pg_sha512_ctx +pg_signal_info pg_snapshot pg_special_case pg_stack_base_t -- 2.43.0
