From cda23be092f4a72e4f335cf182f11e7bd7fd98eb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 17 Nov 2020 18:57:32 +0100 Subject: [PATCH] bpo-41686: Refactor signal_exec() (GH-23346) * Add signal_add_constants() function and add ADD_INT_MACRO macro. * The Python SIGINT handler is now installed at the end of signal_exec(). * Use Py_NewRef(). --- Modules/signalmodule.c | 453 +++++++++++++++++++---------------------- 1 file changed, 210 insertions(+), 243 deletions(-) diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 7cd6666ede8..955d4a56e54 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -248,10 +248,6 @@ report_wakeup_send_error(void* data) static void trip_signal(int sig_num) { - unsigned char byte; - int fd; - Py_ssize_t rc; - _Py_atomic_store_relaxed(&Handlers[sig_num].tripped, 1); /* Set is_tripped after setting .tripped, as it gets @@ -283,6 +279,7 @@ trip_signal(int sig_num) See bpo-30038 for more details. */ + int fd; #ifdef MS_WINDOWS fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); #else @@ -290,10 +287,10 @@ trip_signal(int sig_num) #endif if (fd != INVALID_FD) { - byte = (unsigned char)sig_num; + unsigned char byte = (unsigned char)sig_num; #ifdef MS_WINDOWS if (wakeup.use_send) { - rc = send(fd, &byte, 1, 0); + Py_ssize_t rc = send(fd, &byte, 1, 0); if (rc < 0) { int last_error = GetLastError(); @@ -313,7 +310,7 @@ trip_signal(int sig_num) { /* _Py_write_noraise() retries write() if write() is interrupted by a signal (fails with EINTR). */ - rc = _Py_write_noraise(fd, &byte, 1); + Py_ssize_t rc = _Py_write_noraise(fd, &byte, 1); if (rc < 0) { if (wakeup.warn_on_full_buffer || @@ -516,8 +513,7 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler) } old_handler = Handlers[signalnum].func; - Py_INCREF(handler); - Handlers[signalnum].func = handler; + Handlers[signalnum].func = Py_NewRef(handler); if (old_handler != NULL) { return old_handler; @@ -555,8 +551,7 @@ signal_getsignal_impl(PyObject *module, int signalnum) } old_handler = Handlers[signalnum].func; if (old_handler != NULL) { - Py_INCREF(old_handler); - return old_handler; + return Py_NewRef(old_handler); } else { Py_RETURN_NONE; @@ -711,7 +706,7 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds) if (sockfd == (SOCKET_T)(-1) && PyErr_Occurred()) return NULL; #else - int fd, old_fd; + int fd; if (!PyArg_ParseTupleAndKeywords(args, kwds, "i|$p:set_wakeup_fd", kwlist, &fd, &warn_on_full_buffer)) @@ -793,7 +788,7 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args, PyObject *kwds) } } - old_fd = wakeup.fd; + int old_fd = wakeup.fd; wakeup.fd = fd; wakeup.warn_on_full_buffer = warn_on_full_buffer; @@ -814,14 +809,14 @@ The fd must be non-blocking."); int PySignal_SetWakeupFd(int fd) { - int old_fd; - if (fd < 0) + if (fd < 0) { fd = -1; + } #ifdef MS_WINDOWS - old_fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); + int old_fd = Py_SAFE_DOWNCAST(wakeup.fd, SOCKET_T, int); #else - old_fd = wakeup.fd; + int old_fd = wakeup.fd; #endif wakeup.fd = fd; wakeup.warn_on_full_buffer = 1; @@ -852,7 +847,7 @@ signal_setitimer_impl(PyObject *module, int which, PyObject *seconds, PyObject *interval) /*[clinic end generated code: output=65f9dcbddc35527b input=de43daf194e6f66f]*/ { - struct itimerval new, old; + struct itimerval new; if (timeval_from_double(seconds, &new.it_value) < 0) { return NULL; @@ -862,6 +857,7 @@ signal_setitimer_impl(PyObject *module, int which, PyObject *seconds, } /* Let OS check "which" value */ + struct itimerval old; if (setitimer(which, &new, &old) != 0) { PyErr_SetFromErrno(ItimerError); return NULL; @@ -1379,252 +1375,223 @@ the first is the signal number, the second is the interrupted stack frame."); +static int +signal_add_constants(PyObject *module) +{ +#define ADD_INT_MACRO(macro) \ + if (PyModule_AddIntConstant(module, #macro, macro) < 0) { \ + return -1; \ + } + + ADD_INT_MACRO(NSIG); + + // SIG_xxx pthread_sigmask() constants +#ifdef SIG_BLOCK + ADD_INT_MACRO(SIG_BLOCK); +#endif +#ifdef SIG_UNBLOCK + ADD_INT_MACRO(SIG_UNBLOCK); +#endif +#ifdef SIG_SETMASK + ADD_INT_MACRO(SIG_SETMASK); +#endif + + // SIGxxx signal number constants +#ifdef SIGHUP + ADD_INT_MACRO(SIGHUP); +#endif +#ifdef SIGINT + ADD_INT_MACRO(SIGINT); +#endif +#ifdef SIGBREAK + ADD_INT_MACRO(SIGBREAK); +#endif +#ifdef SIGQUIT + ADD_INT_MACRO(SIGQUIT); +#endif +#ifdef SIGILL + ADD_INT_MACRO(SIGILL); +#endif +#ifdef SIGTRAP + ADD_INT_MACRO(SIGTRAP); +#endif +#ifdef SIGIOT + ADD_INT_MACRO(SIGIOT); +#endif +#ifdef SIGABRT + ADD_INT_MACRO(SIGABRT); +#endif +#ifdef SIGEMT + ADD_INT_MACRO(SIGEMT); +#endif +#ifdef SIGFPE + ADD_INT_MACRO(SIGFPE); +#endif +#ifdef SIGKILL + ADD_INT_MACRO(SIGKILL); +#endif +#ifdef SIGBUS + ADD_INT_MACRO(SIGBUS); +#endif +#ifdef SIGSEGV + ADD_INT_MACRO(SIGSEGV); +#endif +#ifdef SIGSYS + ADD_INT_MACRO(SIGSYS); +#endif +#ifdef SIGPIPE + ADD_INT_MACRO(SIGPIPE); +#endif +#ifdef SIGALRM + ADD_INT_MACRO(SIGALRM); +#endif +#ifdef SIGTERM + ADD_INT_MACRO(SIGTERM); +#endif +#ifdef SIGUSR1 + ADD_INT_MACRO(SIGUSR1); +#endif +#ifdef SIGUSR2 + ADD_INT_MACRO(SIGUSR2); +#endif +#ifdef SIGCLD + ADD_INT_MACRO(SIGCLD); +#endif +#ifdef SIGCHLD + ADD_INT_MACRO(SIGCHLD); +#endif +#ifdef SIGPWR + ADD_INT_MACRO(SIGPWR); +#endif +#ifdef SIGIO + ADD_INT_MACRO(SIGIO); +#endif +#ifdef SIGURG + ADD_INT_MACRO(SIGURG); +#endif +#ifdef SIGWINCH + ADD_INT_MACRO(SIGWINCH); +#endif +#ifdef SIGPOLL + ADD_INT_MACRO(SIGPOLL); +#endif +#ifdef SIGSTOP + ADD_INT_MACRO(SIGSTOP); +#endif +#ifdef SIGTSTP + ADD_INT_MACRO(SIGTSTP); +#endif +#ifdef SIGCONT + ADD_INT_MACRO(SIGCONT); +#endif +#ifdef SIGTTIN + ADD_INT_MACRO(SIGTTIN); +#endif +#ifdef SIGTTOU + ADD_INT_MACRO(SIGTTOU); +#endif +#ifdef SIGVTALRM + ADD_INT_MACRO(SIGVTALRM); +#endif +#ifdef SIGPROF + ADD_INT_MACRO(SIGPROF); +#endif +#ifdef SIGXCPU + ADD_INT_MACRO(SIGXCPU); +#endif +#ifdef SIGXFSZ + ADD_INT_MACRO(SIGXFSZ); +#endif +#ifdef SIGRTMIN + ADD_INT_MACRO(SIGRTMIN); +#endif +#ifdef SIGRTMAX + ADD_INT_MACRO(SIGRTMAX); +#endif +#ifdef SIGINFO + ADD_INT_MACRO(SIGINFO); +#endif + + // ITIMER_xxx constants +#ifdef ITIMER_REAL + ADD_INT_MACRO(ITIMER_REAL); +#endif +#ifdef ITIMER_VIRTUAL + ADD_INT_MACRO(ITIMER_VIRTUAL); +#endif +#ifdef ITIMER_PROF + ADD_INT_MACRO(ITIMER_PROF); +#endif + + // CTRL_xxx Windows signals +#ifdef CTRL_C_EVENT + ADD_INT_MACRO(CTRL_C_EVENT); +#endif +#ifdef CTRL_BREAK_EVENT + ADD_INT_MACRO(CTRL_BREAK_EVENT); +#endif + + return 0; + +#undef ADD_INT_MACRO +} + + static int signal_exec(PyObject *m) { - /* add the functions */ + assert(!PyErr_Occurred()); + + if (signal_add_constants(m) < 0) { + return -1; + } + + /* Add some symbolic constants to the module */ + PyObject *d = PyModule_GetDict(m); + if (PyDict_SetItemString(d, "SIG_DFL", DefaultHandler) < 0) { + return -1; + } + if (PyDict_SetItemString(d, "SIG_IGN", IgnoreHandler) < 0) { + return -1; + } +#if defined(HAVE_GETITIMER) || defined(HAVE_SETITIMER) + if (PyDict_SetItemString(d, "ItimerError", ItimerError) < 0) { + return -1; + } +#endif #if defined(HAVE_SIGWAITINFO) || defined(HAVE_SIGTIMEDWAIT) if (PyModule_AddType(m, &SiginfoType) < 0) { return -1; } #endif - /* Add some symbolic constants to the module */ - PyObject *d = PyModule_GetDict(m); - - if (PyDict_SetItemString(d, "SIG_DFL", DefaultHandler) < 0) { - return -1; + // Get signal handlers + for (int signum = 1; signum < NSIG; signum++) { + void (*c_handler)(int) = PyOS_getsig(signum); + if (c_handler == SIG_DFL) { + Handlers[signum].func = Py_NewRef(DefaultHandler); + } + else if (c_handler == SIG_IGN) { + Handlers[signum].func = Py_NewRef(IgnoreHandler); + } + else { + Handlers[signum].func = Py_NewRef(Py_None); // None of our business + } } - if (PyDict_SetItemString(d, "SIG_IGN", IgnoreHandler) < 0) { - return -1; - } - - if (PyModule_AddIntMacro(m, NSIG)) - return -1; - -#ifdef SIG_BLOCK - if (PyModule_AddIntMacro(m, SIG_BLOCK)) - return -1; -#endif -#ifdef SIG_UNBLOCK - if (PyModule_AddIntMacro(m, SIG_UNBLOCK)) - return -1; -#endif -#ifdef SIG_SETMASK - if (PyModule_AddIntMacro(m, SIG_SETMASK)) - return -1; -#endif - - for (int i = 1; i < NSIG; i++) { - void (*t)(int); - t = PyOS_getsig(i); - if (t == SIG_DFL) - Handlers[i].func = DefaultHandler; - else if (t == SIG_IGN) - Handlers[i].func = IgnoreHandler; - else - Handlers[i].func = Py_None; /* None of our business */ - Py_INCREF(Handlers[i].func); - } + // Instal Python SIGINT handler which raises KeyboardInterrupt if (Handlers[SIGINT].func == DefaultHandler) { PyObject *int_handler = PyMapping_GetItemString(d, "default_int_handler"); if (!int_handler) { return -1; } - /* Install default int handler */ Py_SETREF(Handlers[SIGINT].func, int_handler); PyOS_setsig(SIGINT, signal_handler); } -#ifdef SIGHUP - if (PyModule_AddIntMacro(m, SIGHUP)) - return -1; -#endif -#ifdef SIGINT - if (PyModule_AddIntMacro(m, SIGINT)) - return -1; -#endif -#ifdef SIGBREAK - if (PyModule_AddIntMacro(m, SIGBREAK)) - return -1; -#endif -#ifdef SIGQUIT - if (PyModule_AddIntMacro(m, SIGQUIT)) - return -1; -#endif -#ifdef SIGILL - if (PyModule_AddIntMacro(m, SIGILL)) - return -1; -#endif -#ifdef SIGTRAP - if (PyModule_AddIntMacro(m, SIGTRAP)) - return -1; -#endif -#ifdef SIGIOT - if (PyModule_AddIntMacro(m, SIGIOT)) - return -1; -#endif -#ifdef SIGABRT - if (PyModule_AddIntMacro(m, SIGABRT)) - return -1; -#endif -#ifdef SIGEMT - if (PyModule_AddIntMacro(m, SIGEMT)) - return -1; -#endif -#ifdef SIGFPE - if (PyModule_AddIntMacro(m, SIGFPE)) - return -1; -#endif -#ifdef SIGKILL - if (PyModule_AddIntMacro(m, SIGKILL)) - return -1; -#endif -#ifdef SIGBUS - if (PyModule_AddIntMacro(m, SIGBUS)) - return -1; -#endif -#ifdef SIGSEGV - if (PyModule_AddIntMacro(m, SIGSEGV)) - return -1; -#endif -#ifdef SIGSYS - if (PyModule_AddIntMacro(m, SIGSYS)) - return -1; -#endif -#ifdef SIGPIPE - if (PyModule_AddIntMacro(m, SIGPIPE)) - return -1; -#endif -#ifdef SIGALRM - if (PyModule_AddIntMacro(m, SIGALRM)) - return -1; -#endif -#ifdef SIGTERM - if (PyModule_AddIntMacro(m, SIGTERM)) - return -1; -#endif -#ifdef SIGUSR1 - if (PyModule_AddIntMacro(m, SIGUSR1)) - return -1; -#endif -#ifdef SIGUSR2 - if (PyModule_AddIntMacro(m, SIGUSR2)) - return -1; -#endif -#ifdef SIGCLD - if (PyModule_AddIntMacro(m, SIGCLD)) - return -1; -#endif -#ifdef SIGCHLD - if (PyModule_AddIntMacro(m, SIGCHLD)) - return -1; -#endif -#ifdef SIGPWR - if (PyModule_AddIntMacro(m, SIGPWR)) - return -1; -#endif -#ifdef SIGIO - if (PyModule_AddIntMacro(m, SIGIO)) - return -1; -#endif -#ifdef SIGURG - if (PyModule_AddIntMacro(m, SIGURG)) - return -1; -#endif -#ifdef SIGWINCH - if (PyModule_AddIntMacro(m, SIGWINCH)) - return -1; -#endif -#ifdef SIGPOLL - if (PyModule_AddIntMacro(m, SIGPOLL)) - return -1; -#endif -#ifdef SIGSTOP - if (PyModule_AddIntMacro(m, SIGSTOP)) - return -1; -#endif -#ifdef SIGTSTP - if (PyModule_AddIntMacro(m, SIGTSTP)) - return -1; -#endif -#ifdef SIGCONT - if (PyModule_AddIntMacro(m, SIGCONT)) - return -1; -#endif -#ifdef SIGTTIN - if (PyModule_AddIntMacro(m, SIGTTIN)) - return -1; -#endif -#ifdef SIGTTOU - if (PyModule_AddIntMacro(m, SIGTTOU)) - return -1; -#endif -#ifdef SIGVTALRM - if (PyModule_AddIntMacro(m, SIGVTALRM)) - return -1; -#endif -#ifdef SIGPROF - if (PyModule_AddIntMacro(m, SIGPROF)) - return -1; -#endif -#ifdef SIGXCPU - if (PyModule_AddIntMacro(m, SIGXCPU)) - return -1; -#endif -#ifdef SIGXFSZ - if (PyModule_AddIntMacro(m, SIGXFSZ)) - return -1; -#endif -#ifdef SIGRTMIN - if (PyModule_AddIntMacro(m, SIGRTMIN)) - return -1; -#endif -#ifdef SIGRTMAX - if (PyModule_AddIntMacro(m, SIGRTMAX)) - return -1; -#endif -#ifdef SIGINFO - if (PyModule_AddIntMacro(m, SIGINFO)) - return -1; -#endif - -#ifdef ITIMER_REAL - if (PyModule_AddIntMacro(m, ITIMER_REAL)) - return -1; -#endif -#ifdef ITIMER_VIRTUAL - if (PyModule_AddIntMacro(m, ITIMER_VIRTUAL)) - return -1; -#endif -#ifdef ITIMER_PROF - if (PyModule_AddIntMacro(m, ITIMER_PROF)) - return -1; -#endif - -#if defined(HAVE_GETITIMER) || defined(HAVE_SETITIMER) - if (PyDict_SetItemString(d, "ItimerError", ItimerError) < 0) { - return -1; - } -#endif - -#ifdef CTRL_C_EVENT - if (PyModule_AddIntMacro(m, CTRL_C_EVENT)) - return -1; -#endif - -#ifdef CTRL_BREAK_EVENT - if (PyModule_AddIntMacro(m, CTRL_BREAK_EVENT)) - return -1; -#endif - - if (PyErr_Occurred()) { - return -1; - } - - return 0; + assert(!PyErr_Occurred()); + return 0; }