From e2de8dfb4c15ce671275565eb3bb1e2a42662667 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 27 Apr 2025 17:31:05 -0400 Subject: [PATCH 1/9] PEP 788: Add clarification to some parts --- peps/pep-0788.rst | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 3749258244d..5c8f05c87e9 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -348,14 +348,11 @@ way). This generally happens when a thread calls :c:func:`PyEval_RestoreThread` or in between bytecode instructions, based on :func:`sys.setswitchinterval`. A new, internal field will be added to the ``PyThreadState`` structure that -determines if the thread is daemon. If the thread is daemon, then it will -hang during attachment as usual, but if it's not, then the interpreter will -let the thread attach and continue execution. On with-GIL builds, this again -means handing off the GIL to the thread. During finalization, the interpreter +determines if the thread is daemon. Before finalization, the interpreter will wait until all non-daemon threads call :c:func:`PyThreadState_Delete`. -For backwards compatibility, all thread states created by existing APIs will -remain daemon by default. +For backwards compatibility, all thread states created by existing APIs, +including :c:func:`PyGILState_Ensure`, will remain daemon by default. .. c:function:: int PyThreadState_SetDaemon(int is_daemon) @@ -396,8 +393,8 @@ A non-zero reference count prevents the interpreter from finalizing. This function is generally meant to be used in tandem with :c:func:`PyThreadState_Ensure`. - The caller must have an :term:`attached thread state`, and cannot return - ``NULL``. Failures are always a fatal error. + The caller must have an :term:`attached thread state`. This function + cannot return ``NULL``. Failures are always a fatal error. .. c:function:: PyInterpreterState *PyInterpreterState_Lookup(int64_t interp_id) @@ -438,7 +435,7 @@ replace :c:func:`PyGILState_Ensure` and :c:func:`PyGILState_Release`. there is a subsequent call to :c:func:`PyThreadState_Release` that matches this one. - The interpreter's *interp* reference count is decremented by one. + The reference to the interpreter *interp* is stolen by this function. As such, *interp* should have been acquired by :c:func:`PyInterpreterState_Hold`. @@ -454,8 +451,9 @@ replace :c:func:`PyGILState_Ensure` and :c:func:`PyGILState_Release`. .. c:function:: void PyThreadState_Release() - Detach and destroy the :term:`attached thread state` set by - :c:func:`PyThreadState_Ensure`. + Release the :term:`attached thread state` set by + :c:func:`PyThreadState_Ensure`. Any thread state that was set prior + to the original call to :c:func:`PyThreadState_Ensure` will be restored. This function cannot fail, but may hang the thread if the attached thread state prior to the original :c:func:`!PyThreadState_Ensure` @@ -728,7 +726,7 @@ deleted and cause use-after-free violations. :c:func:`PyInterpreterState_Hold` fixes this issue anyway, but an interpreter ID does have the benefit of requiring less magic in the implementation, but has several downsides: -- Nearly all existing APIs already return a :c:type:`PyInterpreterState` +- Nearly all existing interpreter APIs already return a :c:type:`PyInterpreterState` pointer, not an interpreter ID. Functions like :c:func:`PyThreadState_GetInterpreter` would have to be accompanied by frustrating calls to :c:func:`PyInterpreterState_GetID`. There's also From 5cf41d7b51e6f4fc48d695f3dfe6e16f255a945c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 27 Apr 2025 17:43:51 -0400 Subject: [PATCH 2/9] Link to the motivation. --- peps/pep-0788.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 5c8f05c87e9..e35a087ebaa 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -168,6 +168,8 @@ finalization, because a daemon thread got hung while holding the lock. There are workarounds for this for pure-Python code, but native threads don't have such an option. +.. _pep-788-hanging-backwards-compat + We can't change finalization behavior for ``PyGILState_Ensure`` *************************************************************** @@ -353,6 +355,7 @@ will wait until all non-daemon threads call :c:func:`PyThreadState_Delete`. For backwards compatibility, all thread states created by existing APIs, including :c:func:`PyGILState_Ensure`, will remain daemon by default. +See :ref:`pep-788-hanging-compat`. .. c:function:: int PyThreadState_SetDaemon(int is_daemon) From 4efb1ee27ed7810aa237cbed8bf8ca8114a983bd Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 27 Apr 2025 17:46:20 -0400 Subject: [PATCH 3/9] Fix hyperlink. --- peps/pep-0788.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index e35a087ebaa..d409702987e 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -168,7 +168,7 @@ finalization, because a daemon thread got hung while holding the lock. There are workarounds for this for pure-Python code, but native threads don't have such an option. -.. _pep-788-hanging-backwards-compat +.. _pep-788-hanging-compat: We can't change finalization behavior for ``PyGILState_Ensure`` *************************************************************** From c1a34c12faa024da21908aae4abc911d7cee31d9 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 27 Apr 2025 17:51:45 -0400 Subject: [PATCH 4/9] Add some notes about backwards compatibility in the motivation. --- peps/pep-0788.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index d409702987e..369afc7c1c5 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -188,6 +188,11 @@ error, as noted in `python/cpython#124622 Date: Sun, 27 Apr 2025 17:58:30 -0400 Subject: [PATCH 5/9] Small wording changes. --- peps/pep-0788.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 369afc7c1c5..1d98685daf0 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -189,9 +189,9 @@ error, as noted in `python/cpython#124622 Date: Sun, 27 Apr 2025 18:13:52 -0400 Subject: [PATCH 6/9] Use an indefinite article for 'interpreter' --- peps/pep-0788.rst | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 1d98685daf0..98e8f8c6566 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -23,7 +23,7 @@ become problematic: - They aren't safe for finalization, either causing the calling thread to hang or crashing it with a segmentation fault, preventing further execution. - When they're called before finalization, they force the thread to be - "daemon", meaning that the interpreter won't wait for it to reach any point + "daemon", meaning that an interpreter won't wait for it to reach any point of execution. This is mostly frustrating for developers, but can lead to deadlocks! - Subinterpreters don't play nicely with them, because they all assume that @@ -54,12 +54,12 @@ This is achieved by introducing two concepts into the C API: - "Daemon" and "non-daemon" threads, similar to how it works in the :mod:`threading` module. -- Interpreter reference counts which prevent the interpreter from finalizing. +- Interpreter reference counts which prevent an interpreter from finalizing. In :c:func:`PyThreadState_Ensure`, both of these ideas are applied. The -calling thread is to store a reference to the interpreter via +calling thread is to store a reference to an interpreter via :c:func:`PyInterpreterState_Hold`. :c:func:`PyInterpreterState_Hold` -increases the reference count of the interpreter, requiring the thread +increases the reference count of an interpreter, requiring the thread to finish (by eventually calling :c:func:`PyThreadState_Release`) before beginning finalization. @@ -355,7 +355,7 @@ way). This generally happens when a thread calls :c:func:`PyEval_RestoreThread` or in between bytecode instructions, based on :func:`sys.setswitchinterval`. A new, internal field will be added to the ``PyThreadState`` structure that -determines if the thread is daemon. Before finalization, the interpreter +determines if the thread is daemon. Before finalization, an interpreter will wait until all non-daemon threads call :c:func:`PyThreadState_Delete`. For backwards compatibility, all thread states created by existing APIs, @@ -379,8 +379,8 @@ See :ref:`pep-788-hanging-compat`. Interpreter reference counting ------------------------------ -Internally, the interpreter will have to keep track of the number of -non-daemon native threads, which will determine when the interpreter can +Internally, an interpreter will have to keep track of the number of +non-daemon native threads, which will determine when an interpreter can finalize. This is done to prevent use-after-free crashes in :c:func:`PyThreadState_Ensure` for interpreters with short lifetimes, and to remove needless layers of synchronization between the calling thread and From 81a71a136385cb4c46d65b0406d800270ef79a67 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 28 Apr 2025 11:12:08 -0400 Subject: [PATCH 7/9] Update pep-0788.rst Co-authored-by: Victor Stinner --- peps/pep-0788.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 98e8f8c6566..33d2b525c47 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -189,7 +189,7 @@ error, as noted in `python/cpython#124622 Date: Mon, 28 Apr 2025 16:24:07 -0400 Subject: [PATCH 8/9] Remove the term 'signal handler.' --- peps/pep-0788.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 33d2b525c47..7e80fe72a97 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -337,7 +337,7 @@ that it always works. An approach that were to return a failure based on the start-time of the thread could cause spurious issues. In the case where it is useful to let the interpreter finalize, such as in -a signal handler where there's no guarantee that the thread will start, +an asynchronous callback where there's no guarantee that the thread will start, strong references to an interpreter can be acquired through :c:func:`PyInterpreterState_Lookup`. @@ -409,7 +409,7 @@ A non-zero reference count prevents the interpreter from finalizing. Similar to :c:func:`PyInterpreterState_Hold`, but looks up an interpreter based on an ID (see :c:func:`PyInterpreterState_GetID`). This has the benefit of allowing the interpreter to finalize in cases where the thread - might not start, such as inside of a signal handler. + might not start, such as inside of an asynchronous callback. This function will return ``NULL`` without an exception set on failure. If the return value is non-``NULL``, then the returned interpreter will be @@ -661,10 +661,10 @@ Asynchronous callback example ***************************** As stated in the Motivation_, there are many cases where it's desirable -to call Python in an asynchronous callback, such as a signal handler. In that -case, it's not safe to call :c:func:`PyInterpreterState_Hold`, because it's -not guaranteed that :c:func:`PyThreadState_Ensure` will ever be called, which -would deadlock finalization. +to call Python in an asynchronous callback. In that case, it's not safe to +call :c:func:`PyInterpreterState_Hold`, because it's not guaranteed that +:c:func:`PyThreadState_Ensure` will ever be called, which would deadlock +finalization. This scenario requires :c:func:`PyInterpreterState_Lookup`, which only prevents finalization when the lookup has been made. From d553a891bf619fbe355576d82aa0c6be55b9248b Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 30 Apr 2025 06:02:53 -0400 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: C.A.M. Gerlach --- peps/pep-0788.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/peps/pep-0788.rst b/peps/pep-0788.rst index 7e80fe72a97..0f5b58e1804 100644 --- a/peps/pep-0788.rst +++ b/peps/pep-0788.rst @@ -661,13 +661,13 @@ Asynchronous callback example ***************************** As stated in the Motivation_, there are many cases where it's desirable -to call Python in an asynchronous callback. In that case, it's not safe to +to call Python in an asynchronous callback. In such cases, it's not safe to call :c:func:`PyInterpreterState_Hold`, because it's not guaranteed that -:c:func:`PyThreadState_Ensure` will ever be called, which would deadlock -finalization. +:c:func:`PyThreadState_Ensure` will ever be called. +If not, finalization becomes deadlocked. -This scenario requires :c:func:`PyInterpreterState_Lookup`, which only prevents -finalization when the lookup has been made. +This scenario requires using :c:func:`PyInterpreterState_Lookup` instead, +which only prevents finalization once the lookup has been made. For example: