Can caught panics be converted into Python exception?

[kngwyu]

Hi.
As a maintainer of PyO3 crate, which provides FFI interfaces for Python interpreter, I'm reviewing a PR that coverts Rust's panic to Python exception, using catch_unwind.
This PR is based on the issue opened by @nagisa.
PyO3 wraps user-defined Rust functions by unsafe extern "C" functions, and panics in those functions are currently not caught.
So PyO3-defined Python functions can cause unwinding over FFI, which is UB.
Thus, panics should be caught by catch_unwind.

But my question is, can we continue the program without aborting after panicking?
I'm afraid of this kind of scenario:

try:
    obj.pyo3_function()
except PyO3PanicException as e:
    obj.other_pyo3_function() # !

Suppose obj is a wrapper of a Rust struct and pyo3_function is a wrapper of Rust method.
If we catch panics and then return exceptions, users can catch it in Python codes and touch obj again!
So my question is

• Is it a UB to touch obj after panicking?
• If so, we should abort the program rather than raises exception?

Thanks.

[alice]

Why would it be UB to touch obj after a panic? That would be a special property of the object; it isn't true in general.

[Yandros]

Usage of catch_unwind cannot cause UB per se, as it is not marked unsafe. In other words, if some "Rust obj" can exhibit UB if someone were to catch a panic while manipulating it, then that object is the one unsound, not the call to catch_unwind.

That being said, UB is not the same as logic bugs. The most basic one is leaking memory, but one can more generally imagine a collection that in the middle of an operation reaches a temporary "empty collection state" before fixing that up, and if a panic occurs in between and is caught (so as to be able to access the collection afterwards), then the collection would effectively have accidentally been emptied.

• For "instance", in a multi-threaded scenario, poisoned locks were implemented to guard against this kind of accidents.

For these non-safety-critical "logc bugs", Rust defines the UnwindSafe (and RefUnwindSafe) marker traits, to help "linting" about this.

So, what you could do, to be "more correct" / less logic-error prone, is to never use the AssertUnwindSafe wrapper (to "dismiss" the lint), and instead, bound the given closures with UnwindSafe.

[kngwyu]

@alice @Yandros
Thanks guys.

bound the given closures with UnwindSafe.

Yeah, but maybe our scenario is even more unsafe than you think.
Our function wrapper is as follows:

unsafe extern "C" fn wrapped_method<T>(obj: *mut PyObject, args: *mut PyTuple) -> *mut PyObject {
    convert_result(std::panic::catch_unwind(|| {
        let obj_cell = PyCell::from(obj); // PyCell is our RefCell like wrapper
        T::method(obj_cell.borrow_mut(), covert_args(args))
    }))
}

In a wrapper function, we have to retrieve our RefCell like wrapper from a raw pointer to call user-defined method.
But UnwindSafe doc says

Types such as &mut T and &RefCell<T> are examples which are not unwind safe. The general idea is that any mutable state which can be shared across catch_unwind is not unwind safe by default. This is because it is very easy to witness a broken invariant outside of catch_unwind as the data is simply accessed as usual.

So it seems our usage of catch_unwind can break invariant.

[CAD97]

The key point here is that "unwind safe" is not safe as in "not unsafe". It is perfectly sound in every case to AssertUnwindSafe. There's vague intention of making this clearer but it's a tricky subject. My personal opinion is that Assert/UnwindSafe causes more issues than it prevents, so should probably be (soft) deprecated.

So it all comes down to the unwind safety of PyObj (which is almost certainly a requirement apart from this feature). Propogating the panic as a Python exception is probably the most correct semantic behavior, so your wrappers should work to support it properly.

[kngwyu]

@CAD97
Thank you.
I decided to support this feature.