Skip to content

Commit

Permalink
fix exception handling on Python 3.12
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidhewitt
davidhewitt committed Jul 10, 2023
1 parent db960e5 commit 5dbe51d
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 19 deletions.
80 changes: 80 additions & 0 deletions src/err/err_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,37 @@ use crate::{

#[derive(Clone)]
pub(crate) struct PyErrStateNormalized {
#[cfg(not(Py_3_12))]
pub ptype: Py<PyType>,
pub pvalue: Py<PyBaseException>,
#[cfg(not(Py_3_12))]
pub ptraceback: Option<Py<PyTraceback>>,
}

#[cfg(not(Py_3_12))]
impl PyErrStateNormalized {
pub(crate) fn ptype<'py>(&'py self, py: Python<'py>) -> &'py PyType {
self.ptype.as_ref(py)
}

pub(crate) fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
self.ptraceback
.as_ref()
.map(|traceback| traceback.as_ref(py))
}
}

#[cfg(Py_3_12)]
impl PyErrStateNormalized {
pub(crate) fn ptype<'py>(&'py self, py: Python<'py>) -> &'py PyType {
self.pvalue.as_ref(py).get_type()
}

pub(crate) fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
unsafe { py.from_owned_ptr_or_opt(ffi::PyException_GetTraceback(self.pvalue.as_ptr())) }
}
}

pub(crate) enum PyErrState {
LazyTypeAndValue {
ptype: for<'py> fn(Python<'py>) -> &PyType,
Expand All @@ -21,6 +47,7 @@ pub(crate) enum PyErrState {
ptype: Py<PyType>,
pvalue: Box<dyn for<'py> FnOnce(Python<'py>) -> PyObject + Send + Sync>,
},
#[cfg(not(Py_3_12))]
FfiTuple {
ptype: PyObject,
pvalue: Option<PyObject>,
Expand Down Expand Up @@ -51,6 +78,7 @@ pub(crate) fn boxed_args(
}

impl PyErrState {
#[cfg(not(Py_3_12))]
pub(crate) fn into_ffi_tuple(
self,
py: Python<'_>,
Expand Down Expand Up @@ -86,6 +114,58 @@ impl PyErrState {
}
}

#[cfg(Py_3_12)]
pub(crate) fn into_ffi_value(self, py: Python<'_>) -> *mut ffi::PyObject {
use crate::types::PyTuple;

fn create_exception(ptype: &PyType, pvalue: PyObject) -> *mut ffi::PyObject {
// reproduction of _PyErr_CreateException
// https://github.com/python/cpython/blob/9d582250d8fde240b8e7299b74ba888c574f74a3/Python/errors.c#L39C1-L39C23
let py = ptype.py();

let exc = if pvalue.is_none(py) {
ptype.call0()
} else if let Ok(tuple) = pvalue.downcast::<PyTuple>(py) {
ptype.call1(tuple)
} else {
ptype.call1((pvalue,))
};

let exc = match exc {
Ok(exc) => exc,
Err(e) => return e.into_value(py).into_ptr(),
};

if !exc.is_instance_of::<PyBaseException>() {
return crate::exceptions::PyTypeError::new_err(format!(
"calling {:?} should have returned an instance of BaseException, not {}",
ptype,
// FIXME get_type can probably be made infallible?
exc.get_type().name().unwrap(),
))
.into_value(py)
.into_ptr();
}

exc.into_ptr()
}

match self {
PyErrState::LazyTypeAndValue { ptype, pvalue } => {
let ty = ptype(py);
if unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) } == 0 {
Self::exceptions_must_derive_from_base_exception(py).into_ffi_value(py)
} else {
create_exception(ptype(py), pvalue(py))
}
}
PyErrState::LazyValue { ptype, pvalue } => {
create_exception(ptype.as_ref(py), pvalue(py))
}
PyErrState::Normalized(PyErrStateNormalized { pvalue }) => pvalue.into_ptr(),
}
}

