diff --git a/fdm.c b/fdm.c index 34d0fbff..548e4230 100644 --- a/fdm.c +++ b/fdm.c @@ -6,6 +6,7 @@ #include #include #include +#include #include @@ -15,15 +16,22 @@ #define LOG_ENABLE_DBG 0 #include "log.h" #include "debug.h" +#include "xmalloc.h" -struct handler { +struct fd_handler { int fd; int events; - fdm_handler_t callback; + fdm_fd_handler_t callback; void *callback_data; bool deleted; }; +struct sig_handler { + int signo; + fdm_signal_handler_t callback; + void *callback_data; +}; + struct hook { fdm_hook_t callback; void *callback_data; @@ -34,33 +42,58 @@ typedef tll(struct hook) hooks_t; struct fdm { int epoll_fd; bool is_polling; - tll(struct handler *) fds; - tll(struct handler *) deferred_delete; + tll(struct fd_handler *) fds; + tll(struct fd_handler *) deferred_delete; + + sigset_t sigmask; + struct sig_handler *signal_handlers; + hooks_t hooks_low; hooks_t hooks_normal; hooks_t hooks_high; }; +static volatile sig_atomic_t *received_signals = NULL; + struct fdm * fdm_init(void) { + sigset_t sigmask; + if (sigprocmask(0, NULL, &sigmask) < 0) { + LOG_ERRNO("failed to get process signal mask"); + return NULL; + } + int epoll_fd = epoll_create1(EPOLL_CLOEXEC); if (epoll_fd == -1) { LOG_ERRNO("failed to create epoll FD"); return NULL; } + xassert(received_signals == NULL); /* Only one FDM instance supported */ + received_signals = xcalloc(SIGRTMAX, sizeof(received_signals[0])); + struct fdm *fdm = malloc(sizeof(*fdm)); if (unlikely(fdm == NULL)) { LOG_ERRNO("malloc() failed"); return NULL; } + struct sig_handler *sig_handlers = calloc(SIGRTMAX, sizeof(sig_handlers[0])); + + if (sig_handlers == NULL) { + LOG_ERRNO("failed to allocate signal handler array"); + free(fdm); + return NULL; + } + *fdm = (struct fdm){ .epoll_fd = epoll_fd, .is_polling = false, .fds = tll_init(), .deferred_delete = tll_init(), + .sigmask = sigmask, + .signal_handlers = sig_handlers, .hooks_low = tll_init(), .hooks_normal = tll_init(), .hooks_high = tll_init(), @@ -77,6 +110,11 @@ fdm_destroy(struct fdm *fdm) if (tll_length(fdm->fds) > 0) LOG_WARN("FD list not empty"); + for (int i = 0; i < SIGRTMAX; i++) { + if (fdm->signal_handlers[i].callback != NULL) + LOG_WARN("handler for signal %d not removed", i); + } + if (tll_length(fdm->hooks_low) > 0 || tll_length(fdm->hooks_normal) > 0 || tll_length(fdm->hooks_high) > 0) @@ -90,6 +128,9 @@ fdm_destroy(struct fdm *fdm) xassert(tll_length(fdm->hooks_normal) == 0); xassert(tll_length(fdm->hooks_high) == 0); + sigprocmask(SIG_SETMASK, &fdm->sigmask, NULL); + free(fdm->signal_handlers); + tll_free(fdm->fds); tll_free(fdm->deferred_delete); tll_free(fdm->hooks_low); @@ -97,17 +138,15 @@ fdm_destroy(struct fdm *fdm) tll_free(fdm->hooks_high); close(fdm->epoll_fd); free(fdm); + + free((void *)received_signals); + received_signals = NULL; } bool -fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data) +fdm_add(struct fdm *fdm, int fd, int events, fdm_fd_handler_t cb, void *data) { #if defined(_DEBUG) - int flags = fcntl(fd, F_GETFL); - if (!(flags & O_NONBLOCK)) { - BUG("FD=%d is in blocking mode", fd); - } - tll_foreach(fdm->fds, it) { if (it->item->fd == fd) { BUG("FD=%d already registered", fd); @@ -115,30 +154,30 @@ fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data) } #endif - struct handler *fd_data = malloc(sizeof(*fd_data)); - if (unlikely(fd_data == NULL)) { + struct fd_handler *handler = malloc(sizeof(*handler)); + if (unlikely(handler == NULL)) { LOG_ERRNO("malloc() failed"); return false; } - *fd_data = (struct handler) { + *handler = (struct fd_handler) { .fd = fd, .events = events, - .callback = handler, + .callback = cb, .callback_data = data, .deleted = false, }; - tll_push_back(fdm->fds, fd_data); + tll_push_back(fdm->fds, handler); struct epoll_event ev = { .events = events, - .data = {.ptr = fd_data}, + .data = {.ptr = handler}, }; if (epoll_ctl(fdm->epoll_fd, EPOLL_CTL_ADD, fd, &ev) < 0) { LOG_ERRNO("failed to register FD=%d with epoll", fd); - free(fd_data); + free(handler); tll_pop_back(fdm->fds); return false; } @@ -190,7 +229,7 @@ fdm_del_no_close(struct fdm *fdm, int fd) } static bool -event_modify(struct fdm *fdm, struct handler *fd, int new_events) +event_modify(struct fdm *fdm, struct fd_handler *fd, int new_events) { if (new_events == fd->events) return true; @@ -287,6 +326,69 @@ fdm_hook_del(struct fdm *fdm, fdm_hook_t hook, enum fdm_hook_priority priority) return false; } +static void +signal_handler(int signo) +{ + received_signals[signo] = true; +} + +bool +fdm_signal_add(struct fdm *fdm, int signo, fdm_signal_handler_t handler, void *data) +{ + if (fdm->signal_handlers[signo].callback != NULL) { + LOG_ERR("signal %d already has a handler", signo); + return false; + } + + sigset_t mask, original; + sigemptyset(&mask); + sigaddset(&mask, signo); + + if (sigprocmask(SIG_BLOCK, &mask, &original) < 0) { + LOG_ERRNO("failed to block signal %d", signo); + return false; + } + + struct sigaction action = {.sa_handler = &signal_handler}; + if (sigaction(signo, &action, NULL) < 0) { + LOG_ERRNO("failed to set signal handler for signal %d", signo); + sigprocmask(SIG_SETMASK, &original, NULL); + return false; + } + + received_signals[signo] = false; + fdm->signal_handlers[signo].callback = handler; + fdm->signal_handlers[signo].callback_data = data; + return true; +} + +bool +fdm_signal_del(struct fdm *fdm, int signo) +{ + if (fdm->signal_handlers[signo].callback == NULL) + return false; + + struct sigaction action = {.sa_handler = SIG_DFL}; + if (sigaction(signo, &action, NULL) < 0) { + LOG_ERRNO("failed to restore signal handler for signal %d", signo); + return false; + } + + received_signals[signo] = false; + fdm->signal_handlers[signo].callback = NULL; + fdm->signal_handlers[signo].callback_data = NULL; + + sigset_t mask; + sigemptyset(&mask); + sigaddset(&mask, signo); + if (sigprocmask(SIG_UNBLOCK, &mask, NULL) < 0) { + LOG_ERRNO("failed to unblock signal %d", signo); + return false; + } + + return true; +} + bool fdm_poll(struct fdm *fdm) { @@ -320,10 +422,28 @@ fdm_poll(struct fdm *fdm) struct epoll_event events[tll_length(fdm->fds)]; - int r = epoll_wait(fdm->epoll_fd, events, tll_length(fdm->fds), -1); + int r = epoll_pwait( + fdm->epoll_fd, events, tll_length(fdm->fds), -1, &fdm->sigmask); + if (unlikely(r < 0)) { - if (errno == EINTR) + if (errno == EINTR) { + /* TODO: is it possible to receive a signal without + * getting EINTR here? */ + + 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; + } + } + return true; + } LOG_ERRNO("failed to epoll"); return false; @@ -333,7 +453,7 @@ fdm_poll(struct fdm *fdm) fdm->is_polling = true; for (int i = 0; i < r; i++) { - struct handler *fd = events[i].data.ptr; + struct fd_handler *fd = events[i].data.ptr; if (fd->deleted) continue; diff --git a/fdm.h b/fdm.h index 8661c580..2ccff958 100644 --- a/fdm.h +++ b/fdm.h @@ -4,7 +4,8 @@ struct fdm; -typedef bool (*fdm_handler_t)(struct fdm *fdm, int fd, int events, void *data); +typedef bool (*fdm_fd_handler_t)(struct fdm *fdm, int fd, int events, void *data); +typedef bool (*fdm_signal_handler_t)(struct fdm *fdm, int signo, void *data); typedef void (*fdm_hook_t)(struct fdm *fdm, void *data); enum fdm_hook_priority { @@ -16,7 +17,7 @@ enum fdm_hook_priority { struct fdm *fdm_init(void); void fdm_destroy(struct fdm *fdm); -bool fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data); +bool fdm_add(struct fdm *fdm, int fd, int events, fdm_fd_handler_t handler, void *data); bool fdm_del(struct fdm *fdm, int fd); bool fdm_del_no_close(struct fdm *fdm, int fd); @@ -27,4 +28,7 @@ bool fdm_hook_add(struct fdm *fdm, fdm_hook_t hook, void *data, enum fdm_hook_priority priority); bool fdm_hook_del(struct fdm *fdm, fdm_hook_t hook, enum fdm_hook_priority priority); +bool fdm_signal_add(struct fdm *fdm, int signo, fdm_signal_handler_t handler, void *data); +bool fdm_signal_del(struct fdm *fdm, int signo); + bool fdm_poll(struct fdm *fdm); diff --git a/main.c b/main.c index b3953d22..e4d517d4 100644 --- a/main.c +++ b/main.c @@ -33,12 +33,11 @@ #include "xmalloc.h" #include "xsnprintf.h" -static volatile sig_atomic_t aborted = 0; - -static void -sig_handler(int signo) +static bool +fdm_sigint(struct fdm *fdm, int signo, void *data) { - aborted = 1; + *(volatile sig_atomic_t *)data = true; + return true; } static const char * @@ -465,10 +464,10 @@ main(int argc, char *const *argv) if (as_server && (server = server_init(&conf, fdm, reaper, wayl)) == NULL) goto out; - /* Remember to restore signals in slave */ - const struct sigaction sa = {.sa_handler = &sig_handler}; - if (sigaction(SIGINT, &sa, NULL) < 0 || sigaction(SIGTERM, &sa, NULL) < 0) { - LOG_ERRNO("failed to register signal handlers"); + volatile sig_atomic_t aborted = false; + if (!fdm_signal_add(fdm, SIGINT, &fdm_sigint, (void *)&aborted) || + !fdm_signal_add(fdm, SIGTERM, &fdm_sigint, (void *)&aborted)) + { goto out; } @@ -501,6 +500,8 @@ out: render_destroy(renderer); wayl_destroy(wayl); reaper_destroy(reaper); + fdm_signal_del(fdm, SIGTERM); + fdm_signal_del(fdm, SIGINT); fdm_destroy(fdm); config_free(conf); diff --git a/pgo/pgo.c b/pgo/pgo.c index 4bb67d35..933d100c 100644 --- a/pgo/pgo.c +++ b/pgo/pgo.c @@ -35,7 +35,7 @@ async_write(int fd, const void *data, size_t len, size_t *idx) } bool -fdm_add(struct fdm *fdm, int fd, int events, fdm_handler_t handler, void *data) +fdm_add(struct fdm *fdm, int fd, int events, fdm_fd_handler_t handler, void *data) { return true; } diff --git a/reaper.c b/reaper.c index dec15cb2..a2b7bac0 100644 --- a/reaper.c +++ b/reaper.c @@ -3,8 +3,8 @@ #include #include #include +#include #include -#include #include #define LOG_MODULE "reaper" @@ -20,56 +20,32 @@ struct child { struct reaper { struct fdm *fdm; - int fd; tll(struct child) children; }; -static bool fdm_reap(struct fdm *fdm, int fd, int events, void *data); +static bool fdm_reap(struct fdm *fdm, int signo, void *data); struct reaper * reaper_init(struct fdm *fdm) { - sigset_t mask; - sigemptyset(&mask); - sigaddset(&mask, SIGCHLD); - - /* Block normal signal handling - we're using a signalfd instead */ - if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) { - LOG_ERRNO("failed to block SIGCHLD"); - return NULL; - } - - int fd = signalfd(-1, &mask, SFD_NONBLOCK | SFD_CLOEXEC); - if (fd < 0) { - LOG_ERRNO("failed to create signal FD"); - sigprocmask(SIG_UNBLOCK, &mask, NULL); - return NULL; - } - struct reaper *reaper = malloc(sizeof(*reaper)); if (unlikely(reaper == NULL)) { LOG_ERRNO("malloc() failed"); - close(fd); - sigprocmask(SIG_UNBLOCK, &mask, NULL); return NULL; } *reaper = (struct reaper){ .fdm = fdm, - .fd = fd, .children = tll_init(), }; - if (!fdm_add(fdm, fd, EPOLLIN, &fdm_reap, reaper)){ - LOG_ERR("failed to register with the FDM"); + if (!fdm_signal_add(fdm, SIGCHLD, &fdm_reap, reaper)) goto err; - } return reaper; err: tll_free(reaper->children); - close(fd); free(reaper); return NULL; } @@ -80,14 +56,9 @@ reaper_destroy(struct reaper *reaper) if (reaper == NULL) return; - fdm_del(reaper->fdm, reaper->fd); + fdm_signal_del(reaper->fdm, SIGCHLD); tll_free(reaper->children); free(reaper); - - sigset_t mask; - sigemptyset(&mask); - sigaddset(&mask, SIGCHLD); - sigprocmask(SIG_UNBLOCK, &mask, NULL); } void @@ -111,69 +82,40 @@ reaper_del(struct reaper *reaper, pid_t pid) } static bool -fdm_reap(struct fdm *fdm, int fd, int events, void *data) +fdm_reap(struct fdm *fdm, int signo, void *data) { struct reaper *reaper = data; - bool pollin = events & EPOLLIN; - bool hup = events & EPOLLHUP; + while (true) { + int status; + pid_t pid = waitpid(-1, &status, WNOHANG); + if (pid <= 0) + break; - if (hup && !pollin) - return false; + if (WIFEXITED(status)) + LOG_DBG("pid=%d: exited with status=%d", pid, WEXITSTATUS(status)); + else if (WIFSIGNALED(status)) + LOG_DBG("pid=%d: killed by signal=%d", pid, WTERMSIG(status)); + else + LOG_DBG("pid=%d: died of unknown resason", pid); - xassert(pollin); + tll_foreach(reaper->children, it) { + struct child *_child = &it->item; - struct signalfd_siginfo info; - ssize_t amount = read(reaper->fd, &info, sizeof(info)); - - if (amount < 0) { - LOG_ERRNO("failed to read"); - return false; - } - - xassert((size_t)amount >= sizeof(info)); - - if (info.ssi_signo != SIGCHLD) { - LOG_WARN("got non-SIGCHLD signal: %d", info.ssi_signo); - return true; - } - - tll_foreach(reaper->children, it) { - struct child *_child = &it->item; - - if (_child->pid != (pid_t)info.ssi_pid) - continue; - - /* Make sure we remove it *before* the callback, since it too - * may remove it */ - struct child child = it->item; - tll_remove(reaper->children, it); - - bool reap_ourselves = true; - if (child.cb != NULL) - reap_ourselves = !child.cb(reaper, child.pid, child.cb_data); - - if (reap_ourselves) { - int result; - int res = waitpid(child.pid, &result, WNOHANG); - - if (res <= 0) { - if (res < 0) - LOG_ERRNO("waitpid failed for pid=%d", child.pid); + if (_child->pid != pid) continue; - } - else if (WIFEXITED(result)) - LOG_DBG("pid=%d: exited with status=%d", pid, WEXITSTATUS(result)); - else if (WIFSIGNALED(result)) - LOG_DBG("pid=%d: killed by signal=%d", pid, WTERMSIG(result)); - else - LOG_DBG("pid=%d: died of unknown resason", pid); + /* Make sure we remove it *before* the callback, since it too + * may remove it */ + struct child child = it->item; + tll_remove(reaper->children, it); + + if (child.cb != NULL) + child.cb(reaper, child.pid, status, child.cb_data); + + break; } } - if (hup) - return false; - return true; } diff --git a/reaper.h b/reaper.h index 2cd6dd6e..4416af03 100644 --- a/reaper.h +++ b/reaper.h @@ -10,7 +10,8 @@ struct reaper; struct reaper *reaper_init(struct fdm *fdm); void reaper_destroy(struct reaper *reaper); -typedef bool (*reaper_cb)(struct reaper *reaper, pid_t pid, void *data); +typedef void (*reaper_cb)( + struct reaper *reaper, pid_t pid, int status, void *data); void reaper_add(struct reaper *reaper, pid_t pid, reaper_cb cb, void *cb_data); void reaper_del(struct reaper *reaper, pid_t pid); diff --git a/render.c b/render.c index 026f176f..58a93a33 100644 --- a/render.c +++ b/render.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -1265,6 +1266,10 @@ render_worker_thread(void *_ctx) const int my_id = ctx->my_id; free(ctx); + sigset_t mask; + sigfillset(&mask); + pthread_sigmask(SIG_SETMASK, &mask, NULL); + char proc_title[16]; snprintf(proc_title, sizeof(proc_title), "foot:render:%d", my_id); diff --git a/terminal.c b/terminal.c index c080c0c4..6c54543e 100644 --- a/terminal.c +++ b/terminal.c @@ -984,22 +984,25 @@ load_fonts_from_conf(struct terminal *term) return reload_fonts(term); } -static bool -slave_died(struct reaper *reaper, pid_t pid, void *data) +static void +slave_died(struct reaper *reaper, pid_t pid, int status, void *data) { struct terminal *term = data; LOG_DBG("slave (PID=%u) died", pid); + term->slave_has_been_reaped = true; + term->exit_status = status; + if (term->conf->hold_at_exit) { /* The PTMX FDM handler may already have closed our end */ if (term->ptmx >= 0) { fdm_del(term->fdm, term->ptmx); term->ptmx = -1; } - return true; + return; } - return term_shutdown(term); + term_shutdown(term); } struct terminal * @@ -1075,7 +1078,6 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .fdm = fdm, .reaper = reaper, .conf = conf, - .quit = false, .ptmx = ptmx, .ptmx_buffers = tll_init(), .ptmx_paste_buffers = tll_init(), @@ -1503,70 +1505,76 @@ term_destroy(struct terminal *term) int ret = EXIT_SUCCESS; if (term->slave > 0) { - LOG_DBG("waiting for slave (PID=%u) to die", term->slave); + int exit_status; - /* - * Note: we've closed ptmx, so the slave *should* exit... - * - * But, since it is possible to write clients that ignore - * this, we need to handle it in *some* way. - * - * So, what we do is register a SIGALRM handler, and configure - * a 2 second alarm. If the slave hasn't died after this time, - * we send it a SIGTERM, then wait another 2 seconds (using - * the same alarm mechanism). If it still hasn't died, we send - * it a SIGKILL. - * - * Note that this solution is *not* asynchronous, and any - * other events etc will be ignored during this time. This of - * course only applies to a 'foot --server' instance, where - * there might be other terminals running. - */ - sigaction(SIGALRM, &(const struct sigaction){.sa_handler = &sig_alarm}, NULL); - alarm(2); + if (term->slave_has_been_reaped) + exit_status = term->exit_status; + else { + LOG_DBG("waiting for slave (PID=%u) to die", term->slave); - int status; - int kill_signal = SIGTERM; + /* + * Note: we've closed ptmx, so the slave *should* exit... + * + * But, since it is possible to write clients that ignore + * this, we need to handle it in *some* way. + * + * So, what we do is register a SIGALRM handler, and configure + * a 2 second alarm. If the slave hasn't died after this time, + * we send it a SIGTERM, then wait another 2 seconds (using + * the same alarm mechanism). If it still hasn't died, we send + * it a SIGKILL. + * + * Note that this solution is *not* asynchronous, and any + * other events etc will be ignored during this time. This of + * course only applies to a 'foot --server' instance, where + * there might be other terminals running. + */ + sigaction(SIGALRM, &(const struct sigaction){.sa_handler = &sig_alarm}, NULL); + alarm(2); - while (true) { - int r = waitpid(term->slave, &status, 0); + int kill_signal = SIGTERM; - if (r == term->slave) - break; + while (true) { + int r = waitpid(term->slave, &exit_status, 0); - if (r == -1) { - xassert(errno == EINTR); + if (r == term->slave) + break; - if (alarm_raised) { - LOG_DBG("slave hasn't died yet, sending: %s (%d)", - kill_signal == SIGTERM ? "SIGTERM" : "SIGKILL", - kill_signal); + if (r == -1) { + xassert(errno == EINTR); - kill(term->slave, kill_signal); + if (alarm_raised) { + LOG_DBG("slave hasn't died yet, sending: %s (%d)", + kill_signal == SIGTERM ? "SIGTERM" : "SIGKILL", + kill_signal); - alarm_raised = 0; - if (kill_signal != SIGKILL) - alarm(2); + kill(term->slave, kill_signal); - kill_signal = SIGKILL; + alarm_raised = 0; + if (kill_signal != SIGKILL) + alarm(2); + kill_signal = SIGKILL; + + } } } + + /* Cancel alarm */ + alarm(0); + sigaction(SIGALRM, &(const struct sigaction){.sa_handler = SIG_DFL}, NULL); } - /* Cancel alarm */ - alarm(0); - sigaction(SIGALRM, &(const struct sigaction){.sa_handler = SIG_DFL}, NULL); - ret = EXIT_FAILURE; - if (WIFEXITED(status)) { - ret = WEXITSTATUS(status); + if (WIFEXITED(exit_status)) { + ret = WEXITSTATUS(exit_status); LOG_DBG("slave exited with code %d", ret); - } else if (WIFSIGNALED(status)) { - ret = WTERMSIG(status); + } else if (WIFSIGNALED(exit_status)) { + ret = WTERMSIG(exit_status); LOG_WARN("slave exited with signal %d (%s)", ret, strsignal(ret)); } else { - LOG_WARN("slave exited for unknown reason (status = 0x%08x)", status); + LOG_WARN("slave exited for unknown reason (status = 0x%08x)", + exit_status); } } diff --git a/terminal.h b/terminal.h index 43dd8e03..94433178 100644 --- a/terminal.h +++ b/terminal.h @@ -534,8 +534,9 @@ struct terminal { } ime; #endif - bool quit; bool is_shutting_down; + bool slave_has_been_reaped; + int exit_status; void (*shutdown_cb)(void *data, int exit_code); void *shutdown_data;