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().
diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index 7cd6666..955d4a5 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;
@@ -1380,251 +1376,222 @@ 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;
}