Add catch_unwind! macro to prevent panics crossing ffi boundaries

This commit is contained in:
David Hewitt 2020-03-07 09:59:49 +00:00
parent c4f36535d8
commit 9380bfde6f
5 changed files with 74 additions and 8 deletions

View file

@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Changed
* Panics from Rust will now be caught and raised as Python errors. [#797](https://github.com/PyO3/pyo3/pull/797)
* `PyObject` and `Py<T>` reference counts are now decremented sooner after `drop()`. [#851](https://github.com/PyO3/pyo3/pull/851)
* When the GIL is held, the refcount is now decreased immediately on drop. (Previously would wait until just before releasing the GIL.)
* When the GIL is not held, the refcount is now decreased when the GIL is next acquired. (Previously would wait until next time the GIL was released.)

View file

@ -157,6 +157,8 @@ where
/// It sets up the GILPool and converts the output into a Python object. It also restores
/// any python error returned as an Err variant from the body.
///
/// Finally, any panics inside the callback body will be caught and translated into PanicExceptions.
///
/// # Safety
/// This macro assumes the GIL is held. (It makes use of unsafe code, so usage of it is only
/// possible inside unsafe blocks.)
@ -204,11 +206,27 @@ macro_rules! callback_body {
macro_rules! callback_body_without_convert {
($py:ident, $body:expr) => {{
let pool = $crate::GILPool::new();
let unwind_safe_py = std::panic::AssertUnwindSafe(pool.python());
let result = match std::panic::catch_unwind(move || -> $crate::PyResult<_> {
let $py = *unwind_safe_py;
$body
}) {
Ok(result) => result,
Err(e) => {
// Try to format the error in the same way panic does
if let Some(string) = e.downcast_ref::<String>() {
Err($crate::panic::PanicException::py_err((string.clone(),)))
} else if let Some(s) = e.downcast_ref::<&str>() {
Err($crate::panic::PanicException::py_err((s.to_string(),)))
} else {
Err($crate::panic::PanicException::py_err((
"panic from Rust code",
)))
}
}
};
let $py = pool.python();
let callback = move || -> $crate::PyResult<_> { $body };
callback().unwrap_or_else(|e| {
result.unwrap_or_else(|e| {
e.restore(pool.python());
$crate::callback::callback_error()
})

View file

@ -1,11 +1,12 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
use crate::panic::PanicException;
use crate::type_object::PyTypeObject;
use crate::types::PyType;
use crate::{exceptions, ffi};
use crate::{
AsPyPointer, FromPy, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, ToBorrowedObject,
ToPyObject,
AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, ObjectProtocol, Py, PyAny, PyObject,
Python, ToBorrowedObject, ToPyObject,
};
use libc::c_int;
use std::ffi::CString;
@ -168,13 +169,33 @@ impl PyErr {
///
/// The error is cleared from the Python interpreter.
/// If no error is set, returns a `SystemError`.
pub fn fetch(_: Python) -> PyErr {
///
/// If the error fetched is a `PanicException` (which would have originated from a panic in a
/// pyo3 callback) then this function will resume the panic.
pub fn fetch(py: Python) -> PyErr {
unsafe {
let mut ptype: *mut ffi::PyObject = std::ptr::null_mut();
let mut pvalue: *mut ffi::PyObject = std::ptr::null_mut();
let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut();
ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback);
PyErr::new_from_ffi_tuple(ptype, pvalue, ptraceback)
let err = PyErr::new_from_ffi_tuple(ptype, pvalue, ptraceback);
if ptype == PanicException::type_object().as_ptr() {
let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue)
.and_then(|obj| obj.extract().ok())
.unwrap_or_else(|| String::from("Unwrapped panic from Python code"));
eprintln!(
"--- PyO3 is resuming a panic after fetching a PanicException from Python. ---"
);
eprintln!("Python stack trace below:");
err.print(py);
std::panic::resume_unwind(Box::new(msg))
}
err
}
}
@ -564,6 +585,7 @@ pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> {
#[cfg(test)]
mod tests {
use crate::exceptions;
use crate::panic::PanicException;
use crate::{PyErr, Python};
#[test]
@ -575,4 +597,16 @@ mod tests {
assert!(PyErr::occurred(py));
drop(PyErr::fetch(py));
}
#[test]
fn fetching_panic_exception_panics() {
let gil = Python::acquire_gil();
let py = gil.python();
let err: PyErr = PanicException::py_err("new panic");
err.restore(py);
assert!(PyErr::occurred(py));
let started_unwind =
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| PyErr::fetch(py))).is_err();
assert!(started_unwind);
}
}

View file

@ -188,6 +188,7 @@ mod internal_tricks;
pub mod marshal;
mod object;
mod objectprotocol;
pub mod panic;
pub mod prelude;
pub mod pycell;
pub mod pyclass;

12
src/panic.rs Normal file
View file

@ -0,0 +1,12 @@
use crate::exceptions::BaseException;
/// The exception raised when Rust code called from Python panics.
///
/// Like SystemExit, this exception is derived from BaseException so that
/// it will typically propagate all the way through the stack and cause the
/// Python interpreter to exit.
pub struct PanicException {
_private: (),
}
pyo3_exception!(PanicException, BaseException);