gh-133465: Allow PyErr_CheckSignals to be called without holding the GIL. by zackw · Pull Request #133466 · python/cpython · GitHub
Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 43 additions & 19 deletions Doc/c-api/exceptions.rst
5 changes: 5 additions & 0 deletions Doc/c-api/init.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is too frequent for my taste. Use interactions feel "delay-less" under 10 msec, and for ^C 100 even msec feels very quick, and 1sec would still be acceptable. After a few seconds I would hit ^C again.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is briefly discussed in the talk. If you add enough PyErr_CheckSignals calls to your extension module that every long-running loop can be interrupted, in practice this makes you do the check way too often, once every few tens of micro seconds. This is fine with the hypothetical new PyErr_CheckSignals_Detached, but with the old version, where you have to reclaim the GIL, it's way too costly. So in the talk I recommended looking at the actual system clock (with clock_gettime) and only calling PyErr_CheckSignals if a millisecond or more had gone by. That's faster than required for human responsiveness, yes, but the remaining overhead is the overhead of looking at the clock and you can't reduce that by doing the actual calls even less often. So that's where "at least once a millisecond" came from.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explain that better in the docs then.

it can be interrupted by the user.

.. _gilstate:

Expand Down
1 change: 1 addition & 0 deletions Include/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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.)
Comment on lines +1 to +7

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A NEWS entry should be more concise, users can refer to docs for in depth explanations.

@zackw zackw May 5, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this better?

:c:func:`PyErr_CheckSignals` has been made safe to call without holding the GIL.
It will acquire the GIL itself when it needs it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A NEWS entry should be more concise, users can refer to docs for in depth explanations.

@StanFromIreland, I disagree. AFAIK you aren't a core dev or triager, I wish you wouldn't give other contributors questionable advice without a reference in such an authoritive-sounding way.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would update this and focus on how the new API differs from the old one: IIUC, "can be called without the GIL [or whatever PC phrasing] and doesn't acquire it unless a signal's handler needs to be called."

4 changes: 2 additions & 2 deletions Modules/_io/fileio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions Modules/_io/winconsoleio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions Modules/_multiprocessing/posixshmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions Modules/_multiprocessing/semaphore.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
32 changes: 16 additions & 16 deletions Modules/fcntlmodule.c
Loading