#[inline]
pub(crate) fn exceptions_must_derive_from_base_exception(py: Python<'_>) -> Self {
PyErrState::LazyValue {
Expand Down
98 changes: 82 additions & 16 deletions src/err/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,27 +181,32 @@ impl PyErr {
/// });
/// ```
pub fn from_value(obj: &PyAny) -> PyErr {
let py = obj.py();
let ptr = obj.as_ptr();

let state = if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 {
PyErrState::Normalized(PyErrStateNormalized {
#[cfg(not(Py_3_12))]
ptype: obj.get_type().into(),
pvalue: unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ptr()) },
pvalue: unsafe { Py::from_borrowed_ptr(py, ptr) },
#[cfg(not(Py_3_12))]
ptraceback: None,
})
} else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 {
PyErrState::FfiTuple {
ptype: obj.into(),
pvalue: None,
ptraceback: None,
} else if unsafe { ffi::PyExceptionClass_Check(ptr) } != 0 {
PyErrState::LazyValue {
ptype: unsafe { Py::from_borrowed_ptr(py, ptr) },
pvalue: Box::new(|py| py.None()),
}
} else {
return exceptions_must_derive_from_base_exception(obj.py());
return exceptions_must_derive_from_base_exception(py);
};

PyErr::from_state(state)
}

#[cfg(not(Py_3_12))]
fn _from_value(obj: &PyAny) -> PyErr {}

/// Returns the type of this exception.
///
/// The object will be normalized first if needed.
Expand All @@ -216,7 +221,7 @@ impl PyErr {
/// });
/// ```
pub fn get_type<'py>(&'py self, py: Python<'py>) -> &'py PyType {
self.normalized(py).ptype.as_ref(py)
self.normalized(py).ptype(py)
}

/// Returns the value of this exception.
Expand Down Expand Up @@ -260,10 +265,7 @@ impl PyErr {
/// });
/// ```
pub fn traceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
self.normalized(py)
.ptraceback
.as_ref()
.map(|obj| obj.as_ref(py))
self.normalized(py).ptraceback(py)
}

/// Gets whether an error is present in the Python interpreter's global state.
Expand All @@ -282,6 +284,11 @@ impl PyErr {
/// expected to have been set, for example from [`PyErr::occurred`] or by an error return value
/// from a C FFI function, use [`PyErr::fetch`].
pub fn take(py: Python<'_>) -> Option<PyErr> {
PyErr::_take(py)
}

#[cfg(not(Py_3_12))]
fn _take(py: Python<'_>) -> Option<PyErr> {
let (ptype, pvalue, ptraceback) = unsafe {
let mut ptype: *mut ffi::PyObject = std::ptr::null_mut();
let mut pvalue: *mut ffi::PyObject = std::ptr::null_mut();
Expand Down Expand Up @@ -314,6 +321,7 @@ impl PyErr {
};

if ptype.as_ptr() == PanicException::type_object_raw(py).cast() {
// FIXME if the exception is normalized, this message isn't correct?
let msg: String = pvalue
.as_ref()
.and_then(|obj| obj.extract(py).ok())
Expand All @@ -339,6 +347,29 @@ impl PyErr {
}))
}

#[cfg(Py_3_12)]
fn _take(py: Python<'_>) -> Option<PyErr> {
let pvalue = unsafe { Py::from_owned_ptr_or_opt(py, ffi::PyErr_GetRaisedException()) }?;
let state = PyErrStateNormalized { pvalue };
if state.ptype(py).as_ptr() == PanicException::type_object_raw(py).cast() {
let msg: String = state.pvalue.as_ref(py).to_string();

eprintln!(
"--- PyO3 is resuming a panic after fetching a PanicException from Python. ---"
);
eprintln!("Python stack trace below:");

unsafe {
ffi::PyErr_SetRaisedException(state.pvalue.into_ptr());
ffi::PyErr_PrintEx(0);
}

std::panic::resume_unwind(Box::new(msg))
}

Some(PyErr::from_state(PyErrState::Normalized(state)))
}

/// Equivalent to [PyErr::take], but when no error is set:
/// - Panics in debug mode.
/// - Returns a `SystemError` in release mode.
Expand Down Expand Up @@ -470,8 +501,14 @@ impl PyErr {
None => unsafe { std::hint::unreachable_unchecked() },
};

let (ptype, pvalue, ptraceback) = state.into_ffi_tuple(py);
unsafe { ffi::PyErr_Restore(ptype, pvalue, ptraceback) }
cfg_if::cfg_if! {
if #[cfg(Py_3_12)] {
unsafe { ffi::PyErr_SetRaisedException(state.into_ffi_value(py))}
} else {
let (ptype, pvalue, ptraceback) = state.into_ffi_tuple(py);
unsafe { ffi::PyErr_Restore(ptype, pvalue, ptraceback) }
}
}
}

/// Reports the error as unraisable.
Expand Down Expand Up @@ -635,10 +672,11 @@ impl PyErr {
match unsafe { &*self.state.get() } {
// In lazy type case, normalize before returning ptype in case the type is not a valid
// exception type.
Some(PyErrState::LazyTypeAndValue { .. }) => self.normalized(py).ptype.as_ptr(),
Some(PyErrState::LazyTypeAndValue { .. }) => self.normalized(py).ptype(py).as_ptr(),
Some(PyErrState::LazyValue { ptype, .. }) => ptype.as_ptr(),
#[cfg(not(Py_3_12))]
Some(PyErrState::FfiTuple { ptype, .. }) => ptype.as_ptr(),
Some(PyErrState::Normalized(n)) => n.ptype.as_ptr(),
Some(PyErrState::Normalized(n)) => n.ptype(py).as_ptr(),
None => panic!("Cannot access exception type while normalizing"),
}
}
Expand All @@ -656,6 +694,7 @@ impl PyErr {
}

#[cold]
#[cfg(not(Py_3_12))]
fn make_normalized(&self, py: Python<'_>) -> &PyErrStateNormalized {
// This process is safe because:
// - Access is guaranteed not to be concurrent thanks to `Python` GIL token
Expand Down Expand Up @@ -685,6 +724,33 @@ impl PyErr {
}
}
}

#[cold]
#[cfg(Py_3_12)]
fn make_normalized(&self, py: Python<'_>) -> &PyErrStateNormalized {
// This process is safe because:
// - Access is guaranteed not to be concurrent thanks to `Python` GIL token
// - Write happens only once, and then never will change again.
// - State is set to None during the normalization process, so that a second
// concurrent normalization attempt will panic before changing anything.

let state = unsafe {
(*self.state.get())
.take()
.expect("Cannot normalize a PyErr while already normalizing it.")
};

unsafe {
let self_state = &mut *self.state.get();
*self_state = Some(PyErrState::Normalized(PyErrStateNormalized {
pvalue: Py::from_owned_ptr(py, state.into_ffi_value(py)),
}));
match self_state {
Some(PyErrState::Normalized(n)) => n,
_ => unreachable!(),
}
}
}
}

