From 5f13946bac3e12e6d8e1bd4c4e69eacf5dca6361 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Mon, 13 Jun 2022 18:52:23 +0530 Subject: [PATCH] Allow using our signal handlers in python event loops via an fd pythons signal fd only return signal numbers not the full siginfo struct --- kitty/child-monitor.c | 3 +- kitty/data-types.c | 2 + kitty/fast_data_types.pyi | 27 ++++++- kitty/loop-utils.c | 158 ++++++++++++++++++++++++++++++-------- kitty/loop-utils.h | 2 +- kitty_tests/prewarm.py | 48 +++++++++++- 6 files changed, 204 insertions(+), 36 deletions(-) diff --git a/kitty/child-monitor.c b/kitty/child-monitor.c index 3b3952b8c..771e98547 100644 --- a/kitty/child-monitor.c +++ b/kitty/child-monitor.c @@ -1255,7 +1255,7 @@ read_bytes(int fd, Screen *screen) { typedef struct { bool kill_signal, child_died, reload_config; } SignalSet; -static void +static bool handle_signal(const siginfo_t *siginfo, void *data) { SignalSet *ss = data; switch(siginfo->si_signo) { @@ -1276,6 +1276,7 @@ handle_signal(const siginfo_t *siginfo, void *data) { default: break; } + return true; } static void diff --git a/kitty/data-types.c b/kitty/data-types.c index 6d54c8a7e..702c21109 100644 --- a/kitty/data-types.c +++ b/kitty/data-types.c @@ -247,6 +247,7 @@ extern bool init_kittens(PyObject *module); extern bool init_logging(PyObject *module); extern bool init_png_reader(PyObject *module); extern bool init_utmp(PyObject *module); +extern bool init_loop_utils(PyObject *module); #ifdef __APPLE__ extern int init_CoreText(PyObject *); extern bool init_cocoa(PyObject *module); @@ -315,6 +316,7 @@ PyInit_fast_data_types(void) { #endif if (!init_fonts(m)) return NULL; if (!init_utmp(m)) return NULL; + if (!init_loop_utils(m)) return NULL; CellAttrs a; #define s(name, attr) { a.val = 0; a.attr = 1; PyModule_AddIntConstant(m, #name, shift_to_first_set_bit(a)); } diff --git a/kitty/fast_data_types.pyi b/kitty/fast_data_types.pyi index c45a153ee..d4f121842 100644 --- a/kitty/fast_data_types.pyi +++ b/kitty/fast_data_types.pyi @@ -1,8 +1,8 @@ import termios from ctypes import Array, c_ubyte from typing import ( - Any, AnyStr, Callable, Dict, List, NewType, Optional, Tuple, TypedDict, - Union + Any, AnyStr, Callable, Dict, List, NamedTuple, NewType, Optional, Tuple, + TypedDict, Union ) from kitty.boss import Boss @@ -1392,3 +1392,26 @@ def establish_controlling_tty(ttyname: str, stdin: int, stdout: int, stderr: int def random_unix_socket() -> int: pass + + +class SignalInfo(NamedTuple): + si_signo: int + si_code: int + si_pid: int + si_uid: int + si_addr: int + si_status: int + sival_int: int + sival_ptr: int + + +def read_signals(fd: int, callback: Callable[[SignalInfo], None]) -> None: + pass + + +def install_signal_handlers(*signals: int) -> Tuple[int, int]: + pass + + +def remove_signal_handlers() -> None: + pass diff --git a/kitty/loop-utils.c b/kitty/loop-utils.c index 831f8c11a..6b3544050 100644 --- a/kitty/loop-utils.c +++ b/kitty/loop-utils.c @@ -31,25 +31,8 @@ handle_signal(int sig_num UNUSED, siginfo_t *si, void *ucontext UNUSED) { } #endif - -bool -init_loop_data(LoopData *ld, ...) { - ld->num_handled_signals = 0; - va_list valist; - va_start(valist, ld); - while (true) { - int sig = va_arg(valist, int); - if (!sig) break; - ld->handled_signals[ld->num_handled_signals++] = sig; - } - va_end(valist); -#ifdef HAS_EVENT_FD - ld->wakeup_read_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); - if (ld->wakeup_read_fd < 0) return false; -#else - if (!self_pipe(ld->wakeup_fds, true)) return false; - ld->wakeup_read_fd = ld->wakeup_fds[0]; -#endif +static bool +init_signal_handlers(LoopData *ld) { ld->signal_read_fd = -1; #ifdef HAS_SIGNAL_FD sigemptyset(&ld->signals); @@ -72,18 +55,35 @@ init_loop_data(LoopData *ld, ...) { return true; } - -void -free_loop_data(LoopData *ld) { -#define CLOSE(which, idx) if (ld->which[idx] > -1) safe_close(ld->which[idx], __FILE__, __LINE__); ld->which[idx] = -1; -#ifndef HAS_EVENT_FD - CLOSE(wakeup_fds, 0); CLOSE(wakeup_fds, 1); +bool +init_loop_data(LoopData *ld, ...) { + ld->num_handled_signals = 0; + va_list valist; + va_start(valist, ld); + while (true) { + int sig = va_arg(valist, int); + if (!sig) break; + ld->handled_signals[ld->num_handled_signals++] = sig; + } + va_end(valist); +#ifdef HAS_EVENT_FD + ld->wakeup_read_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); + if (ld->wakeup_read_fd < 0) return false; +#else + if (!self_pipe(ld->wakeup_fds, true)) return false; + ld->wakeup_read_fd = ld->wakeup_fds[0]; #endif + return init_signal_handlers(ld); +} + +#define CLOSE(which, idx) if (ld->which[idx] > -1) { safe_close(ld->which[idx], __FILE__, __LINE__); ld->which[idx] = -1; } + +static void +remove_signal_handlers(LoopData *ld) { #ifndef HAS_SIGNAL_FD signal_write_fd = -1; CLOSE(signal_fds, 0); CLOSE(signal_fds, 1); #endif -#undef CLOSE if (ld->signal_read_fd > -1) { #ifdef HAS_SIGNAL_FD safe_close(ld->signal_read_fd, __FILE__, __LINE__); @@ -91,10 +91,21 @@ free_loop_data(LoopData *ld) { #endif for (size_t i = 0; i < ld->num_handled_signals; i++) signal(ld->num_handled_signals, SIG_DFL); } + ld->signal_read_fd = -1; + ld->num_handled_signals = 0; +} + +void +free_loop_data(LoopData *ld) { +#ifndef HAS_EVENT_FD + CLOSE(wakeup_fds, 0); CLOSE(wakeup_fds, 1); +#endif +#undef CLOSE #ifdef HAS_EVENT_FD safe_close(ld->wakeup_read_fd, __FILE__, __LINE__); #endif - ld->signal_read_fd = -1; ld->wakeup_read_fd = -1; + ld->wakeup_read_fd = -1; + remove_signal_handlers(ld); } @@ -143,7 +154,7 @@ read_signals(int fd, handle_signal_func callback, void *data) { si.si_addr = (void*)(uintptr_t)fdsi[i].ssi_addr; si.si_status = fdsi[i].ssi_status; si.si_value.sival_int = fdsi[i].ssi_int; - callback(&si, data); + if (!callback(&si, data)) break; } } #else @@ -157,8 +168,9 @@ read_signals(int fd, handle_signal_func callback, void *data) { break; } buf_pos += len; - while (buf_pos >= sizeof(siginfo_t)) { - callback((siginfo_t*)buf, data); + bool keep_going = true; + while (keep_going && buf_pos >= sizeof(siginfo_t)) { + keep_going = callback((siginfo_t*)buf, data); memmove(buf, buf + sizeof(siginfo_t), sizeof(siginfo_t)); buf_pos -= sizeof(siginfo_t); } @@ -166,3 +178,89 @@ read_signals(int fd, handle_signal_func callback, void *data) { } #endif } + +static LoopData python_loop_data = {0}; + +static PyObject* +init_signal_handlers_py(PyObject *self UNUSED, PyObject *args) { + if (python_loop_data.num_handled_signals) { PyErr_SetString(PyExc_RuntimeError, "signal handlers already initialized"); return NULL; } +#ifndef HAS_SIGNAL_FD + if (signal_write_fd > -1) { PyErr_SetString(PyExc_RuntimeError, "signal handlers already initialized"); return NULL; } +#endif + for (Py_ssize_t i = 0; i < MIN(PyTuple_GET_SIZE(args), (Py_ssize_t)arraysz(python_loop_data.handled_signals)); i++) { + python_loop_data.handled_signals[python_loop_data.num_handled_signals++] = PyLong_AsLong(PyTuple_GET_ITEM(args, i)); + } + if (!init_signal_handlers(&python_loop_data)) return PyErr_SetFromErrno(PyExc_OSError); +#ifdef HAS_SIGNAL_FD + return Py_BuildValue("ii", python_loop_data.signal_read_fd, -1); +#else + return Py_BuildValue("ii", python_loop_data.signal_fds[0], python_loop_data.signal_fds[1]); +#endif +} + +static PyTypeObject SigInfoType; +static PyStructSequence_Field sig_info_fields[] = { + {"si_signo", "Signal number"}, {"si_code", "Signal code"}, {"si_pid", "Sending Process id"}, + {"si_uid", "Real user id of sending process"}, {"si_addr", "Address of faulting instruction as int"}, + {"si_status", "Exit value or signal"}, {"sival_int", "Signal value as int"}, {"sival_ptr", "Signal value as pointer int"}, + {NULL, NULL} +}; +static PyStructSequence_Desc sig_info_desc = {"SigInfo", NULL, sig_info_fields, 6}; + +static bool +handle_signal_callback_py(const siginfo_t* siginfo, void *data) { + if (PyErr_Occurred()) return false; + PyObject *callback = data; + PyObject *ans = PyStructSequence_New(&SigInfoType); + int pos = 0; +#define S(x) { PyObject *t = x; if (t) { PyStructSequence_SET_ITEM(ans, pos, x); } else { Py_CLEAR(ans); return false; } pos++; } + if (ans) { + S(PyLong_FromLong((long)siginfo->si_signo)); + S(PyLong_FromLong((long)siginfo->si_code)); + S(PyLong_FromLong((long)siginfo->si_pid)); + S(PyLong_FromLong((long)siginfo->si_uid)); + S(PyLong_FromVoidPtr(siginfo->si_addr)); + S(PyLong_FromLong((long)siginfo->si_status)); + S(PyLong_FromLong((long)siginfo->si_value.sival_int)); + S(PyLong_FromVoidPtr(siginfo->si_value.sival_ptr)); + PyObject *ret = PyObject_CallFunctionObjArgs(callback, ans, NULL); + Py_CLEAR(ans); Py_CLEAR(ret); + } + return (PyErr_Occurred()) ? false : true; +#undef S +} + +static PyObject* +read_signals_py(PyObject *self UNUSED, PyObject *args) { + int fd; PyObject *callback; + if (!PyArg_ParseTuple(args, "iO", &fd, &callback)) return NULL; + if (!PyCallable_Check(callback)) { PyErr_SetString(PyExc_TypeError, "callback must be callable"); return NULL; } + read_signals(fd, handle_signal_callback_py, callback); + if (PyErr_Occurred()) return NULL; + Py_RETURN_NONE; +} + + +static PyObject* +remove_signal_handlers_py(PyObject *self UNUSED, PyObject *args UNUSED) { + if (python_loop_data.num_handled_signals) { + remove_signal_handlers(&python_loop_data); + } + Py_RETURN_NONE; +} + +static PyMethodDef methods[] = { + {"install_signal_handlers", init_signal_handlers_py, METH_VARARGS, "Initialize an fd to read signals from" }, + {"read_signals", read_signals_py, METH_VARARGS, "Read pending signals from the specified fd" }, + {"remove_signal_handlers", remove_signal_handlers_py, METH_NOARGS, "Remove signal handlers" }, + { NULL, NULL, 0, NULL }, +}; + +bool +init_loop_utils(PyObject *module) { + if (PyStructSequence_InitType2(&SigInfoType, &sig_info_desc) != 0) return false; + Py_INCREF((PyObject *) &SigInfoType); + PyModule_AddObject(module, "SigInfo", (PyObject *) &SigInfoType); + + return PyModule_AddFunctions(module, methods) == 0; +} diff --git a/kitty/loop-utils.h b/kitty/loop-utils.h index 1cb4c9691..1116bed1e 100644 --- a/kitty/loop-utils.h +++ b/kitty/loop-utils.h @@ -42,7 +42,7 @@ typedef struct { int handled_signals[16]; size_t num_handled_signals; } LoopData; -typedef void(*handle_signal_func)(const siginfo_t* siginfo, void *data); +typedef bool(*handle_signal_func)(const siginfo_t* siginfo, void *data); bool init_loop_data(LoopData *ld, ...); void free_loop_data(LoopData *ld); diff --git a/kitty_tests/prewarm.py b/kitty_tests/prewarm.py index 00e00b675..468e294b3 100644 --- a/kitty_tests/prewarm.py +++ b/kitty_tests/prewarm.py @@ -4,10 +4,14 @@ import json import os +import select +import signal import tempfile -from kitty.constants import kitty_exe -from kitty.fast_data_types import get_options +from kitty.constants import is_macos, kitty_exe +from kitty.fast_data_types import ( + get_options, install_signal_handlers, read_signals, remove_signal_handlers +) from . import BaseTest @@ -55,3 +59,43 @@ import os, json; from kitty.utils import *; from kitty.fast_data_types import ge self.ae(data['env'], env['TEST_ENV_PASS']) self.ae(data['font_family'], 'prewarm') self.ae(int(p.from_worker.readline()), data['pid']) + + def test_signal_handling(self): + import subprocess + expecting_code = 0 + found_signal = False + + def handle_signal(siginfo): + nonlocal found_signal + self.ae(siginfo.si_signo, signal.SIGCHLD) + self.ae(siginfo.si_code, expecting_code) + if expecting_code in (os.CLD_EXITED, os.CLD_KILLED): + p.wait(1) + p.stdin.close() + found_signal = True + + def t(signal, q): + nonlocal expecting_code, found_signal + expecting_code = q + found_signal = False + if signal is not None: + p.send_signal(signal) + if q is not None: + for (fd, event) in poll.poll(4000): + read_signals(signal_read_fd, handle_signal) + self.assertTrue(found_signal, f'Failed to to get SIGCHLD for signal {signal}') + + poll = select.poll() + p = subprocess.Popen([kitty_exe(), '+runpy', 'input()'], stderr=subprocess.DEVNULL, stdin=subprocess.PIPE) + signal_read_fd = install_signal_handlers(signal.SIGCHLD)[0] + try: + poll.register(signal_read_fd, select.POLLIN) + t(signal.SIGTSTP, os.CLD_STOPPED) + # macOS doesnt send SIGCHLD for SIGCONT. This is not required by POSIX sadly + t(signal.SIGCONT, None if is_macos else os.CLD_CONTINUED) + t(signal.SIGINT, os.CLD_KILLED) + p = subprocess.Popen([kitty_exe(), '+runpy', 'input()'], stderr=subprocess.DEVNULL, stdin=subprocess.PIPE) + p.stdin.close() + t(None, os.CLD_EXITED) + finally: + remove_signal_handlers()