Skip to content

Commit

Permalink
use panic-on-drop to trigger abort
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Aug 14, 2022
1 parent 216405e commit e716388
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 56 deletions.
23 changes: 12 additions & 11 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,19 +339,20 @@ fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec<'_>) -> MethodAndSlotDef {
arg: *mut ::std::os::raw::c_void,
) -> ::std::os::raw::c_int
{
let trap = _pyo3::impl_::panic::PanicTrap::new("uncaught panic inside __traverse__ handler");
let pool = _pyo3::GILPool::new();
let py = pool.python();
_pyo3::callback::abort_on_traverse_panic(::std::panic::catch_unwind(move || {
let slf = py.from_borrowed_ptr::<_pyo3::PyCell<#cls>>(slf);

let visit = _pyo3::class::gc::PyVisit::from_raw(visit, arg, py);
let borrow = slf.try_borrow();
if let ::std::result::Result::Ok(borrow) = borrow {
_pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#ident(visit))
} else {
0
}
}))
let slf = py.from_borrowed_ptr::<_pyo3::PyCell<#cls>>(slf);

let visit = _pyo3::class::gc::PyVisit::from_raw(visit, arg, py);
let borrow = slf.try_borrow();
let retval = if let ::std::result::Result::Ok(borrow) = borrow {
_pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#ident(visit))
} else {
0
};
trap.disarm();
retval
}
};
let slot_def = quote! {
Expand Down
15 changes: 2 additions & 13 deletions src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
use crate::err::{PyErr, PyResult};
use crate::exceptions::PyOverflowError;
use crate::ffi::{self, Py_hash_t};
use crate::panic::{abort_with_msg, PanicException, PanicTrap};
use crate::impl_::panic::PanicTrap;
use crate::panic::PanicException;
use crate::{GILPool, IntoPyPointer};
use crate::{IntoPy, PyObject, Python};
use std::any::Any;
Expand Down Expand Up @@ -273,15 +274,3 @@ where
py_err.restore(py);
R::ERR_VALUE
}

/// Aborts if panic has occurred. Used inside `__traverse__` implementations, where panicking is not possible.
#[doc(hidden)]
#[inline]
pub fn abort_on_traverse_panic(
panic_result: Result<c_int, Box<dyn Any + Send + 'static>>,
) -> c_int {
match panic_result {
Ok(traverse_result) => traverse_result,
Err(_payload) => abort_with_msg("panic inside __traverse__ handler"),
}
}
1 change: 1 addition & 0 deletions src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod freelist;
pub mod frompyobject;
pub mod ghost;
pub(crate) mod not_send;
pub mod panic;
#[doc(hidden)]
pub mod pyclass;
#[doc(hidden)]
Expand Down
27 changes: 27 additions & 0 deletions src/impl_/panic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/// Type which will panic if dropped.
///
/// If this is dropped during a panic, this will cause an abort.
///
/// Use this to avoid letting unwinds cross through the FFI boundary, which is UB.
pub(crate) struct PanicTrap {
msg: &'static str,
}

impl PanicTrap {
#[inline]
pub const fn new(msg: &'static str) -> Self {
Self { msg }
}

#[inline]
pub const fn disarm(self) {
std::mem::forget(self)
}
}

impl Drop for PanicTrap {
fn drop(&mut self) {
// Panic here will abort the process, assuming in an unwind.
panic!("{}", self.msg)
}
}
31 changes: 0 additions & 31 deletions src/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,34 +30,3 @@ impl PanicException {
}
}
}

/// Type which will abort the process if dropped.
///
/// Use this to avoid letting unwinds cross through the FFI boundary, which is UB.
pub(crate) struct PanicTrap {
msg: &'static str,
}

impl PanicTrap {
#[inline]
pub const fn new(msg: &'static str) -> Self {
Self { msg }
}

#[inline]
pub const fn disarm(self) {
std::mem::forget(self)
}
}

impl Drop for PanicTrap {
fn drop(&mut self) {
abort_with_msg(self.msg)
}
}

/// Attempts to write `msg` to stderr, then aborts.
pub(crate) fn abort_with_msg(msg: &str) -> ! {
let _ = std::panic::catch_unwind(|| eprintln!("FATAL: {}, aborting", msg));
std::process::abort()
}
3 changes: 2 additions & 1 deletion src/types/function.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::derive_utils::PyFunctionArguments;
use crate::exceptions::PyValueError;
use crate::panic::{PanicException, PanicTrap};
use crate::impl_::panic::PanicTrap;
use crate::panic::PanicException;
use crate::{
ffi,
impl_::pymethods::{self, PyMethodDef},
Expand Down

0 comments on commit e716388

Please sign in to comment.