Skip to content

Commit 2904fec

Browse files
committed
Add catch_unwind! macro to prevent panics crossing ffi boundaries
1 parent e9bec07 commit 2904fec

File tree

5 files changed

+65
-5
lines changed

5 files changed

+65
-5
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
99

1010
### Changed
1111

12+
* Panics from Rust will now be caught and raised as Python errors. [#797](https://github.com/PyO3/pyo3/pull/797)
1213
* `PyObject` and `Py<T>` reference counts are now decremented sooner after `drop()`. [#851](https://github.com/PyO3/pyo3/pull/851)
1314
* When the GIL is held, the refcount is now decreased immediately on drop. (Previously would wait until just before releasing the GIL.)
1415
* 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.)

src/callback.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,10 @@ where
155155
#[doc(hidden)]
156156
pub fn run_callback<T, F>(py: Python, callback: F) -> T
157157
where
158-
F: FnOnce() -> PyResult<T>,
158+
F: FnOnce() -> PyResult<T> + std::panic::UnwindSafe,
159159
T: PyCallbackOutput,
160160
{
161-
callback().unwrap_or_else(|e| {
161+
crate::catch_unwind!(py, callback()).unwrap_or_else(|e| {
162162
e.restore(py);
163163
T::ERR_VALUE
164164
})

src/err.rs

+24-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
// Copyright (c) 2017-present PyO3 Project and Contributors
22

3+
use crate::panic::PanicException;
34
use crate::type_object::PyTypeObject;
45
use crate::types::PyType;
56
use crate::{exceptions, ffi};
67
use crate::{
7-
AsPyPointer, FromPy, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, ToBorrowedObject,
8-
ToPyObject,
8+
AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, ObjectProtocol, Py, PyAny, PyObject,
9+
Python, ToBorrowedObject, ToPyObject,
910
};
1011
use libc::c_int;
1112
use std::ffi::CString;
@@ -168,12 +169,20 @@ impl PyErr {
168169
///
169170
/// The error is cleared from the Python interpreter.
170171
/// If no error is set, returns a `SystemError`.
171-
pub fn fetch(_: Python) -> PyErr {
172+
pub fn fetch(py: Python) -> PyErr {
172173
unsafe {
173174
let mut ptype: *mut ffi::PyObject = std::ptr::null_mut();
174175
let mut pvalue: *mut ffi::PyObject = std::ptr::null_mut();
175176
let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut();
176177
ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback);
178+
179+
if ptype == PanicException::type_object().as_ptr() {
180+
let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue)
181+
.and_then(|obj| obj.extract().ok())
182+
.unwrap_or_else(|| String::from("Unwrapped panic from Python code"));
183+
panic!(msg)
184+
}
185+
177186
PyErr::new_from_ffi_tuple(ptype, pvalue, ptraceback)
178187
}
179188
}
@@ -564,6 +573,7 @@ pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> {
564573
#[cfg(test)]
565574
mod tests {
566575
use crate::exceptions;
576+
use crate::panic::PanicException;
567577
use crate::{PyErr, Python};
568578

569579
#[test]
@@ -575,4 +585,15 @@ mod tests {
575585
assert!(PyErr::occurred(py));
576586
drop(PyErr::fetch(py));
577587
}
588+
589+
#[test]
590+
fn fetching_panic_exception_panics() {
591+
let gil = Python::acquire_gil();
592+
let py = gil.python();
593+
let err: PyErr = PanicException::py_err("new panic");
594+
err.restore(py);
595+
assert!(PyErr::occurred(py));
596+
let started_unwind = std::panic::catch_unwind(|| PyErr::fetch(py)).is_err();
597+
assert!(started_unwind);
598+
}
578599
}

src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ mod internal_tricks;
189189
pub mod marshal;
190190
mod object;
191191
mod objectprotocol;
192+
#[macro_use]
193+
pub mod panic;
192194
pub mod prelude;
193195
pub mod pycell;
194196
pub mod pyclass;

src/panic.rs

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
use crate::exceptions::BaseException;
2+
use crate::Python;
3+
/// The exception raised when Rust code called from Python panics.
4+
///
5+
/// Like SystemExit, this exception is derived from BaseException so that
6+
/// it will typically propagate all the way through the stack and cause the
7+
/// Python interpreter to exit.
8+
pub struct PanicException {
9+
_private: (),
10+
}
11+
12+
pyo3_exception!(PanicException, BaseException);
13+
14+
#[macro_export]
15+
macro_rules! catch_unwind {
16+
($py:ident, $block:expr) => {{
17+
match std::panic::catch_unwind(|| -> $crate::PyResult<_> { $block }) {
18+
Ok(result) => result,
19+
Err(e) => {
20+
// Try to format the error in the same way panic does
21+
if let Some(string) = e.downcast_ref::<String>() {
22+
Err($crate::panic::PanicException::py_err((string.clone(),)))
23+
} else if let Some(s) = e.downcast_ref::<&str>() {
24+
Err($crate::panic::PanicException::py_err((s.to_string(),)))
25+
} else {
26+
Err($crate::panic::PanicException::py_err((
27+
"panic from Rust code",
28+
)))
29+
}
30+
}
31+
}
32+
}};
33+
}
34+
35+
impl std::panic::UnwindSafe for Python<'_> {}
36+
impl std::panic::RefUnwindSafe for Python<'_> {}

0 commit comments

Comments
 (0)