Allow PyErr_CheckSignals to be called without holding the GIL.

Compiled-code modules that implement time-consuming operations that
don’t require manipulating Python objects, are supposed to call
PyErr_CheckSignals frequently throughout each such operation, so that
if the user interrupts the operation with control-C, it is cancelled
promptly.  In the normal case where no signals are pending,
PyErr_CheckSignals is cheap; however, callers must hold the GIL,
and compiled-code modules that implement time-consuming operations
are also supposed to release the GIL during each such operation.
The overhead of reclaiming the GIL in order to call PyErr_CheckSignals,
and then releasing it again, sufficiently often for reasonable user
responsiveness, can be substantial.

If my understanding of the thread-state rules is correct,
PyErr_CheckSignals only *needs* the GIL if it has work to do.
*Checking* whether there is a pending signal, or a pending request
to run the cycle collector, requires only a couple of atomic loads.
Therefore:  Reorganize the logic of PyErr_CheckSignals and its close
relatives (_PyErr_CheckSignals and _PyErr_CheckSignalsTstate) so that
all the “do we have anything to do” checks are done in a batch before
anything that needs the GIL.  If any of them are true, acquire the GIL,
repeat the check (because another thread could have stolen the event
while we were waiting for the GIL), and then actually do the work,
enabling callers to *not* hold the GIL.

(There are some fine details here that I’d really appreciate a second
pair of eyes on — see the comments in the new functions
_PyErr_CheckSignalsHoldingGIL and _PyErr_CheckSignalsNoGIL.)

Author: zackw

Doc/c-api/exceptions.rst

@@ -631,6 +631,8 @@ Querying the error indicator
 Signal Handling
 ===============
 
+See the :mod:`signal` module for an overview of signals and signal
+handling.
 
 .. c:function:: int PyErr_CheckSignals()
 
@@ -639,29 +641,51 @@ Signal Handling
       single: SIGINT (C macro)
       single: KeyboardInterrupt (built-in exception)
 
-   This function interacts with Python's signal handling.
+   This function is to be called by long-running C code that wants to
+   be interruptible by user requests, such as by pressing Ctrl-C.
+
+   When it is called from the main thread and under the main Python
+   interpreter, it checks whether any signals have been delivered to
+   the process, and if so, invokes the corresponding Python-level
+   signal handler (if there is one) for each signal.
+
+   :c:func:`PyErr_CheckSignals()` attempts to call signal handlers
+   for each signal that has been delivered since the last time it
+   was called.  If all signal handlers complete successfully, it
+   returns ``0``.  However, if a signal handler raises an exception,
+   that exception is stored in the error indicator for the main thread,
+   and :c:func:`PyErr_CheckSignals()` immediately returns ``-1``.
+   (When this happens, some of the pending signals may not have had
+   their signal handlers called; they will be called the next time
+   :c:func:`PyErr_CheckSignals()` is called.)
+
+   Callers of :c:func:`PyErr_CheckSignals()` should treat a ``-1``
+   return value the same as any other failure of a C-API function;
+   they should immediately cease work, clean up (deallocating
+   resources, etc.) and propagate the failure status to their
+   callers.
+
+   When this function is called from other than the main thread, or
+   other than the main Python interpreter, it does not invoke any
+   signal handlers, and it always returns ``0``.
+
+   Regardless of context, calling this function may have the side
+   effect of running the cyclic garbage collector.
 
-   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 attempts 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).
+   .. warning::
+      This function may execute arbitrary Python code before returning
+      to its caller.
 
    .. note::
-      The default Python signal handler for :c:macro:`!SIGINT` raises the
-      :exc:`KeyboardInterrupt` exception.
+      This function can be called without an :term:`attached thread state`
+      (see :ref:`threads`).  However, this function may internally
+      attach (and then release) a thread state (only if it has any
+      work to do); it must be safe to do that at each point where
+      this function is called.
 
+   .. note::
+      The default Python signal handler for :c:macro:`!SIGINT` raises
+      the :exc:`KeyboardInterrupt` exception.
 
 .. c:function:: void PyErr_SetInterrupt()
 

Doc/c-api/init.rst

@@ -1045,6 +1045,11 @@ to be released to attach their thread state, allowing true multi-core parallelis
    For example, the standard :mod:`zlib` and :mod:`hashlib` modules detach the
    :term:`thread state <attached thread state>` when compressing or hashing data.
 
+.. note::
+   Any code that executes for a long time without returning to the
+   Python interpreter should call :c:func:`PyErr_CheckSignals()`
+   at reasonable intervals (at least once a millisecond) so that
+   it can be interrupted by the user.
 
 .. _gilstate:
 

Misc/NEWS.d/next/C_API/2025-05-05-15-20-11.gh-issue-133465.ZksxSw.rst

@@ -0,0 +1,7 @@
+:c:func:`PyErr_CheckSignals` has been changed to acquire the global
+interpreter lock (GIL) itself, only when necessary (i.e. when it has work to
+do).  This means that modules that perform lengthy computations with the GIL
+released may now call :c:func:`PyErr_CheckSignals` during those computations
+without re-acquiring the GIL first.  (However, it must be *safe to* acquire
+the GIL at each point where :c:func:`PyErr_CheckSignals` is called.  Also,
+keep in mind that it can run arbitrary Python code before returning to you.)

