From 877e34ac86b46597c30f4ad978868799ba2be791 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sun, 24 Dec 2023 14:49:43 +0000 Subject: [PATCH 1/2] implement `PyErr::write_unraisable_bound` --- src/conversions/chrono.rs | 3 ++- src/err/mod.rs | 16 +++++++++++++--- src/impl_/pyclass.rs | 2 +- src/impl_/trampoline.rs | 6 +++--- src/instance.rs | 7 +++---- src/types/mapping.rs | 5 ++++- src/types/mod.rs | 2 +- src/types/sequence.rs | 5 ++++- tests/test_exceptions.rs | 1 + 9 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs index 0ad5b445..3b53e7ce 100644 --- a/src/conversions/chrono.rs +++ b/src/conversions/chrono.rs @@ -42,6 +42,7 @@ //! } //! ``` use crate::exceptions::{PyTypeError, PyUserWarning, PyValueError}; +use crate::instance::Bound; #[cfg(Py_LIMITED_API)] use crate::sync::GILOnceCell; #[cfg(not(Py_LIMITED_API))] @@ -465,7 +466,7 @@ fn warn_truncated_leap_second(obj: &PyAny) { "ignored leap-second, `datetime` does not support leap-seconds", 0, ) { - e.write_unraisable(py, Some(obj)) + e.write_unraisable_bound(py, Some(Bound::borrowed_from_gil_ref(&obj))) }; } diff --git a/src/err/mod.rs b/src/err/mod.rs index 00ba111f..c2d47a8f 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -535,6 +535,16 @@ impl PyErr { .restore(py) } + /// Deprecated form of `PyErr::write_unraisable_bound`. + #[deprecated( + since = "0.21.0", + note = "`PyErr::write_unraisable` will be replaced by `PyErr::write_unraisable_bound` in a future PyO3 version" + )] + #[inline] + pub fn write_unraisable(self, py: Python<'_>, obj: Option<&PyAny>) { + self.write_unraisable_bound(py, obj.as_ref().map(Bound::borrowed_from_gil_ref)) + } + /// Reports the error as unraisable. /// /// This calls `sys.unraisablehook()` using the current exception and obj argument. @@ -557,16 +567,16 @@ impl PyErr { /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| { /// match failing_function() { - /// Err(pyerr) => pyerr.write_unraisable(py, None), + /// Err(pyerr) => pyerr.write_unraisable_bound(py, None), /// Ok(..) => { /* do something here */ } /// } /// Ok(()) /// }) /// # } #[inline] - pub fn write_unraisable(self, py: Python<'_>, obj: Option<&PyAny>) { + pub fn write_unraisable_bound(self, py: Python<'_>, obj: Option<&Bound<'_, PyAny>>) { self.restore(py); - unsafe { ffi::PyErr_WriteUnraisable(obj.map_or(std::ptr::null_mut(), |x| x.as_ptr())) } + unsafe { ffi::PyErr_WriteUnraisable(obj.map_or(std::ptr::null_mut(), Bound::as_ptr)) } } /// Issues a warning message. diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 5ee67dc9..23cebb26 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -1067,7 +1067,7 @@ impl ThreadCheckerImpl { "{} is unsendable, but is being dropped on another thread", type_name )) - .write_unraisable(py, None); + .write_unraisable_bound(py, None); return false; } diff --git a/src/impl_/trampoline.rs b/src/impl_/trampoline.rs index 5ae75e0f..4b4eac17 100644 --- a/src/impl_/trampoline.rs +++ b/src/impl_/trampoline.rs @@ -10,8 +10,8 @@ use std::{ }; use crate::{ - callback::PyCallbackOutput, ffi, impl_::panic::PanicTrap, methods::IPowModulo, - panic::PanicException, types::PyModule, GILPool, Py, PyResult, Python, + callback::PyCallbackOutput, ffi, ffi_ptr_ext::FfiPtrExt, impl_::panic::PanicTrap, + methods::IPowModulo, panic::PanicException, types::PyModule, GILPool, Py, PyResult, Python, }; #[inline] @@ -224,7 +224,7 @@ where if let Err(py_err) = panic::catch_unwind(move || body(py)) .unwrap_or_else(|payload| Err(PanicException::from_panic_payload(payload))) { - py_err.write_unraisable(py, py.from_borrowed_ptr_or_opt(ctx)); + py_err.write_unraisable_bound(py, ctx.assume_borrowed_or_opt(py).as_deref()); } trap.disarm(); } diff --git a/src/instance.rs b/src/instance.rs index 8406e86a..c3de539a 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -3,6 +3,7 @@ use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell}; use crate::pyclass::boolean_struct::{False, True}; use crate::type_object::HasPyGilRef; use crate::types::any::PyAnyMethods; +use crate::types::string::PyStringMethods; use crate::types::{PyDict, PyString, PyTuple}; use crate::{ ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, PyRef, PyRefMut, @@ -97,10 +98,8 @@ fn python_format( f: &mut std::fmt::Formatter<'_>, ) -> Result<(), std::fmt::Error> { match format_result { - Result::Ok(s) => return f.write_str(&s.as_gil_ref().to_string_lossy()), - Result::Err(err) => { - err.write_unraisable(any.py(), std::option::Option::Some(any.as_gil_ref())) - } + Result::Ok(s) => return f.write_str(&s.to_string_lossy()), + Result::Err(err) => err.write_unraisable_bound(any.py(), Some(any)), } match any.get_type().name() { diff --git a/src/types/mapping.rs b/src/types/mapping.rs index 4eae0cec..239ade9b 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -257,7 +257,10 @@ impl PyTypeCheck for PyMapping { || get_mapping_abc(object.py()) .and_then(|abc| object.is_instance(abc)) .unwrap_or_else(|err| { - err.write_unraisable(object.py(), Some(object)); + err.write_unraisable_bound( + object.py(), + Some(Bound::borrowed_from_gil_ref(&object)), + ); false }) } diff --git a/src/types/mod.rs b/src/types/mod.rs index 0ff41f80..f153c4d9 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -107,7 +107,7 @@ macro_rules! pyobject_native_type_base( { match self.str() { ::std::result::Result::Ok(s) => return f.write_str(&s.to_string_lossy()), - ::std::result::Result::Err(err) => err.write_unraisable(self.py(), ::std::option::Option::Some(self)), + ::std::result::Result::Err(err) => err.write_unraisable_bound(self.py(), ::std::option::Option::Some($crate::Bound::borrowed_from_gil_ref(&self))), } match self.get_type().name() { diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 714288bd..9b497ded 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -541,7 +541,10 @@ impl PyTypeCheck for PySequence { || get_sequence_abc(object.py()) .and_then(|abc| object.is_instance(abc)) .unwrap_or_else(|err| { - err.write_unraisable(object.py(), Some(object)); + err.write_unraisable_bound( + object.py(), + Some(Bound::borrowed_from_gil_ref(&object)), + ); false }) } diff --git a/tests/test_exceptions.rs b/tests/test_exceptions.rs index 700a5c54..1cf443e6 100644 --- a/tests/test_exceptions.rs +++ b/tests/test_exceptions.rs @@ -100,6 +100,7 @@ fn test_exception_nosegfault() { #[test] #[cfg(Py_3_8)] +#[allow(deprecated)] fn test_write_unraisable() { use common::UnraisableCapture; use pyo3::{exceptions::PyRuntimeError, ffi}; From 1004ffa7d6b4cbdaab163913de76540f3fd21bcb Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Sun, 24 Dec 2023 19:35:50 +0000 Subject: [PATCH 2/2] export `Bound` and `Borrowed` from lib.rs --- src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f70ea01b..4802cbc2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -303,7 +303,7 @@ pub use crate::err::{ pub use crate::gil::GILPool; #[cfg(not(PyPy))] pub use crate::gil::{prepare_freethreaded_python, with_embedded_python_interpreter}; -pub use crate::instance::{Py, PyNativeType, PyObject}; +pub use crate::instance::{Borrowed, Bound, Py, PyNativeType, PyObject}; pub use crate::marker::Python; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; pub use crate::pyclass::PyClass; @@ -312,8 +312,6 @@ pub use crate::type_object::{PyTypeCheck, PyTypeInfo}; pub use crate::types::PyAny; pub use crate::version::PythonVersionInfo; -// Expected to become public API in 0.21 under a different name -pub(crate) use crate::instance::Bound; pub(crate) mod ffi_ptr_ext; pub(crate) mod py_result_ext;