[Python-checkins] bpo-43356: Allow passing a signal number to interrupt_main() (GH-24755)

pitrou webhook-mailer at python.org
Thu Mar 11 17:36:02 EST 2021


https://github.com/python/cpython/commit/ba251c2ae6654bfc8abd9d886b219698ad34ac3c
commit: ba251c2ae6654bfc8abd9d886b219698ad34ac3c
branch: master
author: Antoine Pitrou <antoine at python.org>
committer: pitrou <pitrou at free.fr>
date: 2021-03-11T23:35:45+01:00
summary:

bpo-43356: Allow passing a signal number to interrupt_main() (GH-24755)

Also introduce a new C API ``PyErr_SetInterruptEx(int signum)``.

files:
A Misc/NEWS.d/next/Library/2021-03-04-21-51-20.bpo-43356.X7IGBM.rst
M Doc/c-api/exceptions.rst
M Doc/data/stable_abi.dat
M Doc/library/_thread.rst
M Doc/whatsnew/3.10.rst
M Include/internal/pycore_pylifecycle.h
M Include/pyerrors.h
M Lib/test/test_threading.py
M Modules/_threadmodule.c
M Modules/signalmodule.c
M PC/python3dll.c

diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst
index 4e99a0167a632..158672d86b325 100644
--- a/Doc/c-api/exceptions.rst
+++ b/Doc/c-api/exceptions.rst
@@ -505,29 +505,73 @@ Signal Handling
       single: SIGINT
       single: KeyboardInterrupt (built-in exception)
 
-   This function interacts with Python's signal handling.  It checks whether a
-   signal has been sent to the processes and if so, invokes the corresponding
-   signal handler.  If the :mod:`signal` module is supported, this can invoke a
-   signal handler written in Python.  In all cases, the default effect for
-   :const:`SIGINT` is to raise the  :exc:`KeyboardInterrupt` exception.  If an
-   exception is raised the error indicator is set and the function returns ``-1``;
-   otherwise the function returns ``0``.  The error indicator may or may not be
-   cleared if it was previously set.
+   This function interacts with Python's signal handling.
+
+   If the function is called from the main thread and under the main Python
+   interpreter, it checks whether a signal has been sent to the processes
+   and if so, invokes the corresponding signal handler.  If the :mod:`signal`
+   module is supported, this can invoke a signal handler written in Python.
+
+   The function attemps to handle all pending signals, and then returns ``0``.
+   However, if a Python signal handler raises an exception, the error
+   indicator is set and the function returns ``-1`` immediately (such that
+   other pending signals may not have been handled yet: they will be on the
+   next :c:func:`PyErr_CheckSignals()` invocation).
+
+   If the function is called from a non-main thread, or under a non-main
+   Python interpreter, it does nothing and returns ``0``.
+
+   This function can be called by long-running C code that wants to
+   be interruptible by user requests (such as by pressing Ctrl-C).
+
+   .. note::
+      The default Python signal handler for :const:`SIGINT` raises the
+      :exc:`KeyboardInterrupt` exception.
 
 
 .. c:function:: void PyErr_SetInterrupt()
 
    .. index::
+      module: signal
       single: SIGINT
       single: KeyboardInterrupt (built-in exception)
 
-   Simulate the effect of a :const:`SIGINT` signal arriving. The next time
+   Simulate the effect of a :const:`SIGINT` signal arriving.
+   This is equivalent to ``PyErr_SetInterruptEx(SIGINT)``.
+
+   .. note::
+      This function is async-signal-safe.  It can be called without
+      the :term:`GIL` and from a C signal handler.
+
+
+.. c:function:: int PyErr_SetInterruptEx(int signum)
+
+   .. index::
+      module: signal
+      single: KeyboardInterrupt (built-in exception)
+
+   Simulate the effect of a signal arriving. The next time
    :c:func:`PyErr_CheckSignals` is called,  the Python signal handler for
-   :const:`SIGINT` will be called.
+   the given signal number will be called.
+
+   This function can be called by C code that sets up its own signal handling
+   and wants Python signal handlers to be invoked as expected when an
+   interruption is requested (for example when the user presses Ctrl-C
+   to interrupt an operation).
+
+   If the given signal isn't handled by Python (it was set to
+   :data:`signal.SIG_DFL` or :data:`signal.SIG_IGN`), it will be ignored.
+
+   If *signum* is outside of the allowed range of signal numbers, ``-1``
+   is returned.  Otherwise, ``0`` is returned.  The error indicator is
+   never changed by this function.
+
+   .. note::
+      This function is async-signal-safe.  It can be called without
+      the :term:`GIL` and from a C signal handler.
+
+   .. versionadded:: 3.10
 
