Merge pull request #2419 from pi-hole/tweak/prevent_restart_during_gravity_run

Do not restart FTL while `pihole -g` is still ongoing

Author: DL6ER

src/api/action.c

@@ -22,6 +22,8 @@
 // flush_network_table()
 #include "database/network-table.h"
 #include "config/config.h"
+// gravity_running
+#include "daemon.h"
 
 static int run_and_stream_command(struct ftl_conn *api, const char *path, const char *const args[], const char *extra_env)
 {
@@ -57,6 +59,18 @@ static int run_and_stream_command(struct ftl_conn *api, const char *path, const
 		if(extra_env != NULL)
 			setenv(extra_env, "1", 1);
 
+		// Detach child into its own session/process group so signals
+		// sent to the parent's process group (like SIGTERM) do not
+		// propagate to this child.
+		(void)setsid();
+
+		// Ignore SIGTERM so systemd's cgroup-level kill (the
+		// default KillMode=control-group sends SIGTERM to ALL
+		// processes in the cgroup) doesn't terminate gravity
+		// mid-run. SIG_IGN is preserved across execv(), unlike
+		// custom handlers which are reset to SIG_DFL.
+		signal(SIGTERM, SIG_IGN);
+
 		// Run pihole -g
 		execv(path, (char *const *)args);
 
@@ -129,7 +143,16 @@ int api_action_gravity(struct ftl_conn *api)
 		get_bool_var(query, "color", &color);
 
 	const char *extra_env = color ? "FORCE_COLOR" : NULL;
-	return run_and_stream_command(api, "/usr/local/bin/pihole", (const char *const []){ "pihole", "-g", NULL }, extra_env);
+
+	gravity_running = 1;
+	const int ret = run_and_stream_command(api, "/usr/local/bin/pihole", (const char *const []){ "pihole", "-g", NULL }, extra_env);
+	gravity_running = 0;
+
+	// If a termination/restart was requested while gravity was running,
+	// act on it now rather than waiting up to ~1s for the GC thread to pick it up
+	check_if_want_terminate();
+
+	return ret;
 }
 
 int api_action_restartDNS(struct ftl_conn *api)

src/daemon.c

@@ -47,6 +47,8 @@
 pthread_t threads[THREADS_MAX] = { 0 };
 bool resolver_ready = false;
 bool dnsmasq_failed = false;
+volatile sig_atomic_t gravity_running = 0;
+volatile sig_atomic_t want_terminate = 0;
 
 void go_daemon(void)
 {

src/daemon.h

@@ -56,5 +56,7 @@ ssize_t getrandom_fallback(void *buf, size_t buflen, unsigned int flags);
 
 extern bool resolver_ready;
 extern bool dnsmasq_failed;
+extern volatile sig_atomic_t gravity_running;
+extern volatile sig_atomic_t want_terminate;
 
 #endif //DAEMON_H

src/gc.c

@@ -721,6 +721,14 @@ void *GC_thread(void *val)
 			set_dnsmasq_debug(is_debugged, dpid);
 		}
 
+		// Intermediate cancellation-point
+		if(killed)
+			break;
+
+		// Check if we need to terminate/restart FTL but this has been
+		// postponed because of an ongoing gravity run
+		check_if_want_terminate();
+
 		// Sleep for the remaining time of the interval (if any)
 		const double time_end = double_time();
 		const double time_diff = time_end - time_start;

src/signals.c

@@ -234,7 +234,10 @@ const char * const thread_names[THREADS_MAX] = {
 	"ntp-server4",
 	"ntp-server6",
 	"webserver",
- };
+};
+
+// Private prototypes
+static void terminate(void);
 
 // Return the (null-terminated) name of the calling thread
 // The name is stored in the buffer as well as returned for convenience
@@ -577,6 +580,38 @@ static void SIGTERM_handler(int signum, siginfo_t *si, void *context)
 	log_info("Asked to terminate by \"%s\" (PID %ld, user %s UID %ld)",
 	         kill_name, (long int)kill_pid, kill_user, (long int)kill_uid);
 
+	// Check if we can terminate
+	want_terminate = true;
+	check_if_want_terminate();
+}
+
+// Checks if the program should terminate or not
+static time_t last_term_warning = 0;
+void check_if_want_terminate(void)
+{
+	if(!want_terminate)
+		// We are not asked to terminate
+		return;
+
+	// Return early if we are not allowed to terminate
+	if(gravity_running)
+	{
+		// Only log once every 30 seconds or if any debugging is enabled
+		if(time(NULL) - last_term_warning > 30 || debug_flags[DEBUG_ANY])
+		{
+			log_info("Not terminating as gravity is still running...");
+			last_term_warning = time(NULL);
+		}
+		return;
+	}
+
+	// Terminate if gravity is not running
+	terminate();
+}
+
+// Terminates the DNS service by signaling or marking it as failed
+static void terminate(void)
+{
 	// Terminate dnsmasq to stop DNS service
 	if(!dnsmasq_failed)
 	{

src/signals.h

@@ -21,6 +21,7 @@
 void handle_signals(void);
 void handle_realtime_signals(void);
 pid_t main_pid(void);
+void check_if_want_terminate(void);
 void thread_sleepms(const enum thread_types thread, const int milliseconds);
 void init_backtrace(const char *argv0);
 void generate_backtrace(void);