From 92498b8a0e1d50c7e06d435395ac0ac307cc190c Mon Sep 17 00:00:00 2001 From: Sven Trenkel Date: Sat, 6 Aug 2016 15:42:58 +0000 Subject: [PATCH] python plugin: Fix SIGINT handling. --- src/collectd-python.pod | 9 ++++---- src/python.c | 60 ++++++++++++++++++++++--------------------------- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/collectd-python.pod b/src/collectd-python.pod index 4647a114..e0851d90 100644 --- a/src/collectd-python.pod +++ b/src/collectd-python.pod @@ -94,11 +94,12 @@ way of entering your commands. The daemonized collectd won't do that. =item * -B<2.> collectd will block I. Pressing I will usually cause +B<2.> Python will be handling I. Pressing I will usually cause collectd to shut down. This would be problematic in an interactive session, -therefore this signal will be blocked. You can still use it to interrupt -syscalls like sleep and pause but it won't generate a I -exception either. +therefore Python will be handling it in interactive sessions. This allows you +to use I to interrupt Python code without killing collectd. This also +means you can catch I exceptions which does not work during +normal operation. To quit collectd send I (press I at the beginning of a new line). diff --git a/src/python.c b/src/python.c index b991f45f..912c18ae 100644 --- a/src/python.c +++ b/src/python.c @@ -219,7 +219,10 @@ static char reg_shutdown_doc[] = "register_shutdown(callback[, data][, name]) -> " data if it was supplied."; +static pthread_t main_thread; +static PyOS_sighandler_t python_sigint_handler; static _Bool do_interactive = 0; +static int do_interactive = 0; /* This is our global thread state. Python saves some stuff in thread-local * storage. So if we allow the interpreter to run in the background @@ -910,13 +913,8 @@ static int cpy_shutdown(void) { return 0; } -static void cpy_int_handler(int sig) { - return; -} - static void *cpy_interactive(void *data) { - sigset_t sigset; - struct sigaction old; + PyOS_sighandler_t cur_sig; /* Signal handler in a plugin? Bad stuff, but the best way to * handle it I guess. In an interactive session people will @@ -926,46 +924,40 @@ static void *cpy_interactive(void *data) { * mess. Chances are, this isn't what the user wanted to do. * * So this is the plan: - * 1. Block SIGINT in the main thread. - * 2. Install our own signal handler that does nothing. - * 3. Unblock SIGINT in the interactive thread. + * 1. Restore Python's own signal handler + * 2. Tell Python we just forked so it will accept this thread + * as the main one. No version of Python will ever handle + * interrupts anywhere but in the main thread. + * 3. After the interactive loop is done, restore collectd's + * SIGINT handler. + * 4. Raise SIGINT for a clean shutdown. The signal is sent to + * the main thread to ensure it wakes up the main interval + * sleep so that collectd shuts down immediately not in 10 + * seconds. * * This will make sure that SIGINT won't kill collectd but - * still interrupt syscalls like sleep and pause. - * It does not raise a KeyboardInterrupt exception because so - * far nobody managed to figure out how to do that. */ - struct sigaction sig_int_action = { - .sa_handler = cpy_int_handler - }; - sigaction (SIGINT, &sig_int_action, &old); - - sigemptyset(&sigset); - sigaddset(&sigset, SIGINT); - pthread_sigmask(SIG_UNBLOCK, &sigset, NULL); + * still interrupt syscalls like sleep and pause. */ + PyEval_AcquireThread(state); if (PyImport_ImportModule("readline") == NULL) { /* This interactive session will suck. */ cpy_log_exception("interactive session init"); - } + } + cur_sig = PyOS_setsig(SIGINT, python_sigint_handler); + /* We totally forked just now. Everyone saw that, right? */ + PyOS_AfterFork(); PyRun_InteractiveLoop(stdin, ""); + PyOS_setsig(SIGINT, cur_sig); PyErr_Print(); PyEval_ReleaseThread(state); NOTICE("python: Interactive interpreter exited, stopping collectd ..."); - /* Restore the original collectd SIGINT handler and raise SIGINT. - * The main thread still has SIGINT blocked and there's nothing we - * can do about that so this thread will handle it. But that's not - * important, except that it won't interrupt the main loop and so - * it might take a few seconds before collectd really shuts down. */ - sigaction (SIGINT, &old, NULL); - raise(SIGINT); - pause(); + pthread_kill(main_thread, SIGINT); return NULL; } static int cpy_init(void) { PyObject *ret; static pthread_t thread; - sigset_t sigset; if (!Py_IsInitialized()) { WARNING("python: Plugin loaded but not configured."); @@ -981,10 +973,8 @@ static int cpy_init(void) { else Py_DECREF(ret); } - sigemptyset(&sigset); - sigaddset(&sigset, SIGINT); - pthread_sigmask(SIG_BLOCK, &sigset, NULL); state = PyEval_SaveThread(); + main_thread = pthread_self(); if (do_interactive) { if (plugin_thread_create(&thread, NULL, cpy_interactive, NULL)) { ERROR("python: Error creating thread for interactive interpreter."); @@ -1040,6 +1030,7 @@ PyMODINIT_FUNC PyInit_collectd(void) { #endif static int cpy_init_python(void) { + PyOS_sighandler_t cur_sig; PyObject *sys; PyObject *module; @@ -1051,7 +1042,10 @@ static int cpy_init_python(void) { char *argv = ""; #endif + /* Chances are the current signal handler is already SIG_DFL, but let's make sure. */ + cur_sig = PyOS_setsig(SIGINT, SIG_DFL); Py_Initialize(); + python_sigint_handler = PyOS_setsig(SIGINT, cur_sig); PyType_Ready(&ConfigType); PyType_Ready(&PluginDataType); -- 2.11.0