-   If :const:`SIGINT` isn't handled by Python (it was set to
-   :data:`signal.SIG_DFL` or :data:`signal.SIG_IGN`), this function does
-   nothing.
 
 .. c:function:: int PySignal_SetWakeupFd(int fd)
 
diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat
index c2c9c6e11e4ce..906b0a72d1368 100644
--- a/Doc/data/stable_abi.dat
+++ b/Doc/data/stable_abi.dat
@@ -36,6 +36,7 @@ PyCFunction_Call
 PyCFunction_GetFlags
 PyCFunction_GetFunction
 PyCFunction_GetSelf
+PyCFunction_New
 PyCFunction_NewEx
 PyCFunction_Type
 PyCMethod_New
@@ -144,6 +145,7 @@ PyErr_SetFromErrnoWithFilenameObjects
 PyErr_SetImportError
 PyErr_SetImportErrorSubclass
 PyErr_SetInterrupt
+PyErr_SetInterruptEx
 PyErr_SetNone
 PyErr_SetObject
 PyErr_SetString
diff --git a/Doc/library/_thread.rst b/Doc/library/_thread.rst
index bd653ab32bb9c..1e6452b7b826f 100644
--- a/Doc/library/_thread.rst
+++ b/Doc/library/_thread.rst
@@ -61,15 +61,27 @@ This module defines the following constants and functions:
       :func:`sys.unraisablehook` is now used to handle unhandled exceptions.
 
 
-.. function:: interrupt_main()
+.. function:: interrupt_main(signum=signal.SIGINT, /)
 
-   Simulate the effect of a :data:`signal.SIGINT` signal arriving in the main
-   thread. A thread can use this function to interrupt the main thread.
+   Simulate the effect of a signal arriving in the main thread.
+   A thread can use this function to interrupt the main thread, though
+   there is no guarantee that the interruption will happen immediately.
 
-   If :data:`signal.SIGINT` isn't handled by Python (it was set to
+   If given, *signum* is the number of the signal to simulate.
+   If *signum* is not given, :data:`signal.SIGINT` is simulated.
+
+   If the given signal isn't handled by Python (it was set to
    :data:`signal.SIG_DFL` or :data:`signal.SIG_IGN`), this function does
    nothing.
 
+   .. versionchanged:: 3.10
+      The *signum* argument is added to customize the signal number.
+
+   .. note::
+      This does not emit the corresponding signal but schedules a call to
+      the associated handler (if it exists).
+      If you want to truly emit the signal, use :func:`signal.raise_signal`.
+
 
 .. function:: exit()
 
diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst
index 814594a052595..db36f69e8d711 100644
--- a/Doc/whatsnew/3.10.rst
+++ b/Doc/whatsnew/3.10.rst
@@ -788,6 +788,13 @@ Add :data:`sys.stdlib_module_names`, containing the list of the standard library
 module names.
 (Contributed by Victor Stinner in :issue:`42955`.)
 
+_thread
+-------
+
+:func:`_thread.interrupt_main` now takes an optional signal number to
+simulate (the default is still :data:`signal.SIGINT`).
+(Contributed by Antoine Pitrou in :issue:`43356`.)
+
 threading
 ---------
 
@@ -1210,6 +1217,11 @@ New Features
   object is an instance of :class:`set` but not an instance of a subtype.
   (Contributed by Pablo Galindo in :issue:`43277`.)
 
+* Added :c:func:`PyErr_SetInterruptEx` which allows passing a signal number
+  to simulate.
+  (Contributed by Antoine Pitrou in :issue:`43356`.)
+
+
 Porting to Python 3.10
 ----------------------
 
diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h
index a7d84a9b5883c..7ae107d73dae3 100644
--- a/Include/internal/pycore_pylifecycle.h
+++ b/Include/internal/pycore_pylifecycle.h
@@ -8,8 +8,24 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif
 
+#ifdef HAVE_SIGNAL_H
+#include <signal.h>
+#endif
+
 #include "pycore_runtime.h"       // _PyRuntimeState
 
