panic on attach if interpreter is shutting down (#5317)

* panic on attach if interpreter is shutting down

* fixup for older versions

* fix clippy

Author: davidhewitt

newsfragments/5317.added.md

@@ -0,0 +1 @@
+Add FFI definitions `Py_Version` and `Py_IsFinalizing`.

newsfragments/5317.changed.md

@@ -0,0 +1 @@
+`Python::attach` will now panic if the Python interpreter is in the process of shutting down.

newsfragments/5317.fixed.md

@@ -0,0 +1 @@
+Fix FFI definition `Py_Exit` (never returns, was `()` return value, now `!`).

newsfragments/5317.removed.md

@@ -0,0 +1 @@
+Remove private FFI definitions `_Py_IsCoreInitialized` and `_Py_InitializeMain`

pyo3-ffi/src/cpython/pylifecycle.rs

@@ -2,9 +2,10 @@ use crate::{PyConfig, PyPreConfig, PyStatus, Py_ssize_t};
 use libc::wchar_t;
 use std::ffi::{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,14 +57,19 @@ 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,
     allow_fork: 1,
     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

pyo3-ffi/src/pylifecycle.rs

@@ -18,7 +18,7 @@ extern "C" {
     #[cfg_attr(PyPy, link_name = "PyPy_AtExit")]
     pub fn Py_AtExit(func: Option<extern "C" fn()>) -> 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;
 }

src/internal/state.rs

@@ -57,6 +57,14 @@ 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.)
+        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() }
     }
@@ -75,8 +83,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 || is_finalizing() {
             // If the interpreter is not initialized, we cannot attach.
             return None;
         }
@@ -130,6 +138,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) {

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, R>(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, R>(f: F) -> R