Skip to content

Commit

Permalink
Merge pull request #3698 from davidhewitt/unraisable-bound
Browse files Browse the repository at this point in the history
implement `PyErr::write_unraisable_bound`
  • Loading branch information
davidhewitt authored Dec 24, 2023
2 parents 214ed29 + 1004ffa commit 6ca63b5
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 18 deletions.
3 changes: 2 additions & 1 deletion src/conversions/chrono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -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)))
};
}

Expand Down
16 changes: 13 additions & 3 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions src/impl_/trampoline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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();
}
7 changes: 3 additions & 4 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 1 addition & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
5 changes: 4 additions & 1 deletion src/types/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
5 changes: 4 additions & 1 deletion src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
Expand Down
1 change: 1 addition & 0 deletions tests/test_exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down

0 comments on commit 6ca63b5

Please sign in to comment.