mirror of
https://git.proxmox.com/git/mirror_frr
synced 2025-08-03 22:37:49 +00:00
Merge pull request #7045 from mjstapp/fix_signals
lib: Resolve signal handling race in event loop
This commit is contained in:
commit
502dd27af9
@ -63,6 +63,33 @@ static void quagga_signal_handler(int signo)
|
||||
sigmaster.caught = 1;
|
||||
}
|
||||
|
||||
/*
|
||||
* Check whether any signals have been received and are pending. This is done
|
||||
* with the application's key signals blocked. The complete set of signals
|
||||
* is returned in 'setp', so the caller can restore them when appropriate.
|
||||
* If there are pending signals, returns 'true', 'false' otherwise.
|
||||
*/
|
||||
bool frr_sigevent_check(sigset_t *setp)
|
||||
{
|
||||
sigset_t blocked;
|
||||
int i;
|
||||
bool ret;
|
||||
|
||||
sigemptyset(setp);
|
||||
sigemptyset(&blocked);
|
||||
|
||||
/* Set up mask of application's signals */
|
||||
for (i = 0; i < sigmaster.sigc; i++)
|
||||
sigaddset(&blocked, sigmaster.signals[i].signal);
|
||||
|
||||
pthread_sigmask(SIG_BLOCK, &blocked, setp);
|
||||
|
||||
/* Now that the application's signals are blocked, test. */
|
||||
ret = (sigmaster.caught != 0);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* check if signals have been caught and run appropriate handlers */
|
||||
int quagga_sigevent_process(void)
|
||||
{
|
||||
|
@ -48,6 +48,15 @@ struct quagga_signal_t {
|
||||
extern void signal_init(struct thread_master *m, int sigc,
|
||||
struct quagga_signal_t *signals);
|
||||
|
||||
|
||||
/*
|
||||
* Check whether any signals have been received and are pending. This is done
|
||||
* with the application's key signals blocked. The complete set of signals
|
||||
* is returned in 'setp', so the caller can restore them when appropriate.
|
||||
* If there are pending signals, returns 'true', 'false' otherwise.
|
||||
*/
|
||||
bool frr_sigevent_check(sigset_t *setp);
|
||||
|
||||
/* check whether there are signals to handle, process any found */
|
||||
extern int quagga_sigevent_process(void);
|
||||
|
||||
|
119
lib/thread.c
119
lib/thread.c
@ -728,9 +728,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
|
||||
@ -761,15 +765,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;
|
||||
|
||||
num = poll(pfds, count + 1, timeout);
|
||||
/* 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.
|
||||
*/
|
||||
|
||||
unsigned char trash[64];
|
||||
if (num > 0 && pfds[count].revents != 0 && num--)
|
||||
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);
|
||||
}
|
||||
|
||||
#if defined(HAVE_PPOLL)
|
||||
struct timespec ts, *tsp;
|
||||
|
||||
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)
|
||||
;
|
||||
|
||||
@ -1434,7 +1481,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 {
|
||||
@ -1506,14 +1553,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;
|
||||
@ -1703,3 +1750,49 @@ void funcname_thread_execute(struct thread_master *m,
|
||||
/* Give back or free thread. */
|
||||
thread_add_unuse(m, thread);
|
||||
}
|
||||
|
||||
/* Debug signal mask - if 'sigs' is NULL, use current effective mask. */
|
||||
void debug_signals(const sigset_t *sigs)
|
||||
{
|
||||
int i, found;
|
||||
sigset_t tmpsigs;
|
||||
char buf[300];
|
||||
|
||||
/*
|
||||
* We're only looking at the non-realtime signals here, so we need
|
||||
* some limit value. Platform differences mean at some point we just
|
||||
* need to pick a reasonable value.
|
||||
*/
|
||||
#if defined SIGRTMIN
|
||||
# define LAST_SIGNAL SIGRTMIN
|
||||
#else
|
||||
# define LAST_SIGNAL 32
|
||||
#endif
|
||||
|
||||
|
||||
if (sigs == NULL) {
|
||||
sigemptyset(&tmpsigs);
|
||||
pthread_sigmask(SIG_BLOCK, NULL, &tmpsigs);
|
||||
sigs = &tmpsigs;
|
||||
}
|
||||
|
||||
found = 0;
|
||||
buf[0] = '\0';
|
||||
|
||||
for (i = 0; i < LAST_SIGNAL; i++) {
|
||||
char tmp[20];
|
||||
|
||||
if (sigismember(sigs, i) > 0) {
|
||||
if (found > 0)
|
||||
strlcat(buf, ",", sizeof(buf));
|
||||
snprintf(tmp, sizeof(tmp), "%d", i);
|
||||
strlcat(buf, tmp, sizeof(buf));
|
||||
found++;
|
||||
}
|
||||
}
|
||||
|
||||
if (found == 0)
|
||||
snprintf(buf, sizeof(buf), "<none>");
|
||||
|
||||
zlog_debug("%s: %s", __func__, buf);
|
||||
}
|
||||
|
@ -230,6 +230,9 @@ extern pthread_key_t thread_current;
|
||||
extern char *thread_timer_to_hhmmss(char *buf, int buf_size,
|
||||
struct thread *t_timer);
|
||||
|
||||
/* Debug signal mask */
|
||||
void debug_signals(const sigset_t *sigs);
|
||||
|
||||
#ifdef __cplusplus
|
||||
}
|
||||
#endif
|
||||
|
Loading…
Reference in New Issue
Block a user