+#ifndef NSIG
+# if defined(_NSIG)
+#  define NSIG _NSIG            /* For BSD/SysV */
+# elif defined(_SIGMAX)
+#  define NSIG (_SIGMAX + 1)    /* For QNX */
+# elif defined(SIGMAX)
+#  define NSIG (SIGMAX + 1)     /* For djgpp */
+# else
+#  define NSIG 64               /* Use a reasonable default value */
+# endif
+#endif
+
 /* Forward declarations */
 struct _PyArgv;
 struct pyruntimestate;
diff --git a/Include/pyerrors.h b/Include/pyerrors.h
index 692d67175741e..14129d3533cbe 100644
--- a/Include/pyerrors.h
+++ b/Include/pyerrors.h
@@ -224,6 +224,9 @@ PyAPI_FUNC(void) PyErr_WriteUnraisable(PyObject *);
 /* In signalmodule.c */
 PyAPI_FUNC(int) PyErr_CheckSignals(void);
 PyAPI_FUNC(void) PyErr_SetInterrupt(void);
+#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000
+PyAPI_FUNC(int) PyErr_SetInterruptEx(int signum);
+#endif
 
 /* Support for adding program text to SyntaxErrors */
 PyAPI_FUNC(void) PyErr_SyntaxLocation(
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index a7a716ed59fc4..933935ba2ce2c 100644
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -1489,6 +1489,29 @@ def test__all__(self):
 
 
 class InterruptMainTests(unittest.TestCase):
+    def check_interrupt_main_with_signal_handler(self, signum):
+        def handler(signum, frame):
+            1/0
+
+        old_handler = signal.signal(signum, handler)
+        self.addCleanup(signal.signal, signum, old_handler)
+
+        with self.assertRaises(ZeroDivisionError):
+            _thread.interrupt_main()
+
+    def check_interrupt_main_noerror(self, signum):
+        handler = signal.getsignal(signum)
+        try:
+            # No exception should arise.
+            signal.signal(signum, signal.SIG_IGN)
+            _thread.interrupt_main(signum)
+
+            signal.signal(signum, signal.SIG_DFL)
+            _thread.interrupt_main(signum)
+        finally:
+            # Restore original handler
+            signal.signal(signum, handler)
+
     def test_interrupt_main_subthread(self):
         # Calling start_new_thread with a function that executes interrupt_main
         # should raise KeyboardInterrupt upon completion.
@@ -1506,18 +1529,18 @@ def test_interrupt_main_mainthread(self):
         with self.assertRaises(KeyboardInterrupt):
             _thread.interrupt_main()
 
+    def test_interrupt_main_with_signal_handler(self):
+        self.check_interrupt_main_with_signal_handler(signal.SIGINT)
+        self.check_interrupt_main_with_signal_handler(signal.SIGTERM)
+
     def test_interrupt_main_noerror(self):
-        handler = signal.getsignal(signal.SIGINT)
-        try:
-            # No exception should arise.
-            signal.signal(signal.SIGINT, signal.SIG_IGN)
-            _thread.interrupt_main()
+        self.check_interrupt_main_noerror(signal.SIGINT)
+        self.check_interrupt_main_noerror(signal.SIGTERM)
 
-            signal.signal(signal.SIGINT, signal.SIG_DFL)
-            _thread.interrupt_main()
-        finally:
-            # Restore original handler
-            signal.signal(signal.SIGINT, handler)
+    def test_interrupt_main_invalid_signal(self):
+        self.assertRaises(ValueError, _thread.interrupt_main, -1)
+        self.assertRaises(ValueError, _thread.interrupt_main, signal.NSIG)
+        self.assertRaises(ValueError, _thread.interrupt_main, 1000000)
 
 
 class AtexitTests(unittest.TestCase):
diff --git a/Misc/NEWS.d/next/Library/2021-03-04-21-51-20.bpo-43356.X7IGBM.rst b/Misc/NEWS.d/next/Library/2021-03-04-21-51-20.bpo-43356.X7IGBM.rst
new file mode 100644
index 0000000000000..8b106f830f59b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-03-04-21-51-20.bpo-43356.X7IGBM.rst
@@ -0,0 +1 @@
+Allow passing a signal number to ``_thread.interrupt_main()``.
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index f6217e672f7aa..0613dfd3070c5 100644
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -9,6 +9,10 @@
 #include <stddef.h>               // offsetof()
 #include "structmember.h"         // PyMemberDef
 
+#ifdef HAVE_SIGNAL_H
+#  include <signal.h>             // SIGINT
+#endif
+
 // ThreadError is just an alias to PyExc_RuntimeError
 #define ThreadError PyExc_RuntimeError
 
@@ -1173,17 +1177,29 @@ This is synonymous to ``raise SystemExit''.  It will cause the current\n\
 thread to exit silently unless the exception is caught.");
 
 static PyObject *
-thread_PyThread_interrupt_main(PyObject * self, PyObject *Py_UNUSED(ignored))
+thread_PyThread_interrupt_main(PyObject *self, PyObject *args)
 {
-    PyErr_SetInterrupt();
+    int signum = SIGINT;
+    if (!PyArg_ParseTuple(args, "|i:signum", &signum)) {
+        return NULL;
+    }
+
+    if (PyErr_SetInterruptEx(signum)) {
+        PyErr_SetString(PyExc_ValueError, "signal number out of range");
+        return NULL;
+    }
     Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(interrupt_doc,
-"interrupt_main()\n\
+"interrupt_main(signum=signal.SIGINT, /)\n\
+\n\
+Simulate the arrival of the given signal in the main thread,\n\
+where the corresponding signal handler will be executed.\n\
+If *signum* is omitted, SIGINT is assumed.\n\
+A subthread can use this function to interrupt the main thread.\n\
 \n\
-Raise a KeyboardInterrupt in the main thread.\n\
-A subthread can use this function to interrupt the main thread."
+Note: the default signal hander for SIGINT raises ``KeyboardInterrupt``."
 );
 
 static lockobject *newlockobject(PyObject *module);
@@ -1527,8 +1543,8 @@ static PyMethodDef thread_methods[] = {
      METH_NOARGS, exit_doc},
     {"exit",                    thread_PyThread_exit_thread,
      METH_NOARGS, exit_doc},
-    {"interrupt_main",          thread_PyThread_interrupt_main,
-     METH_NOARGS, interrupt_doc},
+    {"interrupt_main",          (PyCFunction)thread_PyThread_interrupt_main,
+     METH_VARARGS, interrupt_doc},
     {"get_ident",               thread_get_ident,
      METH_NOARGS, get_ident_doc},
 #ifdef PY_HAVE_THREAD_NATIVE_ID
diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index c6564be9c6aab..98a938f197673 100644
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -8,6 +8,7 @@
 #include "pycore_call.h"
 #include "pycore_ceval.h"
 #include "pycore_pyerrors.h"
+#include "pycore_pylifecycle.h"
 #include "pycore_pystate.h"    // _PyThreadState_GET()
 
 #ifndef MS_WINDOWS
@@ -49,18 +50,6 @@
 #define SIG_ERR ((PyOS_sighandler_t)(-1))
 #endif
 
-#ifndef NSIG
-# if defined(_NSIG)
-#  define NSIG _NSIG            /* For BSD/SysV */
-# elif defined(_SIGMAX)
-#  define NSIG (_SIGMAX + 1)    /* For QNX */
-# elif defined(SIGMAX)
-#  define NSIG (SIGMAX + 1)     /* For djgpp */
-# else
-#  define NSIG 64               /* Use a reasonable default value */
-# endif
-#endif
-
 #include "clinic/signalmodule.c.h"
 
 /*[clinic input]
@@ -106,7 +95,10 @@ class sigset_t_converter(CConverter):
 
 static volatile struct {
     _Py_atomic_int tripped;
-    PyObject *func;
+    /* func is atomic to ensure that PyErr_SetInterrupt is async-signal-safe
+     * (even though it would probably be otherwise, anyway).
+     */
+    _Py_atomic_address func;
 } Handlers[NSIG];
 
 #ifdef MS_WINDOWS
@@ -143,6 +135,16 @@ static HANDLE sigint_event = NULL;
 static PyObject *ItimerError;
 #endif
 
+Py_LOCAL_INLINE(PyObject *)
+get_handler(int i) {
+    return (PyObject *)_Py_atomic_load(&Handlers[i].func);
+}
+
+Py_LOCAL_INLINE(void)
+SetHandler(int i, PyObject* func) {
+    _Py_atomic_store(&Handlers[i].func, (uintptr_t)func);
+}
+
 #ifdef HAVE_GETITIMER
 /* auxiliary functions for setitimer */
 static int
@@ -516,8 +518,8 @@ signal_signal_impl(PyObject *module, int signalnum, PyObject *handler)
         return NULL;
     }
 
