Merge pull request #797 from davidhewitt/catch-unwind

Add catch_unwind! macro to prevent panics crossing ffi boundaries

Author: kngwyu

CHANGELOG.md

@@ -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.)

src/callback.rs

@@ -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()
         })

src/err.rs

@@ -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);
+    }
 }

src/lib.rs

@@ -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;

src/panic.rs

@@ -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);