process: Replace racy SIGCHLD handler with signalfd

Having an asynchronous signal handler for SIGCHLD handling is quite
racy. We would like to be able to print debug messages when reaping
children, but we cannot use printf or other signal unsafe function
calls.

Signalfd is the solution that works. As we already have event loop
with epoll, we can extend it to read the signals via the
descriptor. And there is also one place less for causing potential
race conditions.

Signed-off-by: Timo Kokkonen <timo.t.kokkonen@iki.fi>

diff --git a/process.c b/process.c
index 791a3ed620b7de70ee13edae55c08d5c0b7aa4e0..e226c90d3371355cfffae139db471114ce6d3ee2 100644 (file)
--- a/process.c
+++ b/process.c
@@ -5,6 +5,7 @@
 #include <sys/epoll.h>
 #include <stdio.h>
 #include <sys/wait.h>
+#include <sys/signalfd.h>
 #include <sys/resource.h>
 
 #include "process.h"
@@ -14,6 +15,7 @@ static int child_count;
 static int parent_count;
 static int job_request_fd[2];
 static int job_get_permission_fd[2];
+static int signal_fd;
 static int epoll_fd;
 static unsigned int max_jobs;
 static unsigned int job_count;
@@ -29,41 +31,26 @@ int get_parent_count(void)
        return parent_count;
 }
 
-static void sigchild_handler(int signal)
+static int handle_signals(void)
 {
-       int status;
-
-       waitpid(0, &status, 0);
-}
-
-static int setup_sigchild_handler(void)
-{
-       struct sigaction sa;
+       struct signalfd_siginfo siginfo;
        int ret;
 
-       sa.sa_handler = sigchild_handler;
-       sa.sa_flags = SA_NOCLDSTOP;
-
-       ret = sigaction(SIGCHLD, &sa, NULL);
-       if (ret)
-               pr_err("Failed to setup SIGCHLD handler: %m\n");
-
-       return ret;
-}
-
-static int cancel_sigchild_handler(void)
-{
-       struct sigaction sa;
-       int ret;
+       ret = read(signal_fd, &siginfo, sizeof(siginfo));
+       if (ret < sizeof(siginfo)) {
+               pr_err("Expected %zd from read, got %d: %m\n",
+                       sizeof(siginfo), ret);
+               return -1;
+       }
 
-       sa.sa_handler = SIG_DFL;
-       sa.sa_flags = SA_NOCLDSTOP;
+       if (siginfo.ssi_signo != SIGCHLD) {
+               pr_err("Unexpected signal %d, ignoring\n", siginfo.ssi_signo);
+               return -1;
+       }
 
-       ret = sigaction(SIGCHLD, &sa, NULL);
-       if (ret)
-               pr_err("Failed to cancel SIGCHLD handler: %m\n");
+       harvest_zombies(siginfo.ssi_pid);
 
-       return ret;
+       return 0;
 }
 
 /*
@@ -78,6 +65,7 @@ int init_jobcontrol(int max_jobs_requested)
        struct epoll_event ev;
        FILE *file;
        int ret;
+       sigset_t sigmask;
        char buf[256];
        char match[8];
 
@@ -91,10 +79,6 @@ int init_jobcontrol(int max_jobs_requested)
                return -1;
        }
 
-       ret = setup_sigchild_handler();
-       if (ret)
-               return ret;
-
        epoll_fd = epoll_create(1);
        if (epoll_fd == -1) {
                pr_err("Failed to epoll_create(): %m\n");
@@ -109,6 +93,29 @@ int init_jobcontrol(int max_jobs_requested)
                return -1;
        }
 
+       sigemptyset(&sigmask);
+       sigaddset(&sigmask, SIGCHLD);
+
+       /* Block SIGCHLD so that it becomes readable via signalfd */
+       ret = sigprocmask(SIG_BLOCK, &sigmask, NULL);
+       if (ret < 0) {
+               pr_err("Failed to sigprocmask: %m\n");
+       }
+
+       signal_fd = signalfd(-1, &sigmask, SFD_CLOEXEC);
+       if (signal_fd < 0) {
+               pr_err("Failed to create signal_fd: %m\n");
+               return -1;
+       }
+
+       ev.data.fd = signal_fd;
+       ret = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, signal_fd, &ev);
+       if (ret) {
+               pr_err("Failed to add epoll_fd: %m\n");
+               return -1;
+       }
+
+
        if (max_jobs_requested > 0) {
                max_jobs = max_jobs_requested;
                goto no_count_cpus;
@@ -235,6 +242,8 @@ int poll_job_requests(int timeout)
 
        if (event.data.fd == job_request_fd[0])
                handle_job_request();
+       else if (event.data.fd == signal_fd)
+               handle_signals();
        else
                pr_err("Unknown fd: %d\n", event.data.fd);
 
@@ -266,13 +275,6 @@ int do_fork(void)
                return child;
        }
 
-       /*
-        * Child processes may want to use waitpid() for synchronizing
-        * with their sub-childs. Disable the signal handler by
-        * default
-        */
-       cancel_sigchild_handler();
-
        /*
         * Also do not notify the master parent the death of this
         * child. Only childs that have been created with