diff --git a/newsfragments/4874.changed.md b/newsfragments/4874.changed.md new file mode 100644 index 00000000000..fd483f43de5 --- /dev/null +++ b/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. diff --git a/pyo3-build-config/src/lib.rs b/pyo3-build-config/src/lib.rs index 9070f6d7401..a908fe88068 100644 --- a/pyo3-build-config/src/lib.rs +++ b/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})"); } } diff --git a/pyo3-ffi/src/pystate.rs b/pyo3-ffi/src/pystate.rs index 23aeea3a1de..1a73731e729 100644 --- a/pyo3-ffi/src/pystate.rs +++ b/pyo3-ffi/src/pystate.rs @@ -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))] diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index e44d1aa0ecf..b3ef5ee283e 100644 --- a/pytests/src/misc.rs +++ b/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>, +} + +// 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> { 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)?)?; diff --git a/pytests/tests/test_hammer_gil_in_thread.py b/pytests/tests/test_hammer_gil_in_thread.py new file mode 100644 index 00000000000..cee1bb6b828 --- /dev/null +++ b/pytests/tests/test_hammer_gil_in_thread.py @@ -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())