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 257d71b9f252a3d356a0b68114c817155b281b12..f979e708f8be86ceb25653567597b5ce95642d47 100644 (file)
--- 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 8012f96b7473bb431cc2bb72615f2c802ff505c3..1dbbc7efe0ff6ec3e51c449d5ddd028a91714efa 100644 (file)
--- 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 d3ecf6c0c0c42c56d8e832e18f5fa0e25fef2ef3..ef18b25cbe4c9de49a3d524e6f912c148b2238c9 100644 (file)
--- 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 109d60e093df085288b631a5fea13fc3e796add1..608a8f46f1bc375a1779075f822f0807041ee7a1 100644 (file)
--- 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;