ipcpd, irmd: Handle signal in main thread

The signals are now handled in the main thread instead of an
asynchronous signal handler. The acceptloop is now correctly cancelled
and the associated timeouts are removed.

Signed-off-by: Dimitri Staessens <[email protected]>
Signed-off-by: Sander Vrijders <[email protected]>

6 files changed, 95 insertions, 140 deletions

diff --git a/src/ipcpd/CMakeLists.txt b/src/ipcpd/CMakeLists.txt
index 3fa3bb0f..1c6f6ae9 100644
--- a/src/ipcpd/CMakeLists.txt
+++ b/src/ipcpd/CMakeLists.txt
@@ -1,5 +1,3 @@
-set(IPCP_ACCEPT_TIMEOUT 100 CACHE STRING
-  "Timeout for accept in IPCP mainloop threads (ms)")
 set(IPCP_QOS_CUBE_BE_PRIO 50 CACHE STRING
   "Priority for best effort QoS cube (0-99)")
 set(IPCP_QOS_CUBE_VIDEO_PRIO 90 CACHE STRING
diff --git a/src/ipcpd/config.h.in b/src/ipcpd/config.h.in
index f7af0808..43307d87 100644
--- a/src/ipcpd/config.h.in
+++ b/src/ipcpd/config.h.in
@@ -26,8 +26,6 @@
 #define PROG_RES_FDS        @PROG_RES_FDS@
 #define PROG_MAX_FLOWS      @PROG_MAX_FLOWS@
 
-#define IPCP_ACCEPT_TIMEOUT @IPCP_ACCEPT_TIMEOUT@
-
 #define SOCKET_TIMEOUT      @SOCKET_TIMEOUT@
 #define CONNECT_TIMEOUT     @CONNECT_TIMEOUT@
 
diff --git a/src/ipcpd/ipcp.c b/src/ipcpd/ipcp.c
index f8df5640..8640bf6e 100644
--- a/src/ipcpd/ipcp.c
+++ b/src/ipcpd/ipcp.c
@@ -71,29 +71,6 @@ struct cmd {
         int              fd;
 };
 
-static void ipcp_sig_handler(int         sig,
-                             siginfo_t * info,
-                             void *      c)
-{
-        (void) c;
-
-        switch(sig) {
-        case SIGINT:
-        case SIGTERM:
-        case SIGHUP:
-        case SIGQUIT:
-                if (info->si_pid == ipcpi.irmd_pid) {
-                        if (ipcp_get_state() == IPCP_INIT)
-                                ipcp_set_state(IPCP_NULL);
-
-                        if (ipcp_get_state() == IPCP_OPERATIONAL)
-                                ipcp_set_state(IPCP_SHUTDOWN);
-                }
-        default:
-                return;
-        }
-}
-
 uint8_t * ipcp_hash_dup(const uint8_t * hash)
 {
         uint8_t * dup = malloc(hash_len(ipcpi.dir_hash_algo));
@@ -120,28 +97,23 @@ void ipcp_hash_str(char *          buf,
         buf[2 * i] = '\0';
 }
 
+static void close_ptr(void * o)
+{
+        close(*((int *) o));
+}
+
 static void * acceptloop(void * o)
 {
         int            csockfd;
         struct timeval tv = {(SOCKET_TIMEOUT / 1000),
                              (SOCKET_TIMEOUT % 1000) * 1000};
-#if defined(__FreeBSD__) || defined(__APPLE__)
-        fd_set         fds;
-        struct timeval timeout = {(IPCP_ACCEPT_TIMEOUT / 1000),
-                                  (IPCP_ACCEPT_TIMEOUT % 1000) * 1000};
-#endif
+
         (void) o;
 
         while (ipcp_get_state() != IPCP_SHUTDOWN &&
                ipcp_get_state() != IPCP_NULL) {
                 struct cmd * cmd;
 
-#if defined(__FreeBSD__) || defined(__APPLE__)
-                FD_ZERO(&fds);
-                FD_SET(ipcpi.sockfd, &fds);
-                if (select(ipcpi.sockfd + 1, &fds, NULL, NULL, &timeout) <= 0)
-                        continue;
-#endif
                 csockfd = accept(ipcpi.sockfd, 0, 0);
                 if (csockfd < 0)
                         continue;
@@ -157,7 +129,14 @@ static void * acceptloop(void * o)
                         break;
                 }
 
+                pthread_cleanup_push(close_ptr, &csockfd);
+                pthread_cleanup_push(free, cmd);
+
                 cmd->len = read(csockfd, cmd->cbuf, SOCK_BUF_SIZE);
+
+                pthread_cleanup_pop(false);
+                pthread_cleanup_pop(false);
+
                 if (cmd->len <= 0) {
                         log_err("Failed to read from socket.");
                         close(csockfd);
@@ -179,11 +158,6 @@ static void * acceptloop(void * o)
         return (void *) 0;
 }
 
-static void close_ptr(void * o)
-{
-        close(*((int *) o));
-}
-
 static void free_msg(void * o)
 {
         ipcp_msg__free_unpacked((ipcp_msg_t *) o, NULL);
@@ -572,8 +546,6 @@ int ipcp_init(int               argc,
 {
         bool               log;
         pthread_condattr_t cattr;
-        struct timeval     tv  = {(IPCP_ACCEPT_TIMEOUT / 1000),
-                                  (IPCP_ACCEPT_TIMEOUT % 1000) * 1000};
         int                ret = -1;
 
         if (parse_args(argc, argv, &log))
@@ -594,10 +566,6 @@ int ipcp_init(int               argc,
                 goto fail_serv_sock;
         }
 
-        if (setsockopt(ipcpi.sockfd, SOL_SOCKET, SO_RCVTIMEO,
-                       (void *) &tv, sizeof(tv)))
-                log_warn("Failed to set timeout on socket.");
-
         ipcpi.ops = ops;
 
         if (pthread_mutex_init(&ipcpi.state_mtx, NULL)) {
@@ -668,7 +636,6 @@ int ipcp_init(int               argc,
 
 int ipcp_boot()
 {
-        struct sigaction sig_act;
         sigset_t  sigset;
         sigemptyset(&sigset);
         sigaddset(&sigset, SIGINT);
@@ -676,18 +643,6 @@ int ipcp_boot()
         sigaddset(&sigset, SIGHUP);
         sigaddset(&sigset, SIGPIPE);
 
-        /* init sig_act */
-        memset(&sig_act, 0, sizeof(sig_act));
-
-        /* install signal traps */
-        sig_act.sa_sigaction = &ipcp_sig_handler;
-        sig_act.sa_flags     = SA_SIGINFO;
-
-        sigaction(SIGINT,  &sig_act, NULL);
-        sigaction(SIGTERM, &sig_act, NULL);
-        sigaction(SIGHUP,  &sig_act, NULL);
-        sigaction(SIGPIPE, &sig_act, NULL);
-
         ipcpi.tpm = tpm_create(IPCP_MIN_THREADS, IPCP_ADD_THREADS,
                                mainloop, NULL);
         if (ipcpi.tpm == NULL)
@@ -706,8 +661,6 @@ int ipcp_boot()
                 goto fail_acceptor;
         }
 
-        pthread_sigmask(SIG_UNBLOCK, &sigset, NULL);
-
         return 0;
 
  fail_acceptor:
@@ -720,6 +673,45 @@ int ipcp_boot()
 
 void ipcp_shutdown()
 {
+        siginfo_t info;
+        sigset_t  sigset;
+
+        sigemptyset(&sigset);
+        sigaddset(&sigset, SIGINT);
+        sigaddset(&sigset, SIGQUIT);
+        sigaddset(&sigset, SIGHUP);
+        sigaddset(&sigset, SIGTERM);
+        sigaddset(&sigset, SIGPIPE);
+
+        while(ipcp_get_state() != IPCP_NULL &&
+              ipcp_get_state() != IPCP_SHUTDOWN) {
+                if (sigwaitinfo(&sigset, &info) < 0) {
+                        log_warn("Bad signal.");
+                        continue;
+                }
+
+                switch(info.si_signo) {
+                case SIGINT:
+                case SIGTERM:
+                case SIGHUP:
+                case SIGQUIT:
+                        if (info.si_pid == ipcpi.irmd_pid) {
+                                if (ipcp_get_state() == IPCP_INIT)
+                                        ipcp_set_state(IPCP_NULL);
+
+                                if (ipcp_get_state() == IPCP_OPERATIONAL)
+                                        ipcp_set_state(IPCP_SHUTDOWN);
+                        }
+                        break;
+                case SIGPIPE:
+                        log_dbg("Ignored SIGPIPE.");
+                default:
+                        continue;
+                }
+        }
+
+        pthread_cancel(ipcpi.acceptor);
+
         pthread_join(ipcpi.acceptor, NULL);
         tpm_stop(ipcpi.tpm);
         tpm_destroy(ipcpi.tpm);
diff --git a/src/irmd/CMakeLists.txt b/src/irmd/CMakeLists.txt
index 458e1efa..59d0d103 100644
--- a/src/irmd/CMakeLists.txt
+++ b/src/irmd/CMakeLists.txt
@@ -4,8 +4,6 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR})
 include_directories(${CMAKE_SOURCE_DIR}/include)
 include_directories(${CMAKE_BINARY_DIR}/include)
 
-set(IRMD_ACCEPT_TIMEOUT 100 CACHE STRING
-  "Timeout for accept in IRMD mainloop threads (ms)")
 set(IRMD_REQ_ARR_TIMEOUT 500 CACHE STRING
   "Timeout for an application to respond to a new flow (ms)")
 set(IRMD_FLOW_TIMEOUT 5000 CACHE STRING
diff --git a/src/irmd/config.h.in b/src/irmd/config.h.in
index 0ac961f8..a4c7128e 100644
--- a/src/irmd/config.h.in
+++ b/src/irmd/config.h.in
@@ -33,7 +33,6 @@
 
 #define SOCKET_TIMEOUT          @SOCKET_TIMEOUT@
 
-#define IRMD_ACCEPT_TIMEOUT     @IRMD_ACCEPT_TIMEOUT@
 #define IRMD_REQ_ARR_TIMEOUT    @IRMD_REQ_ARR_TIMEOUT@
 #define IRMD_FLOW_TIMEOUT       @IRMD_FLOW_TIMEOUT@
 
diff --git a/src/irmd/main.c b/src/irmd/main.c
index d3f7a09e..673e47db 100644
--- a/src/irmd/main.c
+++ b/src/irmd/main.c
@@ -1661,32 +1661,6 @@ static void irm_fini(void)
 #endif
 }
 
-void irmd_sig_handler(int         sig,
-                      siginfo_t * info,
-                      void *      c)
-{
-        (void) info;
-        (void) c;
-
-        switch(sig) {
-        case SIGINT:
-        case SIGTERM:
-        case SIGHUP:
-                if (irmd_get_state() == IRMD_NULL) {
-                        log_info("Patience is bitter, but its fruit is sweet.");
-                        return;
-                }
-
-                log_info("IRMd shutting down...");
-                irmd_set_state(IRMD_NULL);
-                break;
-        case SIGPIPE:
-                log_dbg("Ignored SIGPIPE.");
-        default:
-                return;
-        }
-}
-
 void * irm_sanitize(void * o)
 {
         struct timespec now;
@@ -1808,27 +1782,22 @@ void * irm_sanitize(void * o)
         }
 }
 
+static void close_ptr(void * o)
+{
+        close(*((int *) o));
+}
+
 static void * acceptloop(void * o)
 {
         int            csockfd;
         struct timeval tv = {(SOCKET_TIMEOUT / 1000),
                              (SOCKET_TIMEOUT % 1000) * 1000};
-#if defined(__FreeBSD__) || defined(__APPLE__)
-        fd_set         fds;
-        struct timeval timeout = {(IRMD_ACCEPT_TIMEOUT / 1000),
-                                  (IRMD_ACCEPT_TIMEOUT % 1000) * 1000};
-#endif
+
         (void) o;
 
         while (irmd_get_state() == IRMD_RUNNING) {
                 struct cmd * cmd;
 
-#if defined(__FreeBSD__) || defined(__APPLE__)
-                FD_ZERO(&fds);
-                FD_SET(irmd.sockfd, &fds);
-                if (select(irmd.sockfd + 1, &fds, NULL, NULL, &timeout) <= 0)
-                        continue;
-#endif
                 csockfd = accept(irmd.sockfd, 0, 0);
                 if (csockfd < 0)
                         continue;
@@ -1844,7 +1813,14 @@ static void * acceptloop(void * o)
                         break;
                 }
 
+                pthread_cleanup_push(close_ptr, &csockfd);
+                pthread_cleanup_push(free, cmd);
+
                 cmd->len = read(csockfd, cmd->cbuf, SOCK_BUF_SIZE);
+
+                pthread_cleanup_pop(false);
+                pthread_cleanup_pop(false);
+
                 if (cmd->len <= 0) {
                         log_err("Failed to read from socket.");
                         close(csockfd);
@@ -1866,11 +1842,6 @@ static void * acceptloop(void * o)
         return (void *) 0;
 }
 
-static void close_ptr(void * o)
-{
-        close(*((int *) o));
-}
-
 static void free_msg(void * o)
 {
         irm_msg__free_unpacked((irm_msg_t *) o, NULL);
@@ -2091,8 +2062,6 @@ static void * mainloop(void * o)
 static int irm_init(void)
 {
         struct stat        st;
-        struct timeval     timeout = {(IRMD_ACCEPT_TIMEOUT / 1000),
-                                      (IRMD_ACCEPT_TIMEOUT % 1000) * 1000};
         pthread_condattr_t cattr;
 
         memset(&st, 0, sizeof(st));
@@ -2185,15 +2154,9 @@ static int irm_init(void)
                 goto fail_sock_path;
         }
 
-        if (setsockopt(irmd.sockfd, SOL_SOCKET, SO_RCVTIMEO,
-                       (char *) &timeout, sizeof(timeout)) < 0) {
-                log_err("Failed setting socket option.");
-                goto fail_sock_opt;
-        }
-
         if (chmod(IRM_SOCK_PATH, 0666)) {
                 log_err("Failed to chmod socket.");
-                goto fail_sock_opt;
+                goto fail_sock_path;
         }
 
         if ((irmd.rdrb = shm_rdrbuff_create()) == NULL) {
@@ -2225,8 +2188,6 @@ static int irm_init(void)
         shm_rdrbuff_destroy(irmd.rdrb);
 #endif
  fail_rdrbuff:
-        shm_rdrbuff_destroy(irmd.rdrb);
- fail_sock_opt:
         close(irmd.sockfd);
  fail_sock_path:
         unlink(IRM_SOCK_PATH);
@@ -2259,14 +2220,15 @@ static void usage(void)
 int main(int     argc,
          char ** argv)
 {
-        struct sigaction sig_act;
         sigset_t  sigset;
-        bool use_stdout = false;
+        bool      use_stdout = false;
+        int       sig;
 
         sigemptyset(&sigset);
         sigaddset(&sigset, SIGINT);
         sigaddset(&sigset, SIGQUIT);
         sigaddset(&sigset, SIGHUP);
+        sigaddset(&sigset, SIGTERM);
         sigaddset(&sigset, SIGPIPE);
 
         argc--;
@@ -2293,22 +2255,6 @@ int main(int     argc,
                 exit(EXIT_FAILURE);
         }
 
-        /* Init sig_act. */
-        memset(&sig_act, 0, sizeof sig_act);
-
-        /* Install signal traps. */
-        sig_act.sa_sigaction = &irmd_sig_handler;
-        sig_act.sa_flags     = SA_SIGINFO;
-
-        if (sigaction(SIGINT,  &sig_act, NULL) < 0)
-                exit(EXIT_FAILURE);
-        if (sigaction(SIGTERM, &sig_act, NULL) < 0)
-                exit(EXIT_FAILURE);
-        if (sigaction(SIGHUP,  &sig_act, NULL) < 0)
-                exit(EXIT_FAILURE);
-        if (sigaction(SIGPIPE, &sig_act, NULL) < 0)
-                exit(EXIT_FAILURE);
-
         log_init(!use_stdout);
 
         if (irm_init() < 0)
@@ -2321,6 +2267,8 @@ int main(int     argc,
                 goto fail_tpm_create;
         }
 
+        pthread_sigmask(SIG_BLOCK, &sigset, NULL);
+
         if (tpm_start(irmd.tpm)) {
                 irmd_set_state(IRMD_NULL);
                 goto fail_tpm_start;
@@ -2336,6 +2284,30 @@ int main(int     argc,
                 goto fail_acceptor;
         }
 
+        while (irmd_get_state() != IRMD_NULL) {
+                if (sigwait(&sigset, &sig) != 0) {
+                        log_warn("Bad signal.");
+                        continue;
+                }
+
+                switch(sig) {
+                case SIGINT:
+                case SIGQUIT:
+                case SIGTERM:
+                case SIGHUP:
+                        log_info("IRMd shutting down...");
+                        irmd_set_state(IRMD_NULL);
+                        break;
+                case SIGPIPE:
+                        log_dbg("Ignored SIGPIPE.");
+                        break;
+                default:
+                        break;
+                }
+        }
+
+        pthread_cancel(irmd.acceptor);
+
         pthread_join(irmd.acceptor, NULL);
         pthread_join(irmd.irm_sanitize, NULL);
 
@@ -2343,8 +2315,6 @@ int main(int     argc,
 
         tpm_destroy(irmd.tpm);
 
-        pthread_sigmask(SIG_BLOCK, &sigset, NULL);
-
         irm_fini();
 
         pthread_sigmask(SIG_UNBLOCK, &sigset, NULL);