From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Mon, 13 May 2024 15:45:07 +0000 (-0600)
Subject: More predictable signal handling
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2113bca9efcca3924175f44e8d287fb8d91305e7;p=thirdparty%2Ffreeradius-server.git

More predictable signal handling
---

diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c
index 8c7270d9262..903d40a4e5c 100644
--- a/src/bin/radiusd.c
+++ b/src/bin/radiusd.c
@@ -901,7 +901,10 @@ int main(int argc, char *argv[])
 	 *  what to do, so we might as well do the default, and die.
 	 */
 #ifdef SIGPIPE
-	signal(SIGPIPE, SIG_IGN);
+	if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) {
+		ERROR("Failed ignoring SIGPIPE: %s", fr_syserror(errno));
+		goto cleanup;
+	}
 #endif
 
 	if (fr_set_signal(SIGHUP, sig_hup) < 0) goto set_signal_error;
@@ -988,7 +991,10 @@ int main(int argc, char *argv[])
 	/*
 	 *  Ignore the TERM signal: we're about to die.
 	 */
-	signal(SIGTERM, SIG_IGN);
+	if (unlikely(signal(SIGTERM, SIG_IGN) == SIG_ERR)) {
+		ERROR("Failed blocking SIGTERM, we may receive spurious signals: %s",
+		      fr_syserror(errno));
+	}
 
 	/*
 	 *  Unprotect global memory
@@ -1060,20 +1066,8 @@ int main(int argc, char *argv[])
 	 *  processes created by the exec code or triggers.
 	 */
 	if (config->spawn_workers) {
-		sigset_t mask;
-
 		INFO("All threads have exited, sending SIGTERM to remaining children");
 
-		/*
-		 *	If we believe the POSIX documentation, sending
-		 *	a kill to a negative PGID could also signal us,
-		 *	so block the signal first, we're already exiting,
-		 *	so it doesn't matter if we don't get the signal.
-		 */
-		sigemptyset(&mask);
-		sigaddset(&mask, SIGTERM);
-		sigprocmask(SIG_BLOCK, &mask, NULL);
-
 		/*
 		 *	If pid is negative, but not -1, sig
 		 *	shall be sent to all processes
diff --git a/src/lib/io/schedule.c b/src/lib/io/schedule.c
index eb4ec47bf02..abdbe8cbf8d 100644
--- a/src/lib/io/schedule.c
+++ b/src/lib/io/schedule.c
@@ -172,6 +172,16 @@ static void *fr_schedule_worker_thread(void *arg)
 	fr_schedule_child_status_t	status = FR_CHILD_FAIL;
 	fr_schedule_network_t		*sn;
 	char				worker_name[32];
+	sigset_t			sigset;
+
+	sigfillset(&sigset);
+
+	/*
+	 *	Ensure workers aren't interrupted by signals.
+	 *	The main thread, and main event loop are mostly
+	 *	idle, so they can handle signals.
+	 */
+	pthread_sigmask(SIG_BLOCK, &sigset, NULL);
 
 	worker_id = sw->id;		/* Store the current worker ID */
 
@@ -293,6 +303,16 @@ static void *fr_schedule_network_thread(void *arg)
 	fr_schedule_child_status_t	status = FR_CHILD_FAIL;
 	fr_event_list_t			*el;
 	char				network_name[32];
+	sigset_t			sigset;
+
+	sigfillset(&sigset);
+
+	/*
+	 *	Ensure network threads aren't interrupted by
+	 *	signals. The main thread, and main event loop
+	 *	are mostly idle, so they can handle signals.
+	 */
+	pthread_sigmask(SIG_BLOCK, &sigset, NULL);
 
 	snprintf(network_name, sizeof(network_name), "Network %d", sn->id);