diff --git a/reaper.c b/reaper.c index 9d98854e..4abc785e 100644 --- a/reaper.c +++ b/reaper.c @@ -3,13 +3,13 @@ #include #include #include +#include #include -#include +#include #define LOG_MODULE "reaper" #define LOG_ENABLE_DBG 0 #include "log.h" -#include "tllist.h" struct child { pid_t pid; @@ -19,56 +19,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; } @@ -79,14 +55,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 @@ -110,69 +81,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); - assert(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; - } - - assert((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/terminal.c b/terminal.c index c65fe974..f12465c3 100644 --- a/terminal.c +++ b/terminal.c @@ -953,22 +953,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 * @@ -1044,7 +1047,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(), @@ -1457,70 +1459,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) { - assert(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) { + assert(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 b6b150ae..3c341299 100644 --- a/terminal.h +++ b/terminal.h @@ -487,8 +487,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;