From 38dfd6da00870e5a8f1e59258f351d295599720f Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Sat, 31 May 2014 07:38:15 +0200 Subject: More signal handling fixes --- src/fastd.c | 30 +++++++++++++++--------------- src/fastd.h | 10 ++++++++++ src/poll.c | 37 ++++++++++++++++++++----------------- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/fastd.c b/src/fastd.c index 378d641..c8f8e54 100644 --- a/src/fastd.c +++ b/src/fastd.c @@ -66,30 +66,30 @@ fastd_context_t ctx = {}; -static volatile bool sighup = false; /**< Is set to true when a SIGHUP is received */ -static volatile bool terminate = false; /**< Is set to true when a SIGTERM, SIGQUIT or SIGINT is received */ -static volatile bool dump = false; /**< Is set to true when a SIGUSR1 is received */ -static volatile bool sigchld = false; /**< Is set to true when a SIGCHLD is received */ +volatile bool fastd_sig_hup = false; /**< Is set to true when a SIGHUP is received */ +volatile bool fastd_sig_terminate = false; /**< Is set to true when a SIGTERM, SIGQUIT or SIGINT is received */ +volatile bool fastd_sig_dump = false; /**< Is set to true when a SIGUSR1 is received */ +volatile bool fastd_sig_chld = false; /**< Is set to true when a SIGCHLD is received */ /** SIGHUP handler */ static void on_sighup(int signo UNUSED) { - sighup = true; + fastd_sig_hup = true; } /** SIGTERM, SIGQUIT and SIGINT handler */ static void on_terminate(int signo UNUSED) { - terminate = true; + fastd_sig_terminate = true; } /** SIGUSR1 handler */ static void on_sigusr1(int signo UNUSED) { - dump = true; + fastd_sig_dump = true; } /** SIGCHLD handler */ static void on_sigchld(int signo UNUSED) { - sigchld = true; + fastd_sig_chld = true; } /** Installs signal handlers */ @@ -625,8 +625,8 @@ static inline void reap_zombies(void) { /** The \em real signal handlers */ static inline void handle_signals(void) { - if (sighup) { - sighup = false; + if (fastd_sig_hup) { + fastd_sig_hup = false; pr_info("reconfigure triggered"); @@ -634,13 +634,13 @@ static inline void handle_signals(void) { init_peers(); } - if (dump) { - dump = false; + if (fastd_sig_dump) { + fastd_sig_dump = false; dump_state(); } - if (sigchld) { - sigchld = false; + if (fastd_sig_chld) { + fastd_sig_chld = false; reap_zombies(); } } @@ -699,7 +699,7 @@ static inline void cleanup(void) { int main(int argc, char *argv[]) { init(argc, argv); - while (!terminate) + while (!fastd_sig_terminate) run(); cleanup(); diff --git a/src/fastd.h b/src/fastd.h index f69f2f8..eeb2d3d 100644 --- a/src/fastd.h +++ b/src/fastd.h @@ -292,6 +292,11 @@ struct fastd_string_stack { extern fastd_context_t ctx; extern fastd_config_t conf; +extern volatile bool fastd_sig_hup; +extern volatile bool fastd_sig_terminate; +extern volatile bool fastd_sig_dump; +extern volatile bool fastd_sig_chld; + void fastd_send(const fastd_socket_t *sock, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *remote_addr, fastd_peer_t *peer, fastd_buffer_t buffer, size_t stat_size); void fastd_send_handshake(const fastd_socket_t *sock, const fastd_peer_address_t *local_addr, const fastd_peer_address_t *remote_addr, fastd_peer_t *peer, fastd_buffer_t buffer); @@ -371,6 +376,11 @@ static inline size_t alignto(size_t l, size_t a) { return block_count(l, a)*a; } +/** Returns true if any unhandled signal in currently queued */ +static inline bool fastd_signalled(void) { + return (fastd_sig_hup || fastd_sig_terminate || fastd_sig_dump || fastd_sig_chld); +} + /** Returns the maximum payload size \em fastd is configured to transport */ static inline size_t fastd_max_payload(void) { switch (conf.mode) { diff --git a/src/poll.c b/src/poll.c index f05a345..a74d249 100644 --- a/src/poll.c +++ b/src/poll.c @@ -133,23 +133,19 @@ void fastd_poll_handle(void) { if (timeout < 0 || timeout > maintenance_timeout) timeout = maintenance_timeout; - sigset_t set, oldset; + sigset_t set; sigemptyset(&set); - pthread_sigmask(SIG_SETMASK, &set, &oldset); struct epoll_event events[16]; - int ret = epoll_wait(ctx.epoll_fd, events, 16, timeout); - if (ret < 0) { - if (errno == EINTR) - return; - + int ret = epoll_pwait(ctx.epoll_fd, events, 16, timeout, &set); + if (ret < 0 && errno != EINTR) exit_errno("epoll_wait"); - } - - pthread_sigmask(SIG_SETMASK, &oldset, NULL); fastd_update_time(); + if (ret < 0) + return; + size_t i; for (i = 0; i < (size_t)ret; i++) { if (events[i].data.ptr == &ctx.tunfd) { @@ -257,18 +253,25 @@ void fastd_poll_handle(void) { sigemptyset(&set); pthread_sigmask(SIG_SETMASK, &set, &oldset); - int ret = poll(VECTOR_DATA(ctx.pollfds), VECTOR_LEN(ctx.pollfds), timeout); - if (ret < 0) { - if (errno == EINTR) - return; - - exit_errno("poll"); + int ret = -1; + + if (!fastd_signalled()) { + /* + There is a race condition here: if a signal occurs after the fastd_signalled() check, but before the poll + call, poll() will be called with its normal timeout, potentially delaying the actual signal handling. On + OpenBSD we could use ppoll() to fix this, but on FreeBSD we're out of luck. + */ + ret = poll(VECTOR_DATA(ctx.pollfds), VECTOR_LEN(ctx.pollfds), timeout); + if (ret < 0 && errno != EINTR) + exit_errno("poll"); } pthread_sigmask(SIG_SETMASK, &oldset, NULL); - fastd_update_time(); + if (ret < 0) + return; + if (VECTOR_INDEX(ctx.pollfds, 0).revents & POLLIN) fastd_tuntap_handle(); if (VECTOR_INDEX(ctx.pollfds, 1).revents & POLLIN) -- cgit v1.2.3