diff --git a/kitty/boss.py b/kitty/boss.py index f49a25b5a..53fcffc27 100644 --- a/kitty/boss.py +++ b/kitty/boss.py @@ -135,7 +135,7 @@ class Boss: yield from t def add_child(self, window): - self.child_monitor.add_child(window.id, window.child.child_fd, window.screen) + self.child_monitor.add_child(window.id, window.child.pid, window.child.child_fd, window.screen) self.window_id_map[window.id] = window wakeup() diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index 612c9f8cd..f71dfdb89 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,7 @@ typedef struct { int fd; unsigned long id; double last_paint_at; + pid_t pid; } Child; static const Child EMPTY_CHILD = {0}; @@ -202,7 +204,7 @@ add_child(ChildMonitor *self, PyObject *args) { if (self->count + add_queue_count >= MAX_CHILDREN) { PyErr_SetString(PyExc_ValueError, "Too many children"); children_mutex(unlock); return NULL; } add_queue[add_queue_count] = EMPTY_CHILD; #define A(attr) &add_queue[add_queue_count].attr - if (!PyArg_ParseTuple(args, "kiO", A(id), A(fd), A(screen))) { + if (!PyArg_ParseTuple(args, "kiiO", A(id), A(pid), A(fd), A(screen))) { children_mutex(unlock); return NULL; } @@ -446,6 +448,70 @@ add_children(ChildMonitor *self) { } } + +static inline void +hangup(pid_t pid) { + errno = 0; + pid_t pgid = getpgid(pid); + if (errno == ESRCH) return; + if (errno != 0) { perror("Failed to get process group id for child"); return; } + if (killpg(pgid, SIGHUP) != 0) { + if (errno != ESRCH) perror("Failed to kill child"); + } +} + +static pid_t pid_buf[MAX_CHILDREN] = {0}; +static size_t pid_buf_pos = 0; +static pthread_t reap_thread; + +static inline void +set_thread_name(pthread_t UNUSED thread, const char *name) { + int ret = 0; +#ifdef __APPLE__ + ret = pthread_setname_np(name); +#else + ret = pthread_setname_np(thread, name); +#endif + if (ret != 0) perror("Failed to set thread name"); +} + + +static void* +reap(void *pid_p) { +#ifdef __APPLE__ + set_thread_name(reap_thread, "KittyReapChild"); +#endif + pid_t pid = *((pid_t*)pid_p); + while(true) { + pid_t ret = waitpid(pid, NULL, 0); + if (ret != pid) { + if (errno == EINTR) continue; + fprintf(stderr, "Failed to reap child process with pid: %d with error: %s\n", pid, strerror(errno)); + } + break; + } + return 0; +} + +static inline void +cleanup_child(ssize_t i) { + close(children[i].fd); + hangup(children[i].pid); + pid_buf[pid_buf_pos] = children[i].pid; + if (waitpid(pid_buf[pid_buf_pos], NULL, WNOHANG) != pid_buf[pid_buf_pos]) { + errno = 0; + int ret = pthread_create(&reap_thread, NULL, reap, pid_buf + pid_buf_pos); + if (ret != 0) perror("Failed to create thread to reap child"); + else { +#ifndef __APPLE__ + set_thread_name(reap_thread, "KittyReapChild"); +#endif + } + } + pid_buf_pos = (pid_buf_pos + 1) % MAX_CHILDREN; +} + + static inline void remove_children(ChildMonitor *self) { if (self->count > 0) { @@ -453,7 +519,7 @@ remove_children(ChildMonitor *self) { for (ssize_t i = self->count - 1; i >= 0; i--) { if (children[i].needs_removal) { count++; - close(fds[EXTRA_FDS + i].fd); + cleanup_child(i); remove_queue[remove_queue_count] = children[i]; remove_queue_count++; children[i] = EMPTY_CHILD; @@ -542,13 +608,7 @@ io_loop(void *data) { bool has_more, data_received; Screen *screen; ChildMonitor *self = (ChildMonitor*)data; - -#define THREAD_NAME "KittyChildMon" -#ifdef __APPLE__ - pthread_setname_np(THREAD_NAME); -#else - pthread_setname_np(self->io_thread, THREAD_NAME); -#endif + set_thread_name(self->io_thread, "KittyChildMon"); while (LIKELY(!self->shutting_down)) { children_mutex(lock); diff --git a/kitty/child.py b/kitty/child.py index 76a2973c0..6253e316f 100644 --- a/kitty/child.py +++ b/kitty/child.py @@ -4,7 +4,6 @@ import os import fcntl -import signal from threading import Thread from .constants import terminfo_dir @@ -74,23 +73,3 @@ class Child: t.daemon = True t.start() return pid - - def hangup(self): - if self.pid is not None: - pid, self.pid = self.pid, None - try: - pgrp = os.getpgid(pid) - except ProcessLookupError: - return - os.killpg(pgrp, signal.SIGHUP) - self.child_fd = None - - def __del__(self): - self.hangup() - - def get_child_status(self): - if self.pid is not None: - try: - return os.waitid(os.P_PID, self.pid, os.WEXITED | os.WNOHANG) - except ChildProcessError: - self.pid = None diff --git a/kitty/window.py b/kitty/window.py index e72bb5def..0296c849a 100644 --- a/kitty/window.py +++ b/kitty/window.py @@ -103,8 +103,6 @@ class Window: if self.destroyed: return self.destroyed = True - self.child.hangup() - self.child.get_child_status() # Ensure child does not become zombie # Remove cycles so that screen is de-allocated immediately boss = get_boss() self.screen.reset_callbacks()