ci: enable more tests on 3.14t (#5524)

davidhewitt · web-flow · commit 999ee8ab0fed · 2025-10-17T10:26:18.000Z
* ci: enable more tests on 3.14t

* simplify implementation of `UnraisableCapture`

* fix imports on 3.14t

* force object cleanup on 3.14t
diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs
@@ -95,7 +95,7 @@ fn test_buffer_referenced() {
 }
 
 #[test]
-#[cfg(all(Py_3_8, not(Py_GIL_DISABLED)))] // sys.unraisablehook not available until Python 3.8
+#[cfg(Py_3_8)] // sys.unraisablehook not available until Python 3.8
 fn test_releasebuffer_unraisable_error() {
     use pyo3::exceptions::PyValueError;
     use test_utils::UnraisableCapture;
@@ -120,20 +120,20 @@ fn test_releasebuffer_unraisable_error() {
     }
 
     Python::attach(|py| {
-        let capture = UnraisableCapture::install(py);
-
         let instance = Py::new(py, ReleaseBufferError {}).unwrap();
-        let env = [("ob", instance.clone_ref(py))].into_py_dict(py).unwrap();
 
-        assert!(capture.borrow(py).capture.is_none());
+        let (err, object) = UnraisableCapture::enter(py, |capture| {
+            let env = [("ob", instance.clone_ref(py))].into_py_dict(py).unwrap();
+
+            assert!(capture.take_capture().is_none());
 
-        py_assert!(py, *env, "bytes(ob) == b'hello world'");
+            py_assert!(py, *env, "bytes(ob) == b'hello world'");
+
+            capture.take_capture().unwrap()
+        });
 
-        let (err, object) = capture.borrow_mut(py).capture.take().unwrap();
         assert_eq!(err.to_string(), "ValueError: oh dear");
         assert!(object.is(&instance));
-
-        capture.borrow_mut(py).uninstall(py);
     });
 }
 
diff --git a/tests/test_class_attributes.rs b/tests/test_class_attributes.rs
@@ -151,7 +151,7 @@ fn recursive_class_attributes() {
 }
 
 #[test]
-#[cfg(all(Py_3_8, not(Py_GIL_DISABLED)))] // sys.unraisablehook not available until Python 3.8
+#[cfg(Py_3_8)] // sys.unraisablehook not available until Python 3.8
 fn test_fallible_class_attribute() {
     use pyo3::exceptions::PyValueError;
     use test_utils::UnraisableCapture;
@@ -168,29 +168,31 @@ fn test_fallible_class_attribute() {
     }
 
     Python::attach(|py| {
-        let capture = UnraisableCapture::install(py);
-        assert!(std::panic::catch_unwind(|| py.get_type::<BrokenClass>()).is_err());
+        let (err, object) = UnraisableCapture::enter(py, |capture| {
+            // Accessing the type will attempt to initialize the class attributes
+            assert!(std::panic::catch_unwind(|| py.get_type::<BrokenClass>()).is_err());
 
-        let (err, object) = capture.borrow_mut(py).capture.take().unwrap();
-        assert!(object.is_none(py));
+            capture.take_capture().unwrap()
+        });
 
+        assert!(object.is_none());
         assert_eq!(
             err.to_string(),
             "RuntimeError: An error occurred while initializing class BrokenClass"
         );
+
         let cause = err.cause(py).unwrap();
         assert_eq!(
             cause.to_string(),
             "RuntimeError: An error occurred while initializing `BrokenClass.fails_to_init`"
         );
+
         let cause = cause.cause(py).unwrap();
         assert_eq!(
             cause.to_string(),
             "ValueError: failed to create class attribute"
         );
         assert!(cause.cause(py).is_none());
-
-        capture.borrow_mut(py).uninstall(py);
     });
 }
 
diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs
@@ -1,5 +1,7 @@
 #![cfg(feature = "macros")]
 
+#[cfg(Py_3_8)]
+use pyo3::ffi::c_str;
 use pyo3::prelude::*;
 use pyo3::types::PyType;
 use pyo3::{py_run, PyClass};
@@ -615,7 +617,7 @@ fn access_frozen_class_without_gil() {
 }
 
 #[test]
-#[cfg(all(Py_3_8, not(Py_GIL_DISABLED)))] // sys.unraisablehook not available until Python 3.8
+#[cfg(Py_3_8)]
 #[cfg_attr(target_arch = "wasm32", ignore)]
 fn drop_unsendable_elsewhere() {
     use std::sync::{
@@ -637,35 +639,40 @@ fn drop_unsendable_elsewhere() {
     }
 
     Python::attach(|py| {
-        let capture = UnraisableCapture::install(py);
+        let (err, object) = UnraisableCapture::enter(py, |capture| {
+            let dropped = Arc::new(AtomicBool::new(false));
 
-        let dropped = Arc::new(AtomicBool::new(false));
-
-        let unsendable = Py::new(
-            py,
-            Unsendable {
-                dropped: dropped.clone(),
-            },
-        )
-        .unwrap();
-
-        py.detach(|| {
-            spawn(move || {
-                Python::attach(move |_py| {
-                    drop(unsendable);
-                });
-            })
-            .join()
+            let unsendable = Py::new(
+                py,
+                Unsendable {
+                    dropped: dropped.clone(),
+                },
+            )
             .unwrap();
-        });
 
-        assert!(!dropped.load(Ordering::SeqCst));
+            py.detach(|| {
+                spawn(move || {
+                    Python::attach(move |py| {
+                        drop(unsendable);
+                        // On the free-threaded build, dropping an object on its non-origin thread
+                        // will not immediately drop it because the refcounts need to be merged.
+                        //
+                        // Force GC to ensure the drop happens now on the wrong thread.
+                        py.run(c_str!("import gc; gc.collect()"), None, None)
+                            .unwrap();
+                    });
+                })
+                .join()
+                .unwrap();
+            });
+
+            assert!(!dropped.load(Ordering::SeqCst));
+
+            capture.take_capture().unwrap()
+        });
 
-        let (err, object) = capture.borrow_mut(py).capture.take().unwrap();
         assert_eq!(err.to_string(), "RuntimeError: test_class_basics::drop_unsendable_elsewhere::Unsendable is unsendable, but is being dropped on another thread");
-        assert!(object.is_none(py));
-
-        capture.borrow_mut(py).uninstall(py);
+        assert!(object.is_none());
     });
 }
 
diff --git a/tests/test_exceptions.rs b/tests/test_exceptions.rs
@@ -98,31 +98,28 @@ fn test_exception_nosegfault() {
 }
 
 #[test]
-#[cfg(all(Py_3_8, not(Py_GIL_DISABLED)))]
+#[cfg(Py_3_8)]
 fn test_write_unraisable() {
-    use pyo3::{exceptions::PyRuntimeError, ffi, types::PyNotImplemented};
-    use std::ptr;
+    use pyo3::{exceptions::PyRuntimeError, types::PyNotImplemented};
     use test_utils::UnraisableCapture;
 
     Python::attach(|py| {
-        let capture = UnraisableCapture::install(py);
+        UnraisableCapture::enter(py, |capture| {
+            let err = PyRuntimeError::new_err("foo");
+            err.write_unraisable(py, None);
 
-        assert!(capture.borrow(py).capture.is_none());
+            let (err, object) = capture.take_capture().unwrap();
 
-        let err = PyRuntimeError::new_err("foo");
-        err.write_unraisable(py, None);
+            assert_eq!(err.to_string(), "RuntimeError: foo");
+            assert!(object.is_none());
 
-        let (err, object) = capture.borrow_mut(py).capture.take().unwrap();
-        assert_eq!(err.to_string(), "RuntimeError: foo");
-        assert!(object.is_none(py));
+            let err = PyRuntimeError::new_err("bar");
+            err.write_unraisable(py, Some(&PyNotImplemented::get(py)));
 
-        let err = PyRuntimeError::new_err("bar");
-        err.write_unraisable(py, Some(&PyNotImplemented::get(py)));
+            let (err, object) = capture.take_capture().unwrap();
 
-        let (err, object) = capture.borrow_mut(py).capture.take().unwrap();
-        assert_eq!(err.to_string(), "RuntimeError: bar");
-        assert!(unsafe { ptr::eq(object.as_ptr(), ffi::Py_NotImplemented()) });
-
-        capture.borrow_mut(py).uninstall(py);
+            assert_eq!(err.to_string(), "RuntimeError: bar");
+            assert!(object.is(PyNotImplemented::get(py)));
+        });
     });
 }
diff --git a/tests/test_utils/mod.rs b/tests/test_utils/mod.rs
@@ -18,9 +18,11 @@ mod inner {
 
     use pyo3::prelude::*;
 
+    #[cfg(any(not(all(Py_GIL_DISABLED, Py_3_14)), all(feature = "macros", Py_3_8)))]
     use pyo3::sync::MutexExt;
     use pyo3::types::{IntoPyDict, PyList};
 
+    #[cfg(any(not(all(Py_GIL_DISABLED, Py_3_14)), all(feature = "macros", Py_3_8)))]
     use std::sync::{Mutex, PoisonError};
 
     use uuid::Uuid;
@@ -118,49 +120,88 @@ mod inner {
     }
 
     // sys.unraisablehook not available until Python 3.8
-    #[cfg(all(feature = "macros", Py_3_8, not(Py_GIL_DISABLED)))]
-    #[pyclass(crate = "pyo3")]
-    pub struct UnraisableCapture {
-        pub capture: Option<(PyErr, Py<PyAny>)>,
-        old_hook: Option<Py<PyAny>>,
+    #[cfg(all(feature = "macros", Py_3_8))]
+    pub struct UnraisableCapture<'py> {
+        hook: Bound<'py, UnraisableCaptureHook>,
     }
 
-    #[cfg(all(feature = "macros", Py_3_8, not(Py_GIL_DISABLED)))]
+    #[cfg(all(feature = "macros", Py_3_8))]
+    impl<'py> UnraisableCapture<'py> {
+        /// Runs the closure `f` with a custom sys.unraisablehook installed.
+        ///
+        /// `f`
+        pub fn enter<R>(py: Python<'py>, f: impl FnOnce(&Self) -> R) -> R {
+            // unraisablehook is a global, so only one thread can be using this struct at a time.
+            static UNRAISABLE_HOOK_MUTEX: Mutex<()> = Mutex::new(());
+
+            // NB this is best-effort, other tests could always modify sys.unraisablehook directly.
+            let mutex_guard = UNRAISABLE_HOOK_MUTEX
+                .lock_py_attached(py)
+                .unwrap_or_else(PoisonError::into_inner);
+
+            let guard = Self {
+                hook: UnraisableCaptureHook::install(py),
+            };
+
+            let result = f(&guard);
+
+            drop(guard);
+            drop(mutex_guard);
+
+            result
+        }
+
+        /// Takes the captured unraisable error, if any.
+        pub fn take_capture(&self) -> Option<(PyErr, Bound<'py, PyAny>)> {
+            let mut guard = self.hook.get().capture.lock().unwrap();
+            guard.take().map(|(e, o)| (e, o.into_bound(self.hook.py())))
+        }
+    }
+
+    #[cfg(all(feature = "macros", Py_3_8))]
+    impl Drop for UnraisableCapture<'_> {
+        fn drop(&mut self) {
+            let py = self.hook.py();
+            self.hook.get().uninstall(py);
+        }
+    }
+
+    #[cfg(all(feature = "macros", Py_3_8))]
+    #[pyclass(crate = "pyo3", frozen)]
+    struct UnraisableCaptureHook {
+        pub capture: Mutex<Option<(PyErr, Py<PyAny>)>>,
+        old_hook: Py<PyAny>,
+    }
+
+    #[cfg(all(feature = "macros", Py_3_8))]
     #[pymethods(crate = "pyo3")]
-    impl UnraisableCapture {
-        pub fn hook(&mut self, unraisable: Bound<'_, PyAny>) {
+    impl UnraisableCaptureHook {
+        pub fn hook(&self, unraisable: Bound<'_, PyAny>) {
             let err = PyErr::from_value(unraisable.getattr("exc_value").unwrap());
             let instance = unraisable.getattr("object").unwrap();
-            self.capture = Some((err, instance.into()));
+            self.capture.lock().unwrap().replace((err, instance.into()));
         }
     }
 
-    #[cfg(all(feature = "macros", Py_3_8, not(Py_GIL_DISABLED)))]
-    impl UnraisableCapture {
-        pub fn install(py: Python<'_>) -> Py<Self> {
+    #[cfg(all(feature = "macros", Py_3_8))]
+    impl UnraisableCaptureHook {
+        fn install(py: Python<'_>) -> Bound<'_, Self> {
             let sys = py.import("sys").unwrap();
+
             let old_hook = sys.getattr("unraisablehook").unwrap().into();
+            let capture = Mutex::new(None);
 
-            let capture = Py::new(
-                py,
-                UnraisableCapture {
-                    capture: None,
-                    old_hook: Some(old_hook),
-                },
-            )
-            .unwrap();
+            let capture = Bound::new(py, UnraisableCaptureHook { capture, old_hook }).unwrap();
 
-            sys.setattr("unraisablehook", capture.getattr(py, "hook").unwrap())
+            sys.setattr("unraisablehook", capture.getattr("hook").unwrap())
                 .unwrap();
 
             capture
         }
 
-        pub fn uninstall(&mut self, py: Python<'_>) {
-            let old_hook = self.old_hook.take().unwrap();
-
+        fn uninstall(&self, py: Python<'_>) {
             let sys = py.import("sys").unwrap();
-            sys.setattr("unraisablehook", old_hook).unwrap();
+            sys.setattr("unraisablehook", &self.old_hook).unwrap();
         }
     }
 
@@ -170,6 +211,7 @@ mod inner {
 
     /// catch_warnings is not thread-safe, so only one thread can be using this struct at
     /// a time.
+    #[cfg(not(all(Py_GIL_DISABLED, Py_3_14)))] // Python 3.14t has thread-safe catch_warnings
     static CATCH_WARNINGS_MUTEX: Mutex<()> = Mutex::new(());
 
     impl<'py> CatchWarnings<'py> {
@@ -178,6 +220,7 @@ mod inner {
             f: impl FnOnce(&Bound<'py, PyList>) -> PyResult<R>,
         ) -> PyResult<R> {
             // NB this is best-effort, other tests could always call the warnings API directly.
+            #[cfg(not(all(Py_GIL_DISABLED, Py_3_14)))]
             let _mutex_guard = CATCH_WARNINGS_MUTEX
                 .lock_py_attached(py)
                 .unwrap_or_else(PoisonError::into_inner);
