Skip to content

Commit f41b1df

Browse files
anuraagadavidhewitt
anuraaga
and
davidhewitt
authored
Hang when reattaching after detach during shutdown (#6085)
* Hang when reattaching after detach during shutdown * Fix test * Comment * Tweak * Lints * alphabetize * Format * Cleanups * Combine test * Format * Format * Drift * Simplify test * Fix stub * fix test name * safety comment * skip detach test on graalpy --------- Co-authored-by: David Hewitt <mail@davidhewitt.dev>
1 parent 5ae66a8 commit f41b1df

7 files changed

Lines changed: 85 additions & 18 deletions

File tree

‎newsfragments/6085.changed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* PyO3 threads now hang instead of `pthread_exit` trying to acquire the GIL when the interpreter is shutting down after detaching. This mimics the [Python 3.14](https://github.com/python/cpython/issues/87135) behavior and avoids undefined behavior and crashes and applies the same logic as done for [Python::attach](https://github.com/PyO3/pyo3/pull/4874).

‎pyo3-ffi/src/ceval.rs‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ extern_libpython! {
107107

108108
#[cfg_attr(PyPy, link_name = "PyPyEval_SaveThread")]
109109
pub fn PyEval_SaveThread() -> *mut PyThreadState;
110-
#[cfg_attr(PyPy, link_name = "PyPyEval_RestoreThread")]
111-
pub fn PyEval_RestoreThread(arg1: *mut PyThreadState);
112110

113111
#[cfg(not(Py_3_13))]
114112
#[cfg_attr(PyPy, link_name = "PyPyEval_ThreadsInitialized")]
@@ -139,6 +137,35 @@ extern_libpython! {
139137
pub fn PyEval_ReleaseThread(tstate: *mut PyThreadState);
140138
}
141139

140+
// PyEval_RestoreThread calls take_gil, which calls pthread_exit on non-main threads
141+
// during interpreter finalization on Python < 3.14. Redirect to the "safe" version that hangs instead,
142+
// as Python 3.14 does.
143+
// See https://github.com/rust-lang/rust/issues/135929
144+
// C-unwind only supported (and necessary) since 1.71. Python 3.14+ does not do
145+
// pthread_exit from PyEval_RestoreThread (https://github.com/python/cpython/issues/87135).
146+
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
147+
mod raw {
148+
use crate::pytypedefs::PyThreadState;
149+
extern_libpython! { "C-unwind" {
150+
#[cfg_attr(PyPy, link_name = "PyPyEval_RestoreThread")]
151+
pub fn PyEval_RestoreThread(tstate: *mut PyThreadState);
152+
}}
153+
}
154+
155+
#[cfg(any(Py_3_14, target_arch = "wasm32"))]
156+
extern_libpython! {
157+
#[cfg_attr(PyPy, link_name = "PyPyEval_RestoreThread")]
158+
pub fn PyEval_RestoreThread(tstate: *mut PyThreadState);
159+
}
160+
161+
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
162+
pub unsafe extern "C" fn PyEval_RestoreThread(tstate: *mut PyThreadState) {
163+
// Same note as in PyGILState_Ensure
164+
let guard = crate::impl_::HangThread;
165+
raw::PyEval_RestoreThread(tstate);
166+
core::mem::forget(guard);
167+
}
168+
142169
// skipped Py_BEGIN_ALLOW_THREADS
143170
// skipped Py_BLOCK_THREADS
144171
// skipped Py_UNBLOCK_THREADS

‎pyo3-ffi/src/impl_/mod.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,16 @@ mod atomic_c_ulong {
2020
#[cfg(all(Py_GIL_DISABLED, not(Py_LIMITED_API)))]
2121
#[doc(hidden)]
2222
pub type AtomicCULong = atomic_c_ulong::TYPE;
23+
24+
/// Guard to hang the current thread indefinitely when dropped.
25+
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
26+
pub struct HangThread;
27+
28+
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
29+
impl Drop for HangThread {
30+
fn drop(&mut self) {
31+
loop {
32+
std::thread::park(); // Block forever.
33+
}
34+
}
35+
}

‎pyo3-ffi/src/pystate.rs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -79,18 +79,6 @@ pub enum PyGILState_STATE {
7979
PyGILState_UNLOCKED,
8080
}
8181

82-
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
83-
struct HangThread;
84-
85-
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
86-
impl Drop for HangThread {
87-
fn drop(&mut self) {
88-
loop {
89-
std::thread::park(); // Block forever.
90-
}
91-
}
92-
}
93-
9482
// The PyGILState_Ensure function will call pthread_exit during interpreter shutdown,
9583
// which causes undefined behavior. Redirect to the "safe" version that hangs instead,
9684
// as Python 3.14 does.
@@ -115,7 +103,7 @@ mod raw {
115103

116104
#[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
117105
pub unsafe extern "C" fn PyGILState_Ensure() -> PyGILState_STATE {
118-
let guard = HangThread;
106+
let guard = crate::impl_::HangThread;
119107
// If `PyGILState_Ensure` calls `pthread_exit`, which it does on Python < 3.14
120108
// when the interpreter is shutting down, this will cause a forced unwind.
121109
// doing a forced unwind through a function with a Rust destructor is unspecified

‎pytests/src/misc.rs‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,35 @@ fn hammer_attaching_in_thread() -> LockHolder {
3131
LockHolder { sender }
3232
}
3333

34+
/// Wrapper to mark Receiver as Sync.
35+
struct SyncReceiver<T>(std::sync::mpsc::Receiver<T>);
36+
37+
impl<T> std::ops::Deref for SyncReceiver<T> {
38+
type Target = std::sync::mpsc::Receiver<T>;
39+
40+
fn deref(&self) -> &Self::Target {
41+
&self.0
42+
}
43+
}
44+
45+
// SAFETY: only used to allow the receiver to be used after detaching
46+
unsafe impl<T> Sync for SyncReceiver<T> {}
47+
48+
#[pyfunction]
49+
fn detach_during_finalization() -> LockHolder {
50+
let (sender, receiver) = std::sync::mpsc::channel();
51+
let receiver = SyncReceiver(receiver);
52+
std::thread::spawn(move || {
53+
Python::attach(|py| {
54+
py.detach(|| {
55+
receiver.recv().ok();
56+
// Interpreter is finalizing while we try to reattach after returning
57+
});
58+
});
59+
});
60+
LockHolder { sender }
61+
}
62+
3463
#[pyfunction]
3564
fn get_type_fully_qualified_name<'py>(obj: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyString>> {
3665
obj.get_type().fully_qualified_name()
@@ -58,7 +87,7 @@ fn get_item_and_run_callback(dict: Bound<'_, PyDict>, callback: Bound<'_, PyAny>
5887
pub mod misc {
5988
#[pymodule_export]
6089
use super::{
61-
accepts_bool, get_item_and_run_callback, get_type_fully_qualified_name,
62-
hammer_attaching_in_thread, issue_219,
90+
accepts_bool, detach_during_finalization, get_item_and_run_callback,
91+
get_type_fully_qualified_name, hammer_attaching_in_thread, issue_219,
6392
};
6493
}

‎pytests/stubs/misc.pyi‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from typing import Any
22

33
def accepts_bool(val: bool) -> bool: ...
4+
def detach_during_finalization() -> Any: ...
45
def get_item_and_run_callback(dict: dict, callback: Any) -> None: ...
56
def get_type_fully_qualified_name(obj: Any) -> str: ...
67
def hammer_attaching_in_thread() -> Any: ...

pytests/tests/test_hammer_attaching_in_thread.py renamed to pytests/tests/test_finalization.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import sysconfig
2+
from sys import implementation
23

34
import pytest
4-
55
from pyo3_pytests import misc
66

77

@@ -26,3 +26,11 @@ def make_loop():
2626
)
2727
def test_hammer_attaching_in_thread():
2828
loopy.append(misc.hammer_attaching_in_thread())
29+
30+
31+
@pytest.mark.skipif(
32+
implementation.name == "graalpy",
33+
reason="graalpy aborts instead of unwinding the thread",
34+
)
35+
def test_detach_during_finalization():
36+
loopy.append(misc.detach_during_finalization())

0 commit comments

Comments
 (0)