From f7f9ceec3438f65e4a285db079925b1143304a48 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Mon, 12 Mar 2018 21:53:02 +0530 Subject: [PATCH] Consolidate signal handling Move the signal handler for SIGCHLD into child-monitor.c Makes for cleaner code. --- kitty/child-monitor.c | 72 +++++++++++++++++++++++++++++++++++++------ kitty/data-types.c | 27 ---------------- kitty/main.py | 7 ++--- 3 files changed, 64 insertions(+), 42 deletions(-) diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index 2bad710f7..015ce335c 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -71,7 +71,7 @@ static unsigned long remove_notify[MAX_CHILDREN] = {0}; static size_t add_queue_count = 0, remove_queue_count = 0; static struct pollfd fds[MAX_CHILDREN + EXTRA_FDS] = {{0}}; static pthread_mutex_t children_lock; -static bool signal_received = false; +static bool kill_signal_received = false; static ChildMonitor *the_monitor = NULL; static uint8_t drain_buf[1024]; static int signal_fds[2], wakeup_fds[2]; @@ -139,10 +139,9 @@ new(PyTypeObject *type, PyObject *args, PyObject UNUSED *kwds) { if (!self_pipe(wakeup_fds)) return PyErr_SetFromErrno(PyExc_OSError); if (!self_pipe(signal_fds)) return PyErr_SetFromErrno(PyExc_OSError); struct sigaction act = {.sa_handler=handle_signal}; - if (sigaction(SIGINT, &act, NULL) != 0) return PyErr_SetFromErrno(PyExc_OSError); - if (sigaction(SIGTERM, &act, NULL) != 0) return PyErr_SetFromErrno(PyExc_OSError); - if (siginterrupt(SIGINT, false) != 0) return PyErr_SetFromErrno(PyExc_OSError); - if (siginterrupt(SIGTERM, false) != 0) return PyErr_SetFromErrno(PyExc_OSError); +#define SA(which) { if (sigaction(which, &act, NULL) != 0) return PyErr_SetFromErrno(PyExc_OSError); if (siginterrupt(which, false) != 0) return PyErr_SetFromErrno(PyExc_OSError);} + SA(SIGINT); SA(SIGTERM); SA(SIGCHLD); +#undef SA self = (ChildMonitor *)type->tp_alloc(type, 0); self->talk_fd = talk_fd; self->listen_fd = listen_fd; @@ -291,6 +290,7 @@ shutdown_monitor(ChildMonitor *self, PyObject *a UNUSED) { #define shutdown_monitor_doc "shutdown_monitor() -> Shutdown the monitor loop." signal(SIGINT, SIG_DFL); signal(SIGTERM, SIG_DFL); + signal(SIGCHLD, SIG_DFL); self->shutting_down = true; wakeup_talk_loop(false); wakeup_io_loop(false); @@ -345,7 +345,7 @@ parse_input(ChildMonitor *self) { } else fatal("Out of memory"); } - if (UNLIKELY(signal_received)) { + if (UNLIKELY(kill_signal_received)) { global_state.close_all_windows = true; } else { count = self->count; @@ -874,6 +874,58 @@ drain_fd(int fd) { } } +static inline void +read_signals(int fd, bool *kill_signal, bool *child_died) { + static char buf[256]; + while(true) { + ssize_t len = read(fd, buf, sizeof(buf)); + if (len < 0) { + if (errno == EINTR) continue; + if (errno != EIO) perror("Call to read() from read_signals() failed"); + break; + } + for (ssize_t i = 0; i < len; i++) { + switch(buf[i]) { + case SIGCHLD: + *child_died = true; break; + case SIGINT: + case SIGTERM: + *kill_signal = true; break; + default: + break; + } + } + break; + } +} + +static inline void +mark_child_for_removal(ChildMonitor *self, pid_t pid) { + children_mutex(lock); + for (size_t i = 0; i < self->count; i++) { + if (children[i].pid == pid) { + children[i].needs_removal = true; + break; + } + } + children_mutex(unlock); +} + +static inline void +reap_children(ChildMonitor *self, bool enable_close_on_child_death) { + int status; + pid_t pid; + (void)self; + while(true) { + pid = waitpid(-1, &status, WNOHANG); + if (pid == -1) { + if (errno != EINTR) break; + } else if (pid > 0) { + if (enable_close_on_child_death) mark_child_for_removal(self, pid); + } else break; + } +} + static inline void write_to_child(int fd, Screen *screen) { size_t written = 0; @@ -925,10 +977,10 @@ io_loop(void *data) { if (fds[0].revents && POLLIN) drain_fd(fds[0].fd); // wakeup if (fds[1].revents && POLLIN) { data_received = true; - drain_fd(fds[1].fd); - children_mutex(lock); - signal_received = true; - children_mutex(unlock); + bool kill_signal = false, child_died = false; + read_signals(fds[1].fd, &kill_signal, &child_died); + if (kill_signal) { children_mutex(lock); kill_signal_received = true; children_mutex(unlock); } + if (child_died) reap_children(self, false); } for (i = 0; i < self->count; i++) { if (fds[EXTRA_FDS + i].revents & (POLLIN | POLLHUP)) { diff --git a/kitty/data-types.c b/kitty/data-types.c index b992db40d..5609fe074 100644 --- a/kitty/data-types.c +++ b/kitty/data-types.c @@ -17,7 +17,6 @@ #include #include #include -#include #ifdef WITH_PROFILER #include #endif @@ -88,31 +87,6 @@ pyset_iutf8(PyObject UNUSED *self, PyObject *args) { Py_RETURN_NONE; } -static void -handle_sigchld(int UNUSED signum, siginfo_t *sinfo, void UNUSED *unused) { - if (sinfo->si_code != CLD_EXITED) return; - int sav_errno = errno, status; - while(true) { - if (waitpid(sinfo->si_pid, &status, WNOHANG) == -1) { - if (errno != EINTR) break; - } else break; - } - // wakeup I/O loop as without this on macOS sometimes poll() does not detect the fd close, so - // kitty does not detect child death. - wakeup_io_loop(true); - errno = sav_errno; -} - -static PyObject* -install_sigchld_handler(PYNOARG) { - struct sigaction sa; - sa.sa_flags = SA_SIGINFO; - sa.sa_sigaction = handle_sigchld; - sigemptyset(&sa.sa_mask); - if (sigaction(SIGCHLD, &sa, NULL) == -1) return PyErr_SetFromErrno(PyExc_OSError); - Py_RETURN_NONE; -} - #ifdef WITH_PROFILER static PyObject* start_profiler(PyObject UNUSED *self, PyObject *args) { @@ -135,7 +109,6 @@ static PyMethodDef module_methods[] = { {"parse_bytes", (PyCFunction)parse_bytes, METH_VARARGS, ""}, {"parse_bytes_dump", (PyCFunction)parse_bytes_dump, METH_VARARGS, ""}, {"redirect_std_streams", (PyCFunction)redirect_std_streams, METH_VARARGS, ""}, - {"install_sigchld_handler", (PyCFunction)install_sigchld_handler, METH_NOARGS, ""}, #ifdef __APPLE__ METHODB(user_cache_dir, METH_NOARGS), #endif diff --git a/kitty/main.py b/kitty/main.py index 8b6f3e1b7..1691eb9b3 100644 --- a/kitty/main.py +++ b/kitty/main.py @@ -4,7 +4,6 @@ import locale import os -import signal import sys from contextlib import contextmanager @@ -14,8 +13,8 @@ from .cli import create_opts, parse_args from .config import cached_values_for, initial_window_size from .constants import appname, glfw_path, is_macos, is_wayland, logo_data_file from .fast_data_types import ( - create_os_window, glfw_init, glfw_terminate, install_sigchld_handler, - set_default_window_icon, set_options, show_window + create_os_window, glfw_init, glfw_terminate, set_default_window_icon, + set_options, show_window ) from .fonts.box_drawing import set_scale from .utils import ( @@ -152,9 +151,7 @@ def _main(): try: with setup_profiling(args): # Avoid needing to launch threads to reap zombies - install_sigchld_handler() run_app(opts, args) - signal.signal(signal.SIGCHLD, signal.SIG_DFL) finally: glfw_terminate()