impl std::fmt::Debug for PyErr {
Expand Down
8 changes: 5 additions & 3 deletions src/exceptions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,13 @@ macro_rules! create_exception_type_object {
}

macro_rules! impl_native_exception (
($name:ident, $exc_name:ident, $doc:expr, $layout:path) => (
($name:ident, $exc_name:ident, $doc:expr, $layout:path $(, #checkfunction=$checkfunction:path)?) => (
#[doc = $doc]
#[allow(clippy::upper_case_acronyms)]
pub struct $name($crate::PyAny);

$crate::impl_exception_boilerplate!($name);
$crate::pyobject_native_type!($name, $layout, |_py| unsafe { $crate::ffi::$exc_name as *mut $crate::ffi::PyTypeObject });
$crate::pyobject_native_type!($name, $layout, |_py| unsafe { $crate::ffi::$exc_name as *mut $crate::ffi::PyTypeObject } $(, #checkfunction=$checkfunction)?);
);
($name:ident, $exc_name:ident, $doc:expr) => (
impl_native_exception!($name, $exc_name, $doc, $crate::ffi::PyBaseExceptionObject);
Expand Down Expand Up @@ -359,7 +359,9 @@ Python::with_gil(|py| {
impl_native_exception!(
PyBaseException,
PyExc_BaseException,
native_doc!("BaseException")
native_doc!("BaseException"),
ffi::PyBaseExceptionObject,
#checkfunction=ffi::PyExceptionInstance_Check
);
impl_native_exception!(PyException, PyExc_Exception, native_doc!("Exception"));
impl_native_exception!(
Expand Down

0 comments on commit 5dbe51d

Please sign in to comment.