From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 29 Aug 2015 16:09:37 +0000 (+0200)
Subject: Improve signal handling
X-Git-Tag: v3.2.4~12
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=737c96fabeab9d66407758b848468aa49190c946;p=thirdparty%2Fccache.git

Improve signal handling

* Block signals while manipulating data read by the signal handler.
* If ccache receives SIGTERM, kill compiler as well. (Other signals like
  SIGINT are sent to the whole process group.)
* Die appropriately at the end of the signal handler by resending the
  signal instead of exiting.
---

diff --git a/NEWS.txt b/NEWS.txt
index 257d71b9f..f979e708f 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -14,9 +14,8 @@ Bug fixes
 - Made conversion-to-bool explicit to avoid build warnings (and potential
   runtime errors) on legacy systems.
 
-- Wait for compiler subprocess (if any) to exit before exiting from a signal.
-  This should fix a race condition (albeit a quite unlikely one) related to
-  removal of the compiler output file(s).
+- Improved signal handling: Kill compiler on SIGTERM; wait for compiler to exit
+  before exiting; die appropriately.
 
 
 ccache 3.2.3
diff --git a/ccache.c b/ccache.c
index 8012f96b7..1dbbc7efe 100644
--- a/ccache.c
+++ b/ccache.c
@@ -239,6 +239,8 @@ struct pending_tmp_file {
 /* Temporary files to remove at program exit. */
 static struct pending_tmp_file *pending_tmp_files = NULL;
 
+static sigset_t fatal_signal_set;
+
 /* PID of currently executing compiler that we have started, if any. 0 means no
  * ongoing compilation. */
 static pid_t compiler_pid = 0;
@@ -318,17 +320,35 @@ temp_dir()
 	return path;
 }
 
+void
+block_signals(void)
+{
+	sigprocmask(SIG_BLOCK, &fatal_signal_set, NULL);
+}
+
+void
+unblock_signals(void)
+{
+	sigset_t empty;
+	sigemptyset(&empty);
+	sigprocmask(SIG_SETMASK, &empty, NULL);
+}
+
 static void
 add_pending_tmp_file(const char *path)
 {
-	struct pending_tmp_file *e = x_malloc(sizeof(*e));
+	struct pending_tmp_file *e;
+
+	block_signals();
+	e = x_malloc(sizeof(*e));
 	e->path = x_strdup(path);
 	e->next = pending_tmp_files;
 	pending_tmp_files = e;
+	unblock_signals();
 }
 
 static void
-clean_up_pending_tmp_files(void)
+do_clean_up_pending_tmp_files(void)
 {
 	struct pending_tmp_file *p = pending_tmp_files;
 	while (p) {
@@ -341,16 +361,72 @@ clean_up_pending_tmp_files(void)
 }
 
 static void
-signal_handler(int signo)
+clean_up_pending_tmp_files(void)
 {
-	int status;
-	(void)signo;
-	clean_up_pending_tmp_files();
+	block_signals();
+	do_clean_up_pending_tmp_files();
+	unblock_signals();
+}
+
+static void
+signal_handler(int signum)
+{
+	/* Unregister handler for this signal so that we can send the signal to
+	 * ourselves at the end of the handler. */
+	signal(signum, SIG_DFL);
+
+	/* If ccache was killed explicitly, then bring the compiler subprocess (if
+	 * any) with us as well. */
+	if (signum == SIGTERM
+	    && compiler_pid != 0
+	    && waitpid(compiler_pid, NULL, WNOHANG) == 0) {
+		kill(compiler_pid, signum);
+	}
+
+	do_clean_up_pending_tmp_files();
+
 	if (compiler_pid != 0) {
 		/* Wait for compiler subprocess to exit before we snuff it. */
-		waitpid(compiler_pid, &status, 0);
+		waitpid(compiler_pid, NULL, 0);
 	}
-	_exit(1);
+
+	/* Resend signal to ourselves to exit properly after returning from the
+	 * handler. */
+	kill(getpid(), signum);
+}
+
+static void
+register_signal_handler(int signum)
+{
+	struct sigaction act;
+	memset(&act, 0, sizeof(act));
+	act.sa_handler = signal_handler;
+	act.sa_mask = fatal_signal_set;
+	act.sa_flags = SA_RESTART;
+	sigaction(signum, &act, NULL);
+}
+
+static void
+set_up_signal_handlers(void)
+{
+	sigemptyset(&fatal_signal_set);
+	sigaddset(&fatal_signal_set, SIGINT);
+	sigaddset(&fatal_signal_set, SIGTERM);
+#ifdef SIGHUP
+	sigaddset(&fatal_signal_set, SIGHUP);
+#endif
+#ifdef SIGQUIT
+	sigaddset(&fatal_signal_set, SIGQUIT);
+#endif
+
+	register_signal_handler(SIGINT);
+	register_signal_handler(SIGTERM);
+#ifdef SIGHUP
+	register_signal_handler(SIGHUP);
+#endif
+#ifdef SIGQUIT
+	register_signal_handler(SIGQUIT);
+#endif
 }
 
 static void
@@ -2867,17 +2943,13 @@ ccache(int argc, char *argv[])
 	/* Arguments to send to the real compiler. */
 	struct args *compiler_args;
 
+	set_up_signal_handlers();
+
 	orig_args = args_init(argc, argv);
 
 	initialize();
 	find_compiler(argv);
 
-#ifndef _WIN32
-	signal(SIGHUP, signal_handler);
-#endif
-	signal(SIGINT, signal_handler);
-	signal(SIGTERM, signal_handler);
-
 	if (str_eq(conf->temporary_dir, "")) {
 		clean_up_internal_tempdir();
 	}
diff --git a/ccache.h b/ccache.h
index d3ecf6c0c..ef18b25cb 100644
--- a/ccache.h
+++ b/ccache.h
@@ -227,6 +227,8 @@ void lockfile_release(const char *path);
 /* ccache.c */
 
 extern time_t time_of_compilation;
+void block_signals(void);
+void unblock_signals(void);
 bool cc_process_args(struct args *args, struct args **preprocessor_args,
                     struct args **compiler_args);
 void cc_reset(void);
diff --git a/execute.c b/execute.c
index 109d60e09..608a8f46f 100644
--- a/execute.c
+++ b/execute.c
@@ -228,7 +228,11 @@ execute(char **argv, int fd_out, int fd_err, pid_t *pid)
 	int status;
 
 	cc_log_argv("Executing ", argv);
+
+	block_signals();
 	*pid = fork();
+	unblock_signals();
+
 	if (*pid == -1) {
 		fatal("Failed to fork: %s", strerror(errno));
 	}
@@ -248,7 +252,10 @@ execute(char **argv, int fd_out, int fd_err, pid_t *pid)
 	if (waitpid(*pid, &status, 0) != *pid) {
 		fatal("waitpid failed: %s", strerror(errno));
 	}
+
+	block_signals();
 	*pid = 0;
+	unblock_signals();
 
 	if (WEXITSTATUS(status) == 0 && WIFSIGNALED(status)) {
 		return -1;