On 2026-04-07 Tu 10:55 AM, Andres Freund wrote:
This seems completely wrong from a layering POV. The wrapper has no business
whatsoever to know that how SIGTERM is interpreted and thus no business
setting variables like ProcDieSenderPid.
Pretty sure have some sigterm handlers that shouldn't set ProcDieSenderPid.
A more correct answer here would be to forward information about the sender of
a signal to the signal handlers and let them interpret the information if
available.
OK, fair points. Does the attached meet your concerns?
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
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/tcop/postgres.c b/src/backend/tcop/postgres.c
index 4fb18741dc5..e02125e720e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3025,6 +3025,8 @@ die(SIGNAL_ARGS)
{
InterruptPending = true;
ProcDiePending = true;
+ ProcDieSenderPid = SignalSenderPid;
+ ProcDieSenderUid = SignalSenderUid;
}
/* for the cumulative stats system */
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index ba8cd99d98d..bc517b009cb 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -43,6 +43,8 @@ volatile sig_atomic_t IdleStatsUpdateTimeoutPending = false;
volatile uint32 InterruptHoldoffCount = 0;
volatile uint32 QueryCancelHoldoffCount = 0;
volatile uint32 CritSectionCount = 0;
+volatile int SignalSenderPid = 0;
+volatile int SignalSenderUid = 0;
volatile int ProcDieSenderPid = 0;
volatile int ProcDieSenderUid = 0;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b6f625e10a6..655b42103ca 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -90,6 +90,8 @@
extern PGDLLIMPORT volatile sig_atomic_t InterruptPending;
extern PGDLLIMPORT volatile sig_atomic_t QueryCancelPending;
extern PGDLLIMPORT volatile sig_atomic_t ProcDiePending;
+extern PGDLLIMPORT volatile int SignalSenderPid;
+extern PGDLLIMPORT volatile int SignalSenderUid;
extern PGDLLIMPORT volatile int ProcDieSenderPid;
extern PGDLLIMPORT volatile int ProcDieSenderUid;
extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending;
diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c
index 8841464b5cb..07985e704d5 100644
--- a/src/port/pqsignal.c
+++ b/src/port/pqsignal.c
@@ -84,17 +84,13 @@ static volatile pqsigfunc pqsignal_handlers[PG_NSIG];
*/
#if !defined(FRONTEND) && defined(HAVE_SA_SIGINFO)
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)
#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
Assert(postgres_signal_arg > 0);
Assert(postgres_signal_arg < PG_NSIG);
@@ -116,10 +112,21 @@ wrapper_handler(SIGNAL_ARGS)
}
#ifdef HAVE_SA_SIGINFO
- if (signo == SIGTERM && info)
+ /*
+ * Record the signal sender's PID and UID so that individual signal
+ * handlers can use them if they wish. These are only valid for the
+ * duration of the handler call and will be overwritten by the next
+ * signal.
+ */
+ if (info)
{
- ProcDieSenderPid = info->si_pid;
- ProcDieSenderUid = info->si_uid;
+ SignalSenderPid = info->si_pid;
+ SignalSenderUid = info->si_uid;
+ }
+ else
+ {
+ SignalSenderPid = 0;
+ SignalSenderUid = 0;
}
#endif
#endif