Modules/signalmodule.c

@@ -1759,12 +1759,22 @@ _PySignal_Fini(void)
     Py_CLEAR(state->ignore_handler);
 }
 
-
-/* Declared in pyerrors.h */
-int
-PyErr_CheckSignals(void)
+/* Subroutine of _PyErr_CheckSignalsNoGIL.  Does all the work that
+   needs the GIL.  When called, 'tstate' must be the thread state for
+   the current thread, and the current thread must hold the GIL.  */
+static int
+_PyErr_CheckSignalsHoldingGIL(PyThreadState *tstate, bool cycle_collect)
 {
-    PyThreadState *tstate = _PyThreadState_GET();
+#if defined(Py_REMOTE_DEBUG) && defined(Py_SUPPORTS_REMOTE_DEBUG)
+    _PyRunRemoteDebugger(tstate);
+#endif
+
+    /* It is necessary to repeat all of the checks of global flags
+       that were done in _PyErr_CheckSignalsNoGIL.  At the time of
+       those checks, we might not have held the GIL; in between those
+       checks and when we acquired the GIL, some other thread may have
+       processed the events that were flagged.  Since we now hold the
+       GIL, a check now will be valid until we release it again.  */
 
     /* Opportunistically check if the GC is scheduled to run and run it
        if we have a request. This is done here because native code needs
@@ -1777,24 +1787,8 @@ PyErr_CheckSignals(void)
         _Py_RunGC(tstate);
     }
 
-#if defined(Py_REMOTE_DEBUG) && defined(Py_SUPPORTS_REMOTE_DEBUG)
-    _PyRunRemoteDebugger(tstate);
-#endif
-
-    if (!_Py_ThreadCanHandleSignals(tstate->interp)) {
-        return 0;
-    }
-
-    return _PyErr_CheckSignalsTstate(tstate);
-}
-
-
-/* Declared in cpython/pyerrors.h */
-int
-_PyErr_CheckSignalsTstate(PyThreadState *tstate)
-{
-    _Py_CHECK_EMSCRIPTEN_SIGNALS();
-    if (!_Py_atomic_load_int(&is_tripped)) {
+    if (!_Py_ThreadCanHandleSignals(tstate->interp)
+        || !_Py_atomic_load_int(&is_tripped)) {
         return 0;
     }
 
@@ -1877,15 +1871,66 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate)
     return 0;
 }
 
+/* Subroutine of the PyErr_CheckSignals family:
+   Determine whether there is actually any work needing to be done.
+   If so, acquire the GIL if necessary, and do that work.  */
+static int
+_PyErr_CheckSignalsNoGIL(PyThreadState *tstate, bool cycle_collect)
+{
+    /* If this thread does not have a thread state at all, then it has
+       never been associated with the Python runtime, so it should not
+       attempt to handle signals or run the cycle collector.  */
+    if (!tstate) {
+        return 0;
+    }
+
+    _Py_CHECK_EMSCRIPTEN_SIGNALS();
+
+    /* Don't acquire the GIL if we don't have anything to do.
+       VERIFYME: I *think* every piece of this expression is safe to
+       execute without holding the GIL and is already sufficiently
+       atomic.  */
+    if ((!_Py_ThreadCanHandleSignals(tstate->interp)
+         || !_Py_atomic_load_int(&is_tripped))
+        && (!cycle_collect
+            || !_Py_eval_breaker_bit_is_set(tstate, _PY_GC_SCHEDULED_BIT))) {
+        return 0;
+    }
 
+    /* FIXME: Given that we already have 'tstate', is there a more efficient
+       way to do this? */
+    PyGILState_STATE st = PyGILState_Ensure();
+    int err = _PyErr_CheckSignalsHoldingGIL(tstate, cycle_collect);
+    PyGILState_Release(st);
 
+    return err;
+}
+
+/* Declared in pycore_pyerrors.h.
+   This function may be called without the GIL. */
+int
+_PyErr_CheckSignalsTstate(PyThreadState *tstate)
+{
+    return _PyErr_CheckSignalsNoGIL(tstate, true);
+}
+
+/* Declared in pycore_pyerrors.h.
+   This function may be called without the GIL.  */
 int
 _PyErr_CheckSignals(void)
 {
-    PyThreadState *tstate = _PyThreadState_GET();
-    return _PyErr_CheckSignalsTstate(tstate);
+    PyThreadState *tstate = PyGILState_GetThisThreadState();
+    return _PyErr_CheckSignalsNoGIL(tstate, false);
 }
 
+/* Declared in pyerrors.h.
+   This function may be called without the GIL. */
+int
+PyErr_CheckSignals(void)
+{
+    PyThreadState *tstate = PyGILState_GetThisThreadState();
+    return _PyErr_CheckSignalsNoGIL(tstate, true);
+}
 
 /* Simulate the effect of a signal arriving. The next time PyErr_CheckSignals
    is called,  the corresponding Python signal handler will be raised.