Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hang instead of pthread_exit during interpreter shutdown #4874

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions newsfragments/4874.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* PyO3 threads now hang instead of `pthread_exit` trying to acquire the GIL when the interpreter is shutting down. This mimics the [Python 3.14](https://github.com/python/cpython/issues/87135) behavior and avoids undefined behavior and crashes.
8 changes: 7 additions & 1 deletion pyo3-build-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ pub fn print_feature_cfgs() {
println!("cargo:rustc-cfg=rustc_has_once_lock");
}

if rustc_minor_version >= 71 {
println!("cargo:rustc-cfg=rustc_has_extern_c_unwind");
}

// invalid_from_utf8 lint was added in Rust 1.74
if rustc_minor_version >= 74 {
println!("cargo:rustc-cfg=invalid_from_utf8_lint");
Expand Down Expand Up @@ -226,12 +230,14 @@ pub fn print_expected_cfgs() {
println!("cargo:rustc-check-cfg=cfg(diagnostic_namespace)");
println!("cargo:rustc-check-cfg=cfg(c_str_lit)");
println!("cargo:rustc-check-cfg=cfg(rustc_has_once_lock)");
println!("cargo:rustc-check-cfg=cfg(rustc_has_extern_c_unwind)");
println!("cargo:rustc-check-cfg=cfg(io_error_more)");
println!("cargo:rustc-check-cfg=cfg(fn_ptr_eq)");

// allow `Py_3_*` cfgs from the minimum supported version up to the
// maximum minor version (+1 for development for the next)
for i in impl_::MINIMUM_SUPPORTED_VERSION.minor..=impl_::ABI3_MAX_MINOR + 1 {
// FIXME: support cfg(Py_3_14) as well due to PyGILState_Ensure
for i in impl_::MINIMUM_SUPPORTED_VERSION.minor..=std::cmp::max(14, impl_::ABI3_MAX_MINOR + 1) {
println!("cargo:rustc-check-cfg=cfg(Py_3_{i})");
}
}
Expand Down
68 changes: 66 additions & 2 deletions pyo3-ffi/src/pystate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,73 @@ pub enum PyGILState_STATE {
PyGILState_UNLOCKED,
}

struct HangThread;

impl Drop for HangThread {
fn drop(&mut self) {
loop {
#[cfg(target_family = "unix")]
unsafe {
libc::pause();
}
#[cfg(not(target_family = "unix"))]
std::thread::sleep(std::time::Duration::from_secs(9_999_999));
}
}
}

// The PyGILState_Ensure function will call pthread_exit during interpreter shutdown,
// which causes undefined behavior. Redirect to the "safe" version that hangs instead,
// as Python 3.14 does.
//
// See https://github.com/rust-lang/rust/issues/135929

// C-unwind only supported (and necessary) since 1.71. Python 3.14+ does not do
// pthread_exit from PyGILState_Ensure (https://github.com/python/cpython/issues/87135).
mod raw {
#[cfg(all(not(Py_3_14), rustc_has_extern_c_unwind))]
extern "C-unwind" {
#[cfg_attr(PyPy, link_name = "PyPyGILState_Ensure")]
pub fn PyGILState_Ensure() -> super::PyGILState_STATE;
}

#[cfg(not(all(not(Py_3_14), rustc_has_extern_c_unwind)))]
extern "C" {
#[cfg_attr(PyPy, link_name = "PyPyGILState_Ensure")]
pub fn PyGILState_Ensure() -> super::PyGILState_STATE;
}
}

#[cfg(not(Py_3_14))]
pub unsafe extern "C" fn PyGILState_Ensure() -> PyGILState_STATE {
let guard = HangThread;
// If `PyGILState_Ensure` calls `pthread_exit`, which it does on Python < 3.14
// when the interpreter is shutting down, this will cause a forced unwind.
// doing a forced unwind through a function with a Rust destructor is unspecified
// behavior.
//
// However, currently it runs the destructor, which will cause the thread to
// hang as it should.
//
// And if we don't catch the unwinding here, then one of our callers probably has a destructor,
// so it's unspecified behavior anyway, and on many configurations causes the process to abort.
//
// The alternative is for pyo3 to contain custom C or C++ code that catches the `pthread_exit`,
// but that's also annoying from a portability point of view.
//
// On Windows, `PyGILState_Ensure` calls `_endthreadex` instead, which AFAICT can't be caught
// and therefore will cause unsafety if there are pinned objects on the stack. AFAICT there's
// nothing we can do it other than waiting for Python 3.14 or not using Windows. At least,
// if there is nothing pinned on the stack, it won't cause the process to crash.
let ret: PyGILState_STATE = raw::PyGILState_Ensure();
std::mem::forget(guard);
ret
}

#[cfg(Py_3_14)]
pub use self::raw::PyGILState_Ensure;

extern "C" {
#[cfg_attr(PyPy, link_name = "PyPyGILState_Ensure")]
pub fn PyGILState_Ensure() -> PyGILState_STATE;
#[cfg_attr(PyPy, link_name = "PyPyGILState_Release")]
pub fn PyGILState_Release(arg1: PyGILState_STATE);
#[cfg(not(PyPy))]
Expand Down
25 changes: 25 additions & 0 deletions pytests/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,30 @@ fn issue_219() {
Python::with_gil(|_| {});
}

#[pyclass]
struct LockHolder {
#[allow(unused)]
// Mutex needed for the MSRV
sender: std::sync::Mutex<std::sync::mpsc::Sender<()>>,
}

// This will hammer the GIL once the LockHolder is dropped.
#[pyfunction]
fn hammer_gil_in_thread() -> LockHolder {
let (sender, receiver) = std::sync::mpsc::channel();
std::thread::spawn(move || {
receiver.recv().ok();
// now the interpreter has shut down, so hammer the GIL. In buggy
// versions of PyO3 this will cause a crash.
loop {
Python::with_gil(|_py| ());
}
});
LockHolder {
sender: std::sync::Mutex::new(sender),
}
}

#[pyfunction]
fn get_type_fully_qualified_name<'py>(obj: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyString>> {
obj.get_type().fully_qualified_name()
Expand All @@ -35,6 +59,7 @@ fn get_item_and_run_callback(dict: Bound<'_, PyDict>, callback: Bound<'_, PyAny>
#[pymodule(gil_used = false)]
pub fn misc(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_function(wrap_pyfunction!(issue_219, m)?)?;
m.add_function(wrap_pyfunction!(hammer_gil_in_thread, m)?)?;
m.add_function(wrap_pyfunction!(get_type_fully_qualified_name, m)?)?;
m.add_function(wrap_pyfunction!(accepts_bool, m)?)?;
m.add_function(wrap_pyfunction!(get_item_and_run_callback, m)?)?;
Expand Down
20 changes: 20 additions & 0 deletions pytests/tests/test_hammer_gil_in_thread.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from pyo3_pytests import misc


def make_loop():
# create a reference loop that will only be destroyed when the GC is called at the end
# of execution
start = []
cur = [start]
for _ in range(1000 * 1000 * 10):
cur = [cur]
start.append(cur)
return start


# set a bomb that will explode when modules are cleaned up
loopy = [make_loop()]


def test_hammer_gil():
loopy.append(misc.hammer_gil_in_thread())
Loading