lib: avoid signal-handling race with event loop poll call

Manage the main pthread's signal mask to avoid a signal-handling
race. Before entering poll, check for pending signals that the
application needs to handle. Use ppoll() to re-enable those
signals during the poll call.

Signed-off-by: Mark Stapp <mjs@voltanet.io>

diff --git a/lib/thread.c b/lib/thread.c
index f83cbea68a9c29d8a53283a0c6663b266ef67427..8d840359e415c80af15ccb21d732b29b8ece8950 100644 (file)
--- a/lib/thread.c
+++ b/lib/thread.c
@@ -723,9 +723,13 @@ static void thread_free(struct thread_master *master, struct thread *thread)
        XFREE(MTYPE_THREAD, thread);
 }
 
-static int fd_poll(struct thread_master *m, struct pollfd *pfds, nfds_t pfdsize,
-                  nfds_t count, const struct timeval *timer_wait)
+static int fd_poll(struct thread_master *m, const struct timeval *timer_wait,
+                  bool *eintr_p)
 {
+       sigset_t origsigs;
+       unsigned char trash[64];
+       nfds_t count = m->handler.copycount;
+
        /*
         * If timer_wait is null here, that means poll() should block
         * indefinitely, unless the thread_master has overridden it by setting
@@ -756,15 +760,58 @@ static int fd_poll(struct thread_master *m, struct pollfd *pfds, nfds_t pfdsize,
        rcu_assert_read_unlocked();
 
        /* add poll pipe poker */
-       assert(count + 1 < pfdsize);
-       pfds[count].fd = m->io_pipe[0];
-       pfds[count].events = POLLIN;
-       pfds[count].revents = 0x00;
+       assert(count + 1 < m->handler.pfdsize);
+       m->handler.copy[count].fd = m->io_pipe[0];
+       m->handler.copy[count].events = POLLIN;
+       m->handler.copy[count].revents = 0x00;
+
+       /* We need to deal with a signal-handling race here: we
+        * don't want to miss a crucial signal, such as SIGTERM or SIGINT,
+        * that may arrive just before we enter poll(). We will block the
+        * key signals, then check whether any have arrived - if so, we return
+        * before calling poll(). If not, we'll re-enable the signals
+        * in the ppoll() call.
+        */
+
+       sigemptyset(&origsigs);
+       if (m->handle_signals) {
+               /* Main pthread that handles the app signals */
+               if (frr_sigevent_check(&origsigs)) {
+                       /* Signal to process - restore signal mask and return */
+                       pthread_sigmask(SIG_SETMASK, &origsigs, NULL);
+                       num = -1;
+                       *eintr_p = true;
+                       goto done;
+               }
+       } else {
+               /* Don't make any changes for the non-main pthreads */
+               pthread_sigmask(SIG_SETMASK, NULL, &origsigs);
+       }
 
-       num = poll(pfds, count + 1, timeout);
+#if defined(HAVE_PPOLL)
+       struct timespec ts, *tsp;
 
-       unsigned char trash[64];
-       if (num > 0 && pfds[count].revents != 0 && num--)
+       if (timeout >= 0) {
+               ts.tv_sec = timeout / 1000;
+               ts.tv_nsec = (timeout % 1000) * 1000000;
+               tsp = &ts;
+       } else
+               tsp = NULL;
+
+       num = ppoll(m->handler.copy, count + 1, tsp, &origsigs);
+       pthread_sigmask(SIG_SETMASK, &origsigs, NULL);
+#else
+       /* Not ideal - there is a race after we restore the signal mask */
+       pthread_sigmask(SIG_SETMASK, &origsigs, NULL);
+       num = poll(m->handler.copy, count + 1, timeout);
+#endif
+
+done:
+
+       if (num < 0 && errno == EINTR)
+               *eintr_p = true;
+
+       if (num > 0 && m->handler.copy[count].revents != 0 && num--)
                while (read(m->io_pipe[0], &trash, sizeof(trash)) > 0)
                        ;
 
@@ -1391,7 +1438,7 @@ struct thread *thread_fetch(struct thread_master *m, struct thread *fetch)
        struct timeval zerotime = {0, 0};
        struct timeval tv;
        struct timeval *tw = NULL;
-
+       bool eintr_p = false;
        int num = 0;
 
        do {
@@ -1463,14 +1510,14 @@ struct thread *thread_fetch(struct thread_master *m, struct thread *fetch)
 
                pthread_mutex_unlock(&m->mtx);
                {
-                       num = fd_poll(m, m->handler.copy, m->handler.pfdsize,
-                                     m->handler.copycount, tw);
+                       eintr_p = false;
+                       num = fd_poll(m, tw, &eintr_p);
                }
                pthread_mutex_lock(&m->mtx);
 
                /* Handle any errors received in poll() */
                if (num < 0) {
-                       if (errno == EINTR) {
+                       if (eintr_p) {
                                pthread_mutex_unlock(&m->mtx);
                                /* loop around to signal handler */
                                continue;