Skip to content

Commit

Permalink
lib: avoid signal-handling race with event loop poll call
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Mark Stapp committed Oct 5, 2020
1 parent e68772c commit e7dc212
Showing 1 changed file with 60 additions and 13 deletions.
73 changes: 60 additions & 13 deletions lib/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,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
Expand Down Expand Up @@ -760,15 +764,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)
;

Expand Down Expand Up @@ -1395,7 +1442,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 {
Expand Down Expand Up @@ -1467,14 +1514,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;
Expand Down

0 comments on commit e7dc212

Please sign in to comment.