diff --git a/tests/common.rs b/tests/common.rs index 75a246a9..1003d546 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -1,5 +1,8 @@ //! Some common macros for tests +#[cfg(all(feature = "macros", Py_3_8))] +use pyo3::prelude::*; + #[macro_export] macro_rules! py_assert { ($py:expr, $($val:ident)+, $assertion:literal) => { @@ -41,3 +44,50 @@ macro_rules! py_expect_exception { err }}; } + +// sys.unraisablehook not available until Python 3.8 +#[cfg(all(feature = "macros", Py_3_8))] +#[pyclass] +pub struct UnraisableCapture { + pub capture: Option<(PyErr, PyObject)>, + old_hook: Option, +} + +#[cfg(all(feature = "macros", Py_3_8))] +#[pymethods] +impl UnraisableCapture { + pub fn hook(&mut self, unraisable: &PyAny) { + let err = PyErr::from_value(unraisable.getattr("exc_value").unwrap()); + let instance = unraisable.getattr("object").unwrap(); + self.capture = Some((err, instance.into())); + } +} + +#[cfg(all(feature = "macros", Py_3_8))] +impl UnraisableCapture { + pub fn install(py: Python<'_>) -> Py { + let sys = py.import("sys").unwrap(); + let old_hook = sys.getattr("unraisablehook").unwrap().into(); + + let capture = Py::new( + py, + UnraisableCapture { + capture: None, + old_hook: Some(old_hook), + }, + ) + .unwrap(); + + sys.setattr("unraisablehook", capture.getattr(py, "hook").unwrap()) + .unwrap(); + + capture + } + + pub fn uninstall(&mut self, py: Python<'_>) { + let old_hook = self.old_hook.take().unwrap(); + + let sys = py.import("sys").unwrap(); + sys.setattr("unraisablehook", old_hook).unwrap(); + } +} diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs index c3843909..a9c0ff8e 100644 --- a/tests/test_buffer_protocol.rs +++ b/tests/test_buffer_protocol.rs @@ -96,6 +96,7 @@ fn test_buffer_referenced() { #[test] #[cfg(Py_3_8)] // sys.unraisablehook not available until Python 3.8 fn test_releasebuffer_unraisable_error() { + use common::UnraisableCapture; use pyo3::exceptions::PyValueError; #[pyclass] @@ -117,27 +118,8 @@ fn test_releasebuffer_unraisable_error() { } } - #[pyclass] - struct UnraisableCapture { - capture: Option<(PyErr, PyObject)>, - } - - #[pymethods] - impl UnraisableCapture { - fn hook(&mut self, unraisable: &PyAny) { - let err = PyErr::from_value(unraisable.getattr("exc_value").unwrap()); - let instance = unraisable.getattr("object").unwrap(); - self.capture = Some((err, instance.into())); - } - } - Python::with_gil(|py| { - let sys = py.import("sys").unwrap(); - let old_hook = sys.getattr("unraisablehook").unwrap(); - let capture = Py::new(py, UnraisableCapture { capture: None }).unwrap(); - - sys.setattr("unraisablehook", capture.getattr(py, "hook").unwrap()) - .unwrap(); + let capture = UnraisableCapture::install(py); let instance = Py::new(py, ReleaseBufferError {}).unwrap(); let env = [("ob", instance.clone())].into_py_dict(py); @@ -150,7 +132,7 @@ fn test_releasebuffer_unraisable_error() { assert_eq!(err.to_string(), "ValueError: oh dear"); assert!(object.is(&instance)); - sys.setattr("unraisablehook", old_hook).unwrap(); + capture.borrow_mut(py).uninstall(py); }); } diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index deff52dc..a0ec4b0c 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -228,31 +228,40 @@ impl UnsendableChild { } fn test_unsendable() -> PyResult<()> { - let obj = std::thread::spawn(|| -> PyResult<_> { + let obj = Python::with_gil(|py| -> PyResult<_> { + let obj: Py = PyType::new::(py).call1((5,))?.extract()?; + + // Accessing the value inside this thread should not panic + let caught_panic = + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| -> PyResult<_> { + assert_eq!(obj.as_ref(py).getattr("value")?.extract::()?, 5); + Ok(()) + })) + .is_err(); + + assert!(!caught_panic); + Ok(obj) + })?; + + let keep_obj_here = obj.clone(); + + let caught_panic = std::thread::spawn(move || { + // This access must panic Python::with_gil(|py| { - let obj: Py = PyType::new::(py).call1((5,))?.extract()?; - - // Accessing the value inside this thread should not panic - let caught_panic = - std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| -> PyResult<_> { - assert_eq!(obj.as_ref(py).getattr("value")?.extract::()?, 5); - Ok(()) - })) - .is_err(); - - assert!(!caught_panic); - Ok(obj) - }) + obj.borrow(py); + }); }) - .join() - .unwrap()?; + .join(); - // This access must panic - Python::with_gil(|py| { - obj.borrow(py); - }); + Python::with_gil(|_py| drop(keep_obj_here)); - panic!("Borrowing unsendable from receiving thread did not panic."); + if let Err(err) = caught_panic { + if let Some(msg) = err.downcast_ref::() { + panic!("{}", msg); + } + } + + Ok(()) } /// If a class is marked as `unsendable`, it panics when accessed by another thread. @@ -528,7 +537,10 @@ fn access_frozen_class_without_gil() { } #[test] +#[cfg(Py_3_8)] // sys.unraisablehook not available until Python 3.8 +#[cfg_attr(target_arch = "wasm32", ignore)] fn drop_unsendable_elsewhere() { + use common::UnraisableCapture; use std::sync::{ atomic::{AtomicBool, Ordering}, Arc, @@ -546,21 +558,35 @@ fn drop_unsendable_elsewhere() { } } - let dropped = Arc::new(AtomicBool::new(false)); + Python::with_gil(|py| { + let capture = UnraisableCapture::install(py); - let unsendable = Python::with_gil(|py| { - let dropped = dropped.clone(); + let dropped = Arc::new(AtomicBool::new(false)); - Py::new(py, Unsendable { dropped }).unwrap() - }); + let unsendable = Py::new( + py, + Unsendable { + dropped: dropped.clone(), + }, + ) + .unwrap(); - spawn(move || { - Python::with_gil(move |_py| { - drop(unsendable); + py.allow_threads(|| { + spawn(move || { + Python::with_gil(move |_py| { + drop(unsendable); + }); + }) + .join() + .unwrap(); }); - }) - .join() - .unwrap(); - assert!(!dropped.load(Ordering::SeqCst)); + assert!(!dropped.load(Ordering::SeqCst)); + + let (err, object) = capture.borrow_mut(py).capture.take().unwrap(); + assert_eq!(err.to_string(), "RuntimeError: test_class_basics::drop_unsendable_elsewhere::Unsendable is unsendbale, but is dropped on another thread!"); + assert!(object.is_none(py)); + + capture.borrow_mut(py).uninstall(py); + }); } diff --git a/tests/test_exceptions.rs b/tests/test_exceptions.rs index cbbbd636..c0cf57a9 100644 --- a/tests/test_exceptions.rs +++ b/tests/test_exceptions.rs @@ -100,29 +100,11 @@ fn test_exception_nosegfault() { #[test] #[cfg(Py_3_8)] fn test_write_unraisable() { + use common::UnraisableCapture; use pyo3::{exceptions::PyRuntimeError, ffi, AsPyPointer}; - #[pyclass] - struct UnraisableCapture { - capture: Option<(PyErr, PyObject)>, - } - - #[pymethods] - impl UnraisableCapture { - fn hook(&mut self, unraisable: &PyAny) { - let err = PyErr::from_value(unraisable.getattr("exc_value").unwrap()); - let instance = unraisable.getattr("object").unwrap(); - self.capture = Some((err, instance.into())); - } - } - Python::with_gil(|py| { - let sys = py.import("sys").unwrap(); - let old_hook = sys.getattr("unraisablehook").unwrap(); - let capture = Py::new(py, UnraisableCapture { capture: None }).unwrap(); - - sys.setattr("unraisablehook", capture.getattr(py, "hook").unwrap()) - .unwrap(); + let capture = UnraisableCapture::install(py); assert!(capture.borrow(py).capture.is_none()); @@ -140,6 +122,6 @@ fn test_write_unraisable() { assert_eq!(err.to_string(), "RuntimeError: bar"); assert!(object.as_ptr() == unsafe { ffi::Py_NotImplemented() }); - sys.setattr("unraisablehook", old_hook).unwrap(); + capture.borrow_mut(py).uninstall(py); }); }