hang instead of pthread_exit during interpreter shutdown (#4874)

* hang instead of pthread_exit during interpreter shutdown

see python/cpython#87135 and
rust-lang/rust#135929

* relnotes

* fix warnings

* version using pthread_cleanup_push

* add tests

* new attempt

* clippy

* comment

* msrv

* address review comments

* update comment

* add comment

* try to skip test on debug builds

---------

Co-authored-by: Ariel Ben-Yehuda <[email protected]>
Co-authored-by: David Hewitt <[email protected]>

Author: 3 people

newsfragments/4874.changed.md

@@ -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.

pyo3-build-config/src/lib.rs

@@ -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");
@@ -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})");
     }
 }

pyo3-ffi/src/pystate.rs

@@ -80,9 +80,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))]

pytests/src/misc.rs

@@ -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()
@@ -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)?)?;

pytests/tests/test_hammer_gil_in_thread.py

@@ -0,0 +1,28 @@
+import sysconfig
+
+import pytest
+
+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()]
+
+
+@pytest.mark.skipif(
+    sysconfig.get_config_var("Py_DEBUG"),
+    reason="causes a crash on debug builds, see discussion in https://github.com/PyO3/pyo3/pull/4874",
+)
+def test_hammer_gil():
+    loopy.append(misc.hammer_gil_in_thread())