How deep do exception safety guarantees need to go in FFI/unsafe code?

[Michael-F-Bryan]

If I create an extern "C" function which will call a method on some other type and that function triggers a panic, is it enough to just wrap the call with std::panic::catch_unwind() and translate it to an error code?

My thinking is that the object could be messed up internally and I should use poisoning to make sure it can never be accessed again, which was my solution in Michael-F-Bryan/thin-trait-objects#2.

But all unsafe code should already ensure it won't trigger UB in the face of panics so exception safety is already guaranteed and further poisoning is overkill... isn't it? Or should unsafe code be overly conservative and try to protect users from implementations which are accidentally not Maximally Exception Safe, to use the terminology from the nomicon?

I feel like my options are to,

1. Add a W: UnwindSafe bound to FileHandle::for_writer() (which isn't overly ergonomic because lots of types are !UnwindSafe) and just wrap panicking calls in std::panic::catch_unwind(), allowing people to have logic errors or bugs when they try to use the object later on
2. Skip the UnwindSafe bound and neuter/poison the FileHandle the moment a panic occurs

(original question was asked in Michael-F-Bryan/thin-trait-objects#1)

[thomcc]

It's worth noting that there's an ongoing discussion on whether or not poisoning was a good idea, and whether or not to deprecate UnwindSafe/RefUnwindSafe. See https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/lock.20poisoning.20survey

It's also worth noting that UnwindSafe has nothing to do with actual safety (AssertUnwindSafe is entirely safe), so these are really choices that matter exclusively to whether or not you like the design of the API. I don't really feel like it's entirely on-topic for the UCG, but that's just my feeling — perhaps good discussion can still be had.

[Michael-F-Bryan]

It's also worth noting that UnwindSafe has nothing to do with actual safety ... so these are really choices that matter exclusively to whether or not you like the design of the API

Thanks @thomcc. My interpretation was that UnwindSafe and friends are used as a speed bump to remind people about maintaining Maximal Exception Safety (i.e. that panics shouldn't be able to trigger logic bugs later on).

I guess if we're questioning the usefulness of UnwindSafe and friends it is every type's responsibility to ensure Minimal Exception Safety (panics can't later trigger UB), and UnwindSafe is irrelevant to unsafe code... That would mean I can write unsafe code with the assumption that anything used internally will already be exception safe and wrapping FFI functions with a std::panic::catch_unwind() is more than sufficient.

[RalfJung]

To be clear, this question is not directly related to thin trait objects, is it?

Whenever Rust code is called from C code, you need to ensure that no panics bubble up into the C code. Eventually, hopefully, rustc will implicit add code which catches panics and turns them into aborts -- see rust-lang/rust#52652. Currently that's blocked on some code requiring such unwinding, and the FFI-unwind working group is figuring out ways to put that on solid footing. So until rustc does this automatically, what you can do is to simply catch unwinding and turn it into aborts yourself.

[Michael-F-Bryan]

To be clear, this question is not directly related to thin trait objects, is it?

This question is more about exception safety and how far unsafe code needs to go to deal with unexpected panics and make sure broken invariants are unobservable.

As an example which isn't conflated with FFI and unwinding into C being UB, let's think of how you might implement a runtime for async code... In an async runtime you are writing a large amount of unsafe code (Pin, Waker, etc.) which calls into user-provided code (Future::poll()). For practical reasons we don't want to abort the entire process if one particular Future were to panic, so we need to catch the panic and figure out what to do next.

You might take different courses of action depending on how much responsibility your unsafe code has in maintaining exception safety:

1. Declare the Future as corrupted and make sure it can't be accessed again (e.g. via std::mem::forget() or poisoning), making sure that any possibly broken invariants can never be observed
2. Declare the Future as corrupted, never poll it again, but still run the destructors
3. Do the bare minimum so that the runtime's internals are still consistent and let your Future be polled again at a later time, even though it may be broken and encounter logic/memory bugs

Does unsafe code have a responsibility to maintain the validity of other components in the face of panics/exceptions? And if so, to what extent?

[RalfJung]

Does unsafe code have a responsibility to maintain the validity of other components in the face of panics/exceptions? And if so, to what extent?

I'm afraid that question is too abstract, I do not understand exactly what you mean. Some concrete pieces of code would really help make this clear. :) (Ideally without pulling in an entire async stack... the smaller the example, the better.)

In general, unsafe codes has the responsibility of not breaking other people's invariants. On the other hand, panics are omnipresent in Rust, so these other people already have to be aware that there could be panics, and have to behave in some safe way.