From 5e1c2a560be623029eb96d2007f23f6767f7603e Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Wed, 13 Aug 2025 21:50:30 +0100 Subject: [PATCH 1/3] panic on attach if interpreter is shutting down --- newsfragments/5317.added.md | 1 + newsfragments/5317.changed.md | 1 + newsfragments/5317.fixed.md | 1 + newsfragments/5317.removed.md | 1 + pyo3-ffi/src/cpython/pylifecycle.rs | 34 +++++++++-------------------- pyo3-ffi/src/pylifecycle.rs | 8 ++++++- src/internal/state.rs | 14 ++++++++++-- src/marker.rs | 23 +++++-------------- 8 files changed, 39 insertions(+), 44 deletions(-) create mode 100644 newsfragments/5317.added.md create mode 100644 newsfragments/5317.changed.md create mode 100644 newsfragments/5317.fixed.md create mode 100644 newsfragments/5317.removed.md diff --git a/newsfragments/5317.added.md b/newsfragments/5317.added.md new file mode 100644 index 00000000000..b6a571de59c --- /dev/null +++ b/newsfragments/5317.added.md @@ -0,0 +1 @@ +Add FFI definitions `Py_Version` and `Py_IsFinalizing`. diff --git a/newsfragments/5317.changed.md b/newsfragments/5317.changed.md new file mode 100644 index 00000000000..7d80ec4cfdd --- /dev/null +++ b/newsfragments/5317.changed.md @@ -0,0 +1 @@ +`Python::attach` will now panic if the Python interpreter is in the process of shutting down. diff --git a/newsfragments/5317.fixed.md b/newsfragments/5317.fixed.md new file mode 100644 index 00000000000..0d0a752e338 --- /dev/null +++ b/newsfragments/5317.fixed.md @@ -0,0 +1 @@ +Fix FFI definition `Py_Exit` (never returns, was `()` return value, now `!`). diff --git a/newsfragments/5317.removed.md b/newsfragments/5317.removed.md new file mode 100644 index 00000000000..5269ccda918 --- /dev/null +++ b/newsfragments/5317.removed.md @@ -0,0 +1 @@ +Remove private FFI definitions `_Py_IsCoreInitialized` and `_Py_InitializeMain` diff --git a/pyo3-ffi/src/cpython/pylifecycle.rs b/pyo3-ffi/src/cpython/pylifecycle.rs index c259c369efd..9aa7d2287c2 100644 --- a/pyo3-ffi/src/cpython/pylifecycle.rs +++ b/pyo3-ffi/src/cpython/pylifecycle.rs @@ -2,9 +2,10 @@ use crate::{PyConfig, PyPreConfig, PyStatus, Py_ssize_t}; use libc::wchar_t; use std::os::raw::{c_char, c_int}; -// "private" functions in cpython/pylifecycle.h accepted in PEP 587 extern "C" { - // skipped _Py_SetStandardStreamEncoding; + + // skipped Py_FrozenMain + pub fn Py_PreInitialize(src_config: *const PyPreConfig) -> PyStatus; pub fn Py_PreInitializeFromBytesArgs( src_config: *const PyPreConfig, @@ -16,34 +17,14 @@ extern "C" { argc: Py_ssize_t, argv: *mut *mut wchar_t, ) -> PyStatus; - pub fn _Py_IsCoreInitialized() -> c_int; pub fn Py_InitializeFromConfig(config: *const PyConfig) -> PyStatus; - pub fn _Py_InitializeMain() -> PyStatus; pub fn Py_RunMain() -> c_int; pub fn Py_ExitStatusException(status: PyStatus) -> !; - // skipped _Py_RestoreSignals - // skipped Py_FdIsInteractive - // skipped _Py_FdIsInteractive - - // skipped _Py_SetProgramFullPath - - // skipped _Py_gitidentifier - // skipped _Py_getversion - - // skipped _Py_IsFinalizing - - // skipped _PyOS_URandom - // skipped _PyOS_URandomNonblock - - // skipped _Py_CoerceLegacyLocale - // skipped _Py_LegacyLocaleDetected - // skipped _Py_SetLocaleFromEnv - } #[cfg(Py_3_12)] @@ -76,6 +57,11 @@ pub const _PyInterpreterConfig_INIT: PyInterpreterConfig = PyInterpreterConfig { gil: PyInterpreterConfig_OWN_GIL, }; +// https://github.com/python/cpython/blob/902de283a8303177eb95bf5bc252d2421fcbd758/Include/cpython/pylifecycle.h#L63-L65 +#[cfg(Py_3_12)] +const _PyInterpreterConfig_LEGACY_CHECK_MULTI_INTERP_EXTENSIONS: c_int = + if cfg!(Py_GIL_DISABLED) { 1 } else { 0 }; + #[cfg(Py_3_12)] pub const _PyInterpreterConfig_LEGACY_INIT: PyInterpreterConfig = PyInterpreterConfig { use_main_obmalloc: 1, @@ -83,7 +69,7 @@ pub const _PyInterpreterConfig_LEGACY_INIT: PyInterpreterConfig = PyInterpreterC allow_exec: 1, allow_threads: 1, allow_daemon_threads: 1, - check_multi_interp_extensions: 0, + check_multi_interp_extensions: _PyInterpreterConfig_LEGACY_CHECK_MULTI_INTERP_EXTENSIONS, gil: PyInterpreterConfig_SHARED_GIL, }; @@ -96,4 +82,4 @@ extern "C" { } // skipped atexit_datacallbackfunc -// skipped _Py_AtExit +// skipped PyUnstable_AtExit diff --git a/pyo3-ffi/src/pylifecycle.rs b/pyo3-ffi/src/pylifecycle.rs index 3f051c54f7c..b1aab674394 100644 --- a/pyo3-ffi/src/pylifecycle.rs +++ b/pyo3-ffi/src/pylifecycle.rs @@ -18,7 +18,7 @@ extern "C" { #[cfg_attr(PyPy, link_name = "PyPy_AtExit")] pub fn Py_AtExit(func: Option) -> c_int; - pub fn Py_Exit(arg1: c_int); + pub fn Py_Exit(arg1: c_int) -> !; pub fn Py_Main(argc: c_int, argv: *mut *mut wchar_t) -> c_int; pub fn Py_BytesMain(argc: c_int, argv: *mut *mut c_char) -> c_int; @@ -89,4 +89,10 @@ type PyOS_sighandler_t = unsafe extern "C" fn(arg1: c_int); extern "C" { pub fn PyOS_getsig(arg1: c_int) -> PyOS_sighandler_t; pub fn PyOS_setsig(arg1: c_int, arg2: PyOS_sighandler_t) -> PyOS_sighandler_t; + + #[cfg(Py_3_11)] + pub static Py_Version: std::ffi::c_ulong; + + #[cfg(Py_3_13)] + pub fn Py_IsFinalizing() -> c_int; } diff --git a/src/internal/state.rs b/src/internal/state.rs index 8095071af88..b645c2e0331 100644 --- a/src/internal/state.rs +++ b/src/internal/state.rs @@ -57,6 +57,16 @@ impl AttachGuard { crate::interpreter_lifecycle::ensure_initialized(); + // Calling `PyGILState_Ensure` while finalizing may crash CPython in unpredictable + // ways, we'll make a best effort attempt here to avoid that. (There's a time of + // check to time-of-use issue, but it's better than nothing.) + // + // SAFETY: This API is always sound to call + #[cfg(Py_3_13)] + if unsafe { ffi::Py_IsFinalizing() } != 0 { + panic!("Cannot attach to the Python interpreter while it is finalizing."); + } + // SAFETY: We have ensured the Python interpreter is initialized. unsafe { Self::acquire_unchecked() } } @@ -75,8 +85,8 @@ impl AttachGuard { _ => {} } - // SAFETY: This API is always sound to call - if unsafe { ffi::Py_IsInitialized() } == 0 { + // SAFETY: These APIs are always sound to call + if unsafe { ffi::Py_IsInitialized() } == 0 || unsafe { ffi::Py_IsFinalizing() } != 0 { // If the interpreter is not initialized, we cannot attach. return None; } diff --git a/src/marker.rs b/src/marker.rs index bf0a2fad3a1..58d99432d72 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -392,6 +392,7 @@ impl Python<'_> { /// /// - If the [`auto-initialize`] feature is not enabled and the Python interpreter is not /// initialized. + /// - If the Python interpreter is in the process of [shutting down]. /// /// # Examples /// @@ -409,6 +410,7 @@ impl Python<'_> { /// ``` /// /// [`auto-initialize`]: https://pyo3.rs/main/features.html#auto-initialize + /// [shutting down]: https://docs.python.org/3/glossary.html#term-interpreter-shutdown #[inline] #[track_caller] pub fn attach(f: F) -> R @@ -422,6 +424,7 @@ impl Python<'_> { /// Variant of [`Python::attach`] which will do no work if the interpreter is in a /// state where it cannot be attached to: /// - in the middle of GC traversal + /// - in the process of shutting down /// - not initialized #[inline] #[track_caller] @@ -464,27 +467,13 @@ impl Python<'_> { /// Like [`Python::attach`] except Python interpreter state checking is skipped. /// - /// Normally when the GIL is acquired, we check that the Python interpreter is an - /// appropriate state (e.g. it is fully initialized). This function skips those - /// checks. + /// Normally when the GIL is acquired, PyO3 checks that the Python interpreter is + /// in an appropriate state (e.g. it is fully initialized). This function skips + /// those checks. /// /// # Safety /// /// If [`Python::attach`] would succeed, it is safe to call this function. - /// - /// In most cases, you should use [`Python::attach`]. - /// - /// A justified scenario for calling this function is during multi-phase interpreter - /// initialization when [`Python::attach`] would fail before - // this link is only valid on 3.8+not pypy and up. - #[cfg_attr( - all(Py_3_8, not(PyPy)), - doc = "[`_Py_InitializeMain`](crate::ffi::_Py_InitializeMain)" - )] - #[cfg_attr(any(not(Py_3_8), PyPy), doc = "`_Py_InitializeMain`")] - /// is called because the interpreter is only partially initialized. - /// - /// Behavior in other scenarios is not documented. #[inline] #[track_caller] pub unsafe fn with_gil_unchecked(f: F) -> R From 311f37c4e2e6faf382817dfae4e1839cc0f98c55 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 15 Aug 2025 09:47:54 +0100 Subject: [PATCH 2/3] fixup for older versions --- src/internal/state.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/internal/state.rs b/src/internal/state.rs index b645c2e0331..c59d5f9461a 100644 --- a/src/internal/state.rs +++ b/src/internal/state.rs @@ -60,10 +60,7 @@ impl AttachGuard { // Calling `PyGILState_Ensure` while finalizing may crash CPython in unpredictable // ways, we'll make a best effort attempt here to avoid that. (There's a time of // check to time-of-use issue, but it's better than nothing.) - // - // SAFETY: This API is always sound to call - #[cfg(Py_3_13)] - if unsafe { ffi::Py_IsFinalizing() } != 0 { + if is_finalizing() { panic!("Cannot attach to the Python interpreter while it is finalizing."); } @@ -86,7 +83,7 @@ impl AttachGuard { } // SAFETY: These APIs are always sound to call - if unsafe { ffi::Py_IsInitialized() } == 0 || unsafe { ffi::Py_IsFinalizing() } != 0 { + if unsafe { ffi::Py_IsInitialized() } == 0 || is_finalizing() { // If the interpreter is not initialized, we cannot attach. return None; } @@ -140,6 +137,18 @@ impl AttachGuard { } } +fn is_finalizing() -> bool { + // SAFTETY: always safe to call this + #[cfg(Py_3_13)] + unsafe { + ffi::Py_IsFinalizing() != 0 + } + + // can't reliably check this before 3.13 + #[cfg(not(Py_3_13))] + false +} + /// The Drop implementation for `AttachGuard` will decrement the attach count (and potentially detach). impl Drop for AttachGuard { fn drop(&mut self) { From 96a0cdea9c7ca8ca81e646a220c65254170b9601 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 15 Aug 2025 22:03:15 +0100 Subject: [PATCH 3/3] fix clippy --- src/internal/state.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/internal/state.rs b/src/internal/state.rs index c59d5f9461a..f397bde0b04 100644 --- a/src/internal/state.rs +++ b/src/internal/state.rs @@ -60,9 +60,10 @@ impl AttachGuard { // Calling `PyGILState_Ensure` while finalizing may crash CPython in unpredictable // ways, we'll make a best effort attempt here to avoid that. (There's a time of // check to time-of-use issue, but it's better than nothing.) - if is_finalizing() { - panic!("Cannot attach to the Python interpreter while it is finalizing."); - } + assert!( + !is_finalizing(), + "Cannot attach to the Python interpreter while it is finalizing." + ); // SAFETY: We have ensured the Python interpreter is initialized. unsafe { Self::acquire_unchecked() }