-    old_handler = Handlers[signalnum].func;
-    Handlers[signalnum].func = Py_NewRef(handler);
+    old_handler = get_handler(signalnum);
+    SetHandler(signalnum, Py_NewRef(handler));
 
     if (old_handler != NULL) {
         return old_handler;
@@ -553,7 +555,7 @@ signal_getsignal_impl(PyObject *module, int signalnum)
                         "signal number out of range");
         return NULL;
     }
-    old_handler = Handlers[signalnum].func;
+    old_handler = get_handler(signalnum);
     if (old_handler != NULL) {
         return Py_NewRef(old_handler);
     }
@@ -1584,17 +1586,21 @@ signal_module_exec(PyObject *m)
         }
         // If signal_module_exec() is called more than one, we must
         // clear the strong reference to the previous function.
-        Py_XSETREF(Handlers[signum].func, Py_NewRef(func));
+        PyObject* old_func = get_handler(signum);
+        SetHandler(signum, Py_NewRef(func));
+        Py_XDECREF(old_func);
     }
 
     // Instal Python SIGINT handler which raises KeyboardInterrupt
-    if (Handlers[SIGINT].func == DefaultHandler) {
+    PyObject* sigint_func = get_handler(SIGINT);
+    if (sigint_func == DefaultHandler) {
         PyObject *int_handler = PyMapping_GetItemString(d, "default_int_handler");
         if (!int_handler) {
             return -1;
         }
 
-        Py_SETREF(Handlers[SIGINT].func, int_handler);
+        SetHandler(SIGINT, int_handler);
+        Py_DECREF(sigint_func);
         PyOS_setsig(SIGINT, signal_handler);
     }
 
@@ -1630,9 +1636,9 @@ _PySignal_Fini(void)
 {
     // Restore default signals and clear handlers
     for (int signum = 1; signum < NSIG; signum++) {
-        PyObject *func = Handlers[signum].func;
+        PyObject *func = get_handler(signum);
         _Py_atomic_store_relaxed(&Handlers[signum].tripped, 0);
-        Handlers[signum].func = NULL;
+        SetHandler(signum, NULL);
         if (func != NULL
             && func != Py_None
             && func != DefaultHandler
@@ -1712,7 +1718,7 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate)
          * signal handler for it by the time PyErr_CheckSignals() is called
          * (see bpo-43406).
          */
-        PyObject *func = Handlers[i].func;
+        PyObject *func = get_handler(i);
         if (func == NULL || func == Py_None || func == IgnoreHandler ||
             func == DefaultHandler) {
             /* No Python signal handler due to aforementioned race condition.
@@ -1761,18 +1767,27 @@ _PyErr_CheckSignals(void)
 }
 
 
-/* Simulate the effect of a signal.SIGINT signal arriving. The next time
-   PyErr_CheckSignals is called,  the Python SIGINT signal handler will be
-   raised.
+/* Simulate the effect of a signal arriving. The next time PyErr_CheckSignals
+   is called,  the corresponding Python signal handler will be raised.
+
+   Missing signal handler for the given signal number is silently ignored. */
+int
+PyErr_SetInterruptEx(int signum)
+{
+    if (signum < 1 || signum >= NSIG) {
+        return -1;
+    }
+    PyObject* func = get_handler(signum);
+    if (func != IgnoreHandler && func != DefaultHandler) {
+        trip_signal(signum);
+    }
+    return 0;
+}
 
-   Missing signal handler for the SIGINT signal is silently ignored. */
 void
 PyErr_SetInterrupt(void)
 {
-    if ((Handlers[SIGINT].func != IgnoreHandler) &&
-        (Handlers[SIGINT].func != DefaultHandler)) {
-        trip_signal(SIGINT);
-    }
+    (void) PyErr_SetInterruptEx(SIGINT);
 }
 
 static int
diff --git a/PC/python3dll.c b/PC/python3dll.c
index 3f87b70b44848..027d4b1c6e386 100644
--- a/PC/python3dll.c
+++ b/PC/python3dll.c
@@ -209,6 +209,7 @@ EXPORT_FUNC(PyErr_SetFromWindowsErrWithFilename)
 EXPORT_FUNC(PyErr_SetImportError)
 EXPORT_FUNC(PyErr_SetImportErrorSubclass)
 EXPORT_FUNC(PyErr_SetInterrupt)
+EXPORT_FUNC(PyErr_SetInterruptEx)
 EXPORT_FUNC(PyErr_SetNone)
 EXPORT_FUNC(PyErr_SetObject)
 EXPORT_FUNC(PyErr_SetString)



More information about the Python-checkins mailing list