Skip to content

Commit

Permalink
Add catch_unwind! macro to prevent panics crossing ffi boundaries
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed May 3, 2020
1 parent e9bec07 commit 2904fec
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down
4 changes: 2 additions & 2 deletions src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ where
#[doc(hidden)]
pub fn run_callback<T, F>(py: Python, callback: F) -> T
where
F: FnOnce() -> PyResult<T>,
F: FnOnce() -> PyResult<T> + std::panic::UnwindSafe,
T: PyCallbackOutput,
{
callback().unwrap_or_else(|e| {
crate::catch_unwind!(py, callback()).unwrap_or_else(|e| {
e.restore(py);
T::ERR_VALUE
})
Expand Down
27 changes: 24 additions & 3 deletions src/err.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -168,12 +169,20 @@ impl PyErr {
///
/// The error is cleared from the Python interpreter.
/// If no error is set, returns a `SystemError`.
pub fn fetch(_: Python) -> PyErr {
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);

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"));
panic!(msg)
}

PyErr::new_from_ffi_tuple(ptype, pvalue, ptraceback)
}
}
Expand Down Expand Up @@ -564,6 +573,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]
Expand All @@ -575,4 +585,15 @@ 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(|| PyErr::fetch(py)).is_err();
assert!(started_unwind);
}
}
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ mod internal_tricks;
pub mod marshal;
mod object;
mod objectprotocol;
#[macro_use]
pub mod panic;
pub mod prelude;
pub mod pycell;
pub mod pyclass;
Expand Down
36 changes: 36 additions & 0 deletions src/panic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use crate::exceptions::BaseException;
use crate::Python;
/// 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);

#[macro_export]
macro_rules! catch_unwind {
($py:ident, $block:expr) => {{
match std::panic::catch_unwind(|| -> $crate::PyResult<_> { $block }) {
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",
)))
}
}
}
}};
}

impl std::panic::UnwindSafe for Python<'_> {}
impl std::panic::RefUnwindSafe for Python<'_> {}

0 comments on commit 2904fec

Please sign in to comment.