diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs index 5a2baa33d05..9af163f515b 100644 --- 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 index e8d104d9d10..f706b414ff3 100644 --- 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::()).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::()).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 index 1dda580c2a4..dfe7f1ef0a5 100644 --- 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 index 97b5466d205..d09dd0a1dc2 100644 --- 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 index 235adc2f99d..e154c16372d 100644 --- 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)>, - old_hook: Option>, + #[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(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)>>, + old_hook: Py, + } + + #[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 { + #[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, ) -> PyResult { // 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);