From c0f5de9719ccf13cae89baeccaf4e9f3079923f2 Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Wed, 30 Apr 2025 15:12:42 -0400 Subject: [PATCH 1/3] Allow PyErr_CheckSignals to be called without holding the GIL. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.) --- Doc/c-api/exceptions.rst | 62 ++++++++---- Doc/c-api/init.rst | 5 + ...-05-05-15-20-11.gh-issue-133465.ZksxSw.rst | 7 ++ Modules/signalmodule.c | 95 ++++++++++++++----- 4 files changed, 125 insertions(+), 44 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2025-05-05-15-20-11.gh-issue-133465.ZksxSw.rst diff --git a/Doc/c-api/exceptions.rst b/Doc/c-api/exceptions.rst index c8e1b5c2461738..6ca21d9da2c2e8 100644 --- a/Doc/c-api/exceptions.rst +++ b/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() diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 52f64a61006b74..7540d168613c0c 100644 --- a/Doc/c-api/init.rst +++ b/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 ` 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: diff --git a/Misc/NEWS.d/next/C_API/2025-05-05-15-20-11.gh-issue-133465.ZksxSw.rst b/Misc/NEWS.d/next/C_API/2025-05-05-15-20-11.gh-issue-133465.ZksxSw.rst new file mode 100644 index 00000000000000..9b40f3ab1930db --- /dev/null +++ b/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.) diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 54bcd3270ef31a..a69fed464be0dc 100644 --- a/Modules/signalmodule.c +++ b/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. From 3e25a9350a6b32e297e2785e8fb15ca522df8c0d Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Mon, 5 May 2025 14:34:04 -0400 Subject: [PATCH 2/3] Take advantage of PyErr_CheckSignals now being callable without the GIL. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The source tree contains dozens of loops of this form: int res; do { Py_BEGIN_ALLOW_THREADS res = some_system_call(arguments...); Py_END_ALLOW_THREADS } while (res < 0 && errno == EINTR && !PyErr_CheckSignals()); Now that it is possible to call PyErr_CheckSignals without holding the GIL, the locking operations can be moved out of the loop: Py_BEGIN_ALLOW_THREADS do { res = some_system_call(arguments...); } while (res < 0 && errno == EINTR && !PyErr_CheckSignals()); Py_END_ALLOW_THREADS This demonstrates the motivation for making it possible to call PyErr_CheckSignals without holding the GIL. It shouldn’t make any measurable difference performance-wise for _these_ loops, which almost never actually cycle; but for loops that do cycle many times it’s very much desirable to not take and release the GIL every time through. In some cases I also moved uses of _Py_(BEGIN|END)_SUPPRESS_IPH, which is often paired with Py_(BEGIN|END)_ALLOW_THREADS, to keep the pairing intact. It was already considered safe to call PyErr_CheckSignals from both inside and outside an IPH suppression region. More could be done in this vein: I didn’t change any loops where the inside of the loop was more complicated than a single system call, _except_ that I did refactor py_getentropy and py_getrandom (in bootstrap_hash.c) to make it possible to move the unlock and lock outside the loop, demonstrating a more complicated case. --- Modules/_io/fileio.c | 4 +- Modules/_io/winconsoleio.c | 2 - Modules/_multiprocessing/posixshmem.c | 8 +- Modules/_multiprocessing/semaphore.c | 4 +- Modules/fcntlmodule.c | 32 ++--- Modules/posixmodule.c | 176 ++++++++++++-------------- Modules/readline.c | 9 +- Modules/signalmodule.c | 4 +- Parser/myreadline.c | 12 +- Python/bootstrap_hash.c | 80 ++++++------ Python/fileutils.c | 27 ++-- 11 files changed, 167 insertions(+), 191 deletions(-) diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 8fcb27049d6c7c..6d8c5fb501774b 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -403,16 +403,16 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode, errno = 0; if (opener == Py_None) { + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS #ifdef MS_WINDOWS self->fd = _wopen(widename, flags, 0666); #else self->fd = open(name, flags, 0666); #endif - Py_END_ALLOW_THREADS } while (self->fd < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (async_err) goto error; diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index 3e783b9da45652..26c84c57493175 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -647,9 +647,7 @@ read_console_w(HANDLE handle, DWORD maxlen, DWORD *readlen) { if (WaitForSingleObjectEx(hInterruptEvent, 100, FALSE) == WAIT_OBJECT_0) { ResetEvent(hInterruptEvent); - Py_BLOCK_THREADS sig = PyErr_CheckSignals(); - Py_UNBLOCK_THREADS if (sig < 0) break; } diff --git a/Modules/_multiprocessing/posixshmem.c b/Modules/_multiprocessing/posixshmem.c index ab45e4136c7d46..87e5702326d509 100644 --- a/Modules/_multiprocessing/posixshmem.c +++ b/Modules/_multiprocessing/posixshmem.c @@ -57,11 +57,11 @@ _posixshmem_shm_open_impl(PyObject *module, PyObject *path, int flags, PyErr_SetString(PyExc_ValueError, "embedded null character"); return -1; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS fd = shm_open(name, flags, mode); - Py_END_ALLOW_THREADS } while (fd < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (fd < 0) { if (!async_err) @@ -102,11 +102,11 @@ _posixshmem_shm_unlink_impl(PyObject *module, PyObject *path) PyErr_SetString(PyExc_ValueError, "embedded null character"); return NULL; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS rv = shm_unlink(name); - Py_END_ALLOW_THREADS } while (rv < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (rv < 0) { if (!async_err) diff --git a/Modules/_multiprocessing/semaphore.c b/Modules/_multiprocessing/semaphore.c index a4a2a866ccbfce..d77852614d20e9 100644 --- a/Modules/_multiprocessing/semaphore.c +++ b/Modules/_multiprocessing/semaphore.c @@ -356,19 +356,19 @@ _multiprocessing_SemLock_acquire_impl(SemLockObject *self, int blocking, if (res < 0 && errno == EAGAIN && blocking) { /* Couldn't acquire immediately, need to block */ + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS if (!use_deadline) { res = sem_wait(self->handle); } else { res = sem_timedwait(self->handle, &deadline); } - Py_END_ALLOW_THREADS err = errno; if (res == MP_EXCEPTION_HAS_BEEN_SET) break; } while (res < 0 && errno == EINTR && !PyErr_CheckSignals()); + Py_END_ALLOW_THREADS } if (res < 0) { diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c index 220ee9ecdffc8a..1136d406034a0d 100644 --- a/Modules/fcntlmodule.c +++ b/Modules/fcntlmodule.c @@ -73,11 +73,11 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) } } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = fcntl(fd, code, (int)int_arg); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL; } @@ -103,11 +103,11 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg) memcpy(buf + len, guard, GUARDSZ); PyBuffer_Release(&view); + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = fcntl(fd, code, buf); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL; } @@ -195,11 +195,11 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, } } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = ioctl(fd, code, int_arg); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL; } @@ -219,11 +219,11 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, memcpy(buf + len, guard, GUARDSZ); ptr = buf; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = ioctl(fd, code, ptr); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { if (!async_err) { PyErr_SetFromErrno(PyExc_OSError); @@ -261,11 +261,11 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg, memcpy(buf + len, guard, GUARDSZ); PyBuffer_Release(&view); + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = ioctl(fd, code, buf); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL; } @@ -308,11 +308,11 @@ fcntl_flock_impl(PyObject *module, int fd, int code) } #ifdef HAVE_FLOCK + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = flock(fd, code); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS #else #ifndef LOCK_SH @@ -335,11 +335,11 @@ fcntl_flock_impl(PyObject *module, int fd, int code) return NULL; } l.l_whence = l.l_start = l.l_len = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS } #endif /* HAVE_FLOCK */ if (ret < 0) { @@ -439,11 +439,11 @@ fcntl_lockf_impl(PyObject *module, int fd, int code, PyObject *lenobj, return NULL; } l.l_whence = whence; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l); - Py_END_ALLOW_THREADS } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS } if (ret < 0) { return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 922694fa367ac3..a3584c149a60a6 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1904,13 +1904,13 @@ posix_fildes_fd(int fd, int (*func)(int)) int res; int async_err = 0; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH res = (*func)(fd); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS } while (res != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (res != 0) return (!async_err) ? posix_error() : NULL; Py_RETURN_NONE; @@ -3793,11 +3793,11 @@ os_fchmod_impl(PyObject *module, int fd, int mode) } #else /* MS_WINDOWS */ int async_err = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS res = fchmod(fd, mode); - Py_END_ALLOW_THREADS } while (res != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (res != 0) return (!async_err) ? posix_error() : NULL; #endif /* MS_WINDOWS */ @@ -4147,11 +4147,11 @@ os_fchown_impl(PyObject *module, int fd, uid_t uid, gid_t gid) return NULL; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS res = fchown(fd, uid, gid); - Py_END_ALLOW_THREADS } while (res != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (res != 0) return (!async_err) ? posix_error() : NULL; @@ -9986,11 +9986,11 @@ os_wait3_impl(PyObject *module, int options) WAIT_TYPE status; WAIT_STATUS_INT(status) = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS pid = wait3(&status, options, &ru); - Py_END_ALLOW_THREADS } while (pid < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (pid < 0) return (!async_err) ? posix_error() : NULL; @@ -10023,11 +10023,11 @@ os_wait4_impl(PyObject *module, pid_t pid, int options) WAIT_TYPE status; WAIT_STATUS_INT(status) = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS res = wait4(pid, &status, options, &ru); - Py_END_ALLOW_THREADS } while (res < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (res < 0) return (!async_err) ? posix_error() : NULL; @@ -10065,11 +10065,11 @@ os_waitid_impl(PyObject *module, idtype_t idtype, id_t id, int options) siginfo_t si; si.si_pid = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS res = waitid(idtype, id, &si, options); - Py_END_ALLOW_THREADS } while (res < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (res < 0) return (!async_err) ? posix_error() : NULL; @@ -10130,11 +10130,11 @@ os_waitpid_impl(PyObject *module, pid_t pid, int options) WAIT_TYPE status; WAIT_STATUS_INT(status) = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS res = waitpid(pid, &status, options); - Py_END_ALLOW_THREADS } while (res < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (res < 0) return (!async_err) ? posix_error() : NULL; @@ -10164,13 +10164,13 @@ os_waitpid_impl(PyObject *module, intptr_t pid, int options) intptr_t res; int async_err = 0; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH res = _cwait(&status, pid, options); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS } while (res < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (res < 0) return (!async_err) ? posix_error() : NULL; @@ -10201,11 +10201,11 @@ os_wait_impl(PyObject *module) WAIT_TYPE status; WAIT_STATUS_INT(status) = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS pid = wait(&status); - Py_END_ALLOW_THREADS } while (pid < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (pid < 0) return (!async_err) ? posix_error() : NULL; @@ -11128,9 +11128,9 @@ os_open_impl(PyObject *module, path_t *path, int flags, int mode, int dir_fd) return -1; } + Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS #ifdef MS_WINDOWS fd = _wopen(path->wide, flags, mode); #else @@ -11147,9 +11147,9 @@ os_open_impl(PyObject *module, path_t *path, int flags, int mode, int dir_fd) #endif /* HAVE_OPENAT */ fd = open(path->narrow, flags, mode); #endif /* !MS_WINDOWS */ - Py_END_ALLOW_THREADS } while (fd < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS #ifdef HAVE_OPENAT if (openat_unavailable) { @@ -11597,11 +11597,11 @@ os_readv_impl(PyObject *module, int fd, PyObject *buffers) if (iov_setup(&iov, &buf, buffers, cnt, PyBUF_WRITABLE) < 0) return -1; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS n = readv(fd, iov, cnt); - Py_END_ALLOW_THREADS } while (n < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS int saved_errno = errno; iov_cleanup(iov, buf, cnt); @@ -11649,13 +11649,13 @@ os_pread_impl(PyObject *module, int fd, Py_ssize_t length, Py_off_t offset) if (buffer == NULL) return NULL; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH n = pread(fd, PyBytes_AS_STRING(buffer), length, offset); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS } while (n < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (n < 0) { if (!async_err) { @@ -11728,16 +11728,14 @@ os_preadv_impl(PyObject *module, int fd, PyObject *buffers, Py_off_t offset, if (iov_setup(&iov, &buf, buffers, cnt, PyBUF_WRITABLE) < 0) { return -1; } -#ifdef HAVE_PREADV2 + + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH +#ifdef HAVE_PREADV2 n = preadv2(fd, iov, cnt, offset, flags); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); #else - do { + #if defined(__APPLE__) && defined(__clang__) /* This entire function will be removed from the module dict when the API * is not available. @@ -11746,18 +11744,16 @@ os_preadv_impl(PyObject *module, int fd, PyObject *buffers, Py_off_t offset, #pragma clang diagnostic ignored "-Wunguarded-availability" #pragma clang diagnostic ignored "-Wunguarded-availability-new" #endif - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH n = preadv(fd, iov, cnt, offset); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); #if defined(__APPLE__) && defined(__clang__) #pragma clang diagnostic pop #endif #endif + } while (n < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS int saved_errno = errno; iov_cleanup(iov, buf, cnt); @@ -11921,16 +11917,16 @@ os_sendfile_impl(PyObject *module, int out_fd, int in_fd, PyObject *offobj, } _Py_BEGIN_SUPPRESS_IPH + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS #ifdef __APPLE__ ret = sendfile(in_fd, out_fd, offset, &sbytes, &sf, flags); #else ret = sendfile(in_fd, out_fd, offset, count, &sf, &sbytes, flags); #endif - Py_END_ALLOW_THREADS } while (ret < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS int saved_errno = errno; if (sf.headers != NULL) @@ -11965,11 +11961,11 @@ os_sendfile_impl(PyObject *module, int out_fd, int in_fd, PyObject *offobj, #else #ifdef __linux__ if (offobj == Py_None) { + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = sendfile(out_fd, in_fd, NULL, count); - Py_END_ALLOW_THREADS } while (ret < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) return (!async_err) ? posix_error() : NULL; return PyLong_FromSsize_t(ret); @@ -11984,11 +11980,11 @@ os_sendfile_impl(PyObject *module, int out_fd, int in_fd, PyObject *offobj, // when the offset is equal or bigger than the in_fd size. struct stat st; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = fstat(in_fd, &st); - Py_END_ALLOW_THREADS } while (ret != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) return (!async_err) ? posix_error() : NULL; @@ -12005,8 +12001,8 @@ os_sendfile_impl(PyObject *module, int out_fd, int in_fd, PyObject *offobj, off_t original_offset = offset; #endif + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = sendfile(out_fd, in_fd, &offset, count); #if defined(__sun) && defined(__SVR4) // This handles illumos-specific sendfile() partial write behavior, @@ -12015,8 +12011,9 @@ os_sendfile_impl(PyObject *module, int out_fd, int in_fd, PyObject *offobj, ret = offset - original_offset; } #endif - Py_END_ALLOW_THREADS } while (ret < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS + if (ret < 0) return (!async_err) ? posix_error() : NULL; return PyLong_FromSsize_t(ret); @@ -12072,11 +12069,11 @@ os_fstat_impl(PyObject *module, int fd) int res; int async_err = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS res = FSTAT(fd, &st); - Py_END_ALLOW_THREADS } while (res != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (res != 0) { #ifdef MS_WINDOWS return PyErr_SetFromWindowsErr(0); @@ -12262,11 +12259,11 @@ os_writev_impl(PyObject *module, int fd, PyObject *buffers) return -1; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS result = writev(fd, iov, cnt); - Py_END_ALLOW_THREADS } while (result < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (result < 0 && !async_err) posix_error(); @@ -12300,13 +12297,13 @@ os_pwrite_impl(PyObject *module, int fd, Py_buffer *buffer, Py_off_t offset) Py_ssize_t size; int async_err = 0; + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH size = pwrite(fd, buffer->buf, (size_t)buffer->len, offset); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS } while (size < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (size < 0 && !async_err) posix_error(); @@ -12374,14 +12371,11 @@ os_pwritev_impl(PyObject *module, int fd, PyObject *buffers, Py_off_t offset, if (iov_setup(&iov, &buf, buffers, cnt, PyBUF_SIMPLE) < 0) { return -1; } -#ifdef HAVE_PWRITEV2 + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH +#ifdef HAVE_PWRITEV2 result = pwritev2(fd, iov, cnt, offset, flags); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - } while (result < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); #else #if defined(__APPLE__) && defined(__clang__) @@ -12392,19 +12386,15 @@ os_pwritev_impl(PyObject *module, int fd, PyObject *buffers, Py_off_t offset, #pragma clang diagnostic ignored "-Wunguarded-availability" #pragma clang diagnostic ignored "-Wunguarded-availability-new" #endif - do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH result = pwritev(fd, iov, cnt, offset); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - } while (result < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); - #if defined(__APPLE__) && defined(__clang__) #pragma clang diagnostic pop #endif #endif + } while (result < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (result < 0) { if (!async_err) { @@ -12473,11 +12463,11 @@ os_copy_file_range_impl(PyObject *module, int src, int dst, Py_ssize_t count, p_offset_dst = &offset_dst_val; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = copy_file_range(src, p_offset_src, dst, p_offset_dst, count, flags); - Py_END_ALLOW_THREADS } while (ret < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { return (!async_err) ? posix_error() : NULL; @@ -12542,11 +12532,11 @@ os_splice_impl(PyObject *module, int src, int dst, Py_ssize_t count, p_offset_dst = &offset_dst_val; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS ret = splice(src, p_offset_src, dst, p_offset_dst, count, flags); - Py_END_ALLOW_THREADS } while (ret < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (ret < 0) { return (!async_err) ? posix_error() : NULL; @@ -12583,8 +12573,8 @@ os_mkfifo_impl(PyObject *module, path_t *path, int mode, int dir_fd) int mkfifoat_unavailable = 0; #endif + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS #ifdef HAVE_MKFIFOAT if (dir_fd != DEFAULT_DIR_FD) { if (HAVE_MKFIFOAT_RUNTIME) { @@ -12597,9 +12587,9 @@ os_mkfifo_impl(PyObject *module, path_t *path, int mode, int dir_fd) } else #endif result = mkfifo(path->narrow, mode); - Py_END_ALLOW_THREADS } while (result != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS #ifdef HAVE_MKFIFOAT if (mkfifoat_unavailable) { @@ -12652,8 +12642,8 @@ os_mknod_impl(PyObject *module, path_t *path, int mode, dev_t device, int mknodat_unavailable = 0; #endif + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS #ifdef HAVE_MKNODAT if (dir_fd != DEFAULT_DIR_FD) { if (HAVE_MKNODAT_RUNTIME) { @@ -12666,9 +12656,9 @@ os_mknod_impl(PyObject *module, path_t *path, int mode, dev_t device, } else #endif result = mknod(path->narrow, mode, device); - Py_END_ALLOW_THREADS } while (result != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS #ifdef HAVE_MKNODAT if (mknodat_unavailable) { argument_unavailable_error(NULL, "dir_fd"); @@ -12786,18 +12776,19 @@ os_ftruncate_impl(PyObject *module, int fd, Py_off_t length) return NULL; } + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH #ifdef MS_WINDOWS result = _chsize_s(fd, length); #else result = ftruncate(fd, length); #endif - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS } while (result != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS + if (result != 0) return (!async_err) ? posix_error() : NULL; Py_RETURN_NONE; @@ -12893,11 +12884,11 @@ os_posix_fallocate_impl(PyObject *module, int fd, Py_off_t offset, int result; int async_err = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS result = posix_fallocate(fd, offset, length); - Py_END_ALLOW_THREADS } while (result == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (result == 0) Py_RETURN_NONE; @@ -12940,11 +12931,11 @@ os_posix_fadvise_impl(PyObject *module, int fd, Py_off_t offset, int result; int async_err = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS result = posix_fadvise(fd, offset, length, advice); - Py_END_ALLOW_THREADS } while (result == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (result == 0) Py_RETURN_NONE; @@ -13453,16 +13444,17 @@ os_fstatvfs_impl(PyObject *module, int fd) int result; int async_err = 0; #ifdef __APPLE__ - struct statfs st; /* On macOS os.fstatvfs is implemented using fstatfs(2) because * the former uses 32-bit values for block counts. */ + + struct statfs st; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS result = fstatfs(fd, &st); - Py_END_ALLOW_THREADS } while (result != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (result != 0) return (!async_err) ? posix_error() : NULL; @@ -13470,12 +13462,12 @@ os_fstatvfs_impl(PyObject *module, int fd) #else struct statvfs st; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS result = fstatvfs(fd, &st); - Py_END_ALLOW_THREADS } while (result != 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (result != 0) return (!async_err) ? posix_error() : NULL; diff --git a/Modules/readline.c b/Modules/readline.c index 0dd99dc66c08e9..d27424fee800a3 100644 --- a/Modules/readline.c +++ b/Modules/readline.c @@ -1436,9 +1436,6 @@ rlhandler(char *text) static char * readline_until_enter_or_signal(const char *prompt, int *signal) { - // Defined in Parser/myreadline.c - extern PyThreadState *_PyOS_ReadlineTState; - char * not_done_reading = ""; fd_set selectset; @@ -1483,11 +1480,7 @@ readline_until_enter_or_signal(const char *prompt, int *signal) rl_callback_read_char(); } else if (err == EINTR) { - int s; - PyEval_RestoreThread(_PyOS_ReadlineTState); - s = PyErr_CheckSignals(); - PyEval_SaveThread(); - if (s < 0) { + if (PyErr_CheckSignals() < 0) { rl_free_line_state(); #if defined(RL_READLINE_VERSION) && RL_READLINE_VERSION >= 0x0700 rl_callback_sigcleanup(); diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index a69fed464be0dc..36ef45b9ab08a9 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -1175,12 +1175,12 @@ signal_sigwaitinfo_impl(PyObject *module, sigset_t sigset) int err; int async_err = 0; + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS err = sigwaitinfo(&sigset, &si); - Py_END_ALLOW_THREADS } while (err == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS if (err == -1) return (!async_err) ? PyErr_SetFromErrno(PyExc_OSError) : NULL; diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 64e8f5383f0602..9cf661d2ae57f7 100644 --- a/Parser/myreadline.c +++ b/Parser/myreadline.c @@ -101,11 +101,7 @@ my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp) #ifdef EINTR if (err == EINTR) { - PyEval_RestoreThread(tstate); - int s = PyErr_CheckSignals(); - PyEval_SaveThread(); - - if (s < 0) { + if (PyErr_CheckSignals() < 0) { return 1; } /* try again */ @@ -156,7 +152,6 @@ _PyOS_WindowsConsoleReadline(PyThreadState *tstate, HANDLE hStdIn) break; } if (n_read == 0) { - int s; err = GetLastError(); if (err != ERROR_OPERATION_ABORTED) goto exit; @@ -165,10 +160,7 @@ _PyOS_WindowsConsoleReadline(PyThreadState *tstate, HANDLE hStdIn) if (WaitForSingleObjectEx(hInterruptEvent, 100, FALSE) == WAIT_OBJECT_0) { ResetEvent(hInterruptEvent); - PyEval_RestoreThread(tstate); - s = PyErr_CheckSignals(); - PyEval_SaveThread(); - if (s < 0) { + if (PyErr_CheckSignals() < 0) { goto exit; } } diff --git a/Python/bootstrap_hash.c b/Python/bootstrap_hash.c index f0fb87c4a5d15e..71be8874504215 100644 --- a/Python/bootstrap_hash.c +++ b/Python/bootstrap_hash.c @@ -95,16 +95,19 @@ py_getrandom(void *buffer, Py_ssize_t size, int blocking, int raise) failed with ENOSYS or EPERM. Need Linux kernel 3.17 or newer, or Solaris 11.3 or newer */ static int getrandom_works = 1; - int flags; - char *dest; + int status = 1; + int flags = blocking ? 0 : GRND_NONBLOCK; + char *dest = buffer; + PyThreadState *_save = 0; long n; if (!getrandom_works) { return 0; } - flags = blocking ? 0 : GRND_NONBLOCK; - dest = buffer; + if (raise) { + _save = PyEval_SaveThread(); + } while (0 < size) { #if defined(__sun) && defined(__SVR4) /* Issue #26735: On Solaris, getrandom() is limited to returning up @@ -117,26 +120,12 @@ py_getrandom(void *buffer, Py_ssize_t size, int blocking, int raise) errno = 0; #ifdef HAVE_GETRANDOM - if (raise) { - Py_BEGIN_ALLOW_THREADS - n = getrandom(dest, n, flags); - Py_END_ALLOW_THREADS - } - else { - n = getrandom(dest, n, flags); - } + n = getrandom(dest, n, flags); #else - /* On Linux, use the syscall() function because the GNU libc doesn't - expose the Linux getrandom() syscall yet. See: + /* Older versions of GNU libc did not expose getrandom(), + try using the raw system call instead. See: https://sourceware.org/bugzilla/show_bug.cgi?id=17252 */ - if (raise) { - Py_BEGIN_ALLOW_THREADS - n = syscall(SYS_getrandom, dest, n, flags); - Py_END_ALLOW_THREADS - } - else { - n = syscall(SYS_getrandom, dest, n, flags); - } + n = syscall(SYS_getrandom, dest, n, flags); # ifdef _Py_MEMORY_SANITIZER if (n > 0) { __msan_unpoison(dest, n); @@ -150,7 +139,8 @@ py_getrandom(void *buffer, Py_ssize_t size, int blocking, int raise) or something else. */ if (errno == ENOSYS || errno == EPERM) { getrandom_works = 0; - return 0; + status = 0; + break; } /* getrandom(GRND_NONBLOCK) fails with EAGAIN if the system urandom @@ -159,13 +149,15 @@ py_getrandom(void *buffer, Py_ssize_t size, int blocking, int raise) even if the system urandom is not initialized yet: see the PEP 524. */ if (errno == EAGAIN && !raise && !blocking) { - return 0; + status = 0; + break; } if (errno == EINTR) { if (raise) { if (PyErr_CheckSignals()) { - return -1; + status = -1; + break; } } @@ -176,13 +168,17 @@ py_getrandom(void *buffer, Py_ssize_t size, int blocking, int raise) if (raise) { PyErr_SetFromErrno(PyExc_OSError); } - return -1; + status = -1; + break; } dest += n; size -= n; } - return 1; + if (raise) { + PyEval_RestoreThread(_save); + } + return status; } #elif defined(HAVE_GETENTROPY) @@ -215,25 +211,21 @@ py_getentropy(char *buffer, Py_ssize_t size, int raise) /* Is getentropy() supported by the running kernel? Set to 0 if getentropy() failed with ENOSYS or EPERM. */ static int getentropy_works = 1; + int status = 1; + PyThreadState *_save = 0; if (!getentropy_works) { return 0; } + if (raise) { + _save = PyEval_SaveThread(); + } while (size > 0) { /* getentropy() is limited to returning up to 256 bytes. Call it multiple times if more bytes are requested. */ Py_ssize_t len = Py_MIN(size, 256); - int res; - - if (raise) { - Py_BEGIN_ALLOW_THREADS - res = getentropy(buffer, len); - Py_END_ALLOW_THREADS - } - else { - res = getentropy(buffer, len); - } + int res = getentropy(buffer, len); if (res < 0) { /* ENOSYS: the syscall is not supported by the running kernel. @@ -241,13 +233,15 @@ py_getentropy(char *buffer, Py_ssize_t size, int raise) or something else. */ if (errno == ENOSYS || errno == EPERM) { getentropy_works = 0; - return 0; + status = 0; + break; } if (errno == EINTR) { if (raise) { if (PyErr_CheckSignals()) { - return -1; + status = -1; + break; } } @@ -258,13 +252,17 @@ py_getentropy(char *buffer, Py_ssize_t size, int raise) if (raise) { PyErr_SetFromErrno(PyExc_OSError); } - return -1; + status = -1; + break; } buffer += len; size -= len; } - return 1; + if (raise) { + PyEval_RestoreThread(_save); + } + return status; } #endif /* defined(HAVE_GETENTROPY) && !(defined(__sun) && defined(__SVR4)) */ diff --git a/Python/fileutils.c b/Python/fileutils.c index 78603d40704f14..4caaf95593d514 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1641,12 +1641,13 @@ _Py_open_impl(const char *pathname, int flags, int gil_held) return -1; } + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS fd = open(pathname, flags); - Py_END_ALLOW_THREADS } while (fd < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS + if (async_err) { Py_DECREF(pathname_obj); return -1; @@ -1793,14 +1794,15 @@ Py_fopen(PyObject *path, const char *mode) return NULL; } + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH f = _wfopen(wpath, wmode); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS } while (f == NULL && errno == EINTR && !(async_err = PyErr_CheckSignals())); + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS + saved_errno = errno; PyMem_Free(wpath); #else @@ -1810,12 +1812,13 @@ Py_fopen(PyObject *path, const char *mode) } const char *path_bytes = PyBytes_AS_STRING(bytes); + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS f = fopen(path_bytes, mode); - Py_END_ALLOW_THREADS } while (f == NULL && errno == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS + saved_errno = errno; Py_DECREF(bytes); #endif @@ -1881,9 +1884,9 @@ _Py_read(int fd, void *buf, size_t count) count = _PY_READ_MAX; } + Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH do { - Py_BEGIN_ALLOW_THREADS errno = 0; #ifdef MS_WINDOWS _doserrno = 0; @@ -1901,10 +1904,10 @@ _Py_read(int fd, void *buf, size_t count) /* save/restore errno because PyErr_CheckSignals() * and PyErr_SetFromErrno() can modify it */ err = errno; - Py_END_ALLOW_THREADS } while (n < 0 && err == EINTR && !(async_err = PyErr_CheckSignals())); _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (async_err) { /* read() was interrupted by a signal (failed with EINTR) @@ -1955,8 +1958,8 @@ _Py_write_impl(int fd, const void *buf, size_t count, int gil_held) } if (gil_held) { + Py_BEGIN_ALLOW_THREADS do { - Py_BEGIN_ALLOW_THREADS errno = 0; #ifdef MS_WINDOWS // write() on a non-blocking pipe fails with ENOSPC on Windows if @@ -1977,9 +1980,9 @@ _Py_write_impl(int fd, const void *buf, size_t count, int gil_held) /* save/restore errno because PyErr_CheckSignals() * and PyErr_SetFromErrno() can modify it */ err = errno; - Py_END_ALLOW_THREADS } while (n < 0 && err == EINTR && !(async_err = PyErr_CheckSignals())); + Py_END_ALLOW_THREADS } else { do { From 1c3af668859e1e50e8bb11e3aa58fd2b37d37e54 Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Mon, 19 May 2025 12:26:25 -0400 Subject: [PATCH 3/3] Add a new function instead of changing PyErr_CheckSignals semantics. --- Include/pyerrors.h | 1 + Modules/signalmodule.c | 75 +++++++++++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 27 deletions(-) diff --git a/Include/pyerrors.h b/Include/pyerrors.h index 5d0028c116e2d8..b40a278601d4a6 100644 --- a/Include/pyerrors.h +++ b/Include/pyerrors.h @@ -235,6 +235,7 @@ PyAPI_FUNC(void) PyErr_WriteUnraisable(PyObject *); /* In signalmodule.c */ PyAPI_FUNC(int) PyErr_CheckSignals(void); +PyAPI_FUNC(int) PyErr_CheckSignals_Detached(void); PyAPI_FUNC(void) PyErr_SetInterrupt(void); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 PyAPI_FUNC(int) PyErr_SetInterruptEx(int signum); diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 36ef45b9ab08a9..c20519754aa651 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -1179,7 +1179,7 @@ signal_sigwaitinfo_impl(PyObject *module, sigset_t sigset) do { err = sigwaitinfo(&sigset, &si); } while (err == -1 - && errno == EINTR && !(async_err = PyErr_CheckSignals())); + && errno == EINTR && !(async_err = PyErr_CheckSignals_Detached())); Py_END_ALLOW_THREADS if (err == -1) return (!async_err) ? PyErr_SetFromErrno(PyExc_OSError) : NULL; @@ -1759,11 +1759,12 @@ _PySignal_Fini(void) Py_CLEAR(state->ignore_handler); } -/* 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. */ + +/* Subroutine of the PyErr_CheckSignals family. Does all the work + that needs an attached thread state. When called, 'tstate' must + be the thread state for the current thread, and it must be attached. */ static int -_PyErr_CheckSignalsHoldingGIL(PyThreadState *tstate, bool cycle_collect) +check_signals_attached(PyThreadState *tstate, bool cycle_collect) { #if defined(Py_REMOTE_DEBUG) && defined(Py_SUPPORTS_REMOTE_DEBUG) _PyRunRemoteDebugger(tstate); @@ -1871,11 +1872,48 @@ _PyErr_CheckSignalsHoldingGIL(PyThreadState *tstate, bool cycle_collect) return 0; } +/* Subroutine of the PyErr_CheckSignals family: + Assert that `tstate` is the current thread's state and it is + attached; then proceed to do the work of checking for pending + signals. */ +static int +check_signals_require_attached(PyThreadState *tstate, bool cycle_collect) +{ + _Py_AssertHoldsTstate(); + _Py_CHECK_EMSCRIPTEN_SIGNALS(); + return check_signals_attached(tstate, cycle_collect); +} + +/* Declared in pycore_pyerrors.h. */ +int +_PyErr_CheckSignalsTstate(PyThreadState *tstate) +{ + return check_signals_require_attached(tstate, true); +} + +/* Declared in pycore_pyerrors.h. */ +int +_PyErr_CheckSignals(void) +{ + PyThreadState *tstate = PyGILState_GetThisThreadState(); + return check_signals_require_attached(tstate, false); +} + +/* Declared in pyerrors.h. */ +int +PyErr_CheckSignals(void) +{ + PyThreadState *tstate = PyGILState_GetThisThreadState(); + return check_signals_require_attached(tstate, true); +} + /* 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. */ + If so, attach the thread state if necessary, and do that work. + The 'tstate' argument must be the thread state (if any) for the + current thread, but it does not need to be attached. */ static int -_PyErr_CheckSignalsNoGIL(PyThreadState *tstate, bool cycle_collect) +check_signals_detached(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 @@ -1900,36 +1938,19 @@ _PyErr_CheckSignalsNoGIL(PyThreadState *tstate, bool cycle_collect) /* 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); + int err = check_signals_attached(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 = PyGILState_GetThisThreadState(); - return _PyErr_CheckSignalsNoGIL(tstate, false); -} - /* Declared in pyerrors.h. This function may be called without the GIL. */ int -PyErr_CheckSignals(void) +PyErr_CheckSignals_Detached(void) { PyThreadState *tstate = PyGILState_GetThisThreadState(); - return _PyErr_CheckSignalsNoGIL(tstate, true); + return check_signals_detached(tstate, true); } /* Simulate the effect of a signal arriving. The next time PyErr_CheckSignals