From 890dbc49cf440cb765a4d758abab6e39dc9b2e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Fri, 21 May 2021 20:00:45 +0200 Subject: [PATCH] fdm: always check for signals after epoll_pwait(), not only on EINTR This fixes an issue where we, at least on FreeBSD, sometimes get stuck in epoll_pwait() after the shell has exited. It turned out to be because the SIGCHLD signal was delivered at the same time FDs were made readable/writeable. I.e. epoll_pwait() returned a non-zero value even though it should have been interrupted by the SIGCHLD. To avoid having to search the entire signal array *every time* epoll_pwait() returns, add a flag that is set in the signal handler. This tells the FDM to scan the signal array after returning from epoll_pwait(). --- CHANGELOG.md | 3 +++ fdm.c | 34 +++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cbe3eba..1fd34734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,9 @@ scaling factor > 1 (https://codeberg.org/dnkl/foot/issues/509). * Crash caused by certain CSI sequences with very large parameter values (https://codeberg.org/dnkl/foot/issues/522). +* Rare occurrences where the window did not close when the shell + exited. Only seen on FreeBSD + (https://codeberg.org/dnkl/foot/issues/534) ### Security diff --git a/fdm.c b/fdm.c index d0466a6d..c5eaf6c1 100644 --- a/fdm.c +++ b/fdm.c @@ -52,6 +52,7 @@ struct fdm { hooks_t hooks_high; }; +static volatile sig_atomic_t got_signal = false; static volatile sig_atomic_t *received_signals = NULL; struct fdm * @@ -71,6 +72,7 @@ fdm_init(void) xassert(received_signals == NULL); /* Only one FDM instance supported */ received_signals = xcalloc(SIGRTMAX, sizeof(received_signals[0])); + got_signal = false; struct fdm *fdm = malloc(sizeof(*fdm)); if (unlikely(fdm == NULL)) { @@ -328,6 +330,7 @@ fdm_hook_del(struct fdm *fdm, fdm_hook_t hook, enum fdm_hook_priority priority) static void signal_handler(int signo) { + got_signal = true; received_signals[signo] = true; } @@ -424,27 +427,28 @@ fdm_poll(struct fdm *fdm) int r = epoll_pwait( fdm->epoll_fd, events, tll_length(fdm->fds), -1, &fdm->sigmask); - if (unlikely(r < 0)) { - if (errno == EINTR) { - /* TODO: is it possible to receive a signal without - * getting EINTR here? */ + int errno_copy = errno; - for (int i = 0; i < SIGRTMAX; i++) { - if (received_signals[i]) { + if (unlikely(got_signal)) { + got_signal = false; - received_signals[i] = false; - struct sig_handler *handler = &fdm->signal_handlers[i]; + for (int i = 0; i < SIGRTMAX; i++) { + if (received_signals[i]) { + received_signals[i] = false; + struct sig_handler *handler = &fdm->signal_handlers[i]; - xassert(handler->callback != NULL); - if (!handler->callback(fdm, i, handler->callback_data)) - return false; - } + xassert(handler->callback != NULL); + if (!handler->callback(fdm, i, handler->callback_data)) + return false; } - - return true; } + } - LOG_ERRNO("failed to epoll"); + if (unlikely(r < 0)) { + if (errno_copy == EINTR) + return true; + + LOG_ERRNO_P(errno_copy, "failed to epoll"); return false; }