Experts catching panics wrong?

If you're writing a library, and you're relying on catch_unwind for soundness (for example, any FFI wrapper that accepts a Rust function as a callback called from C must, since panics must not cross FFI boundaries), then your library is unsound if you don't handle this because your library being sound means that it can't cause UB no matter what any of the code interacting with it does.

(And, a more squishy-philosophy-of-software-engineering matter: programs get extremely complex and it really matters that we arrange that things are sound in 100% of cases rather than 99.9% of cases, because when you multiply together lots of those almost-perfect successes you get an uncomfortable failure rate. (0.999).powi(100) ≈ 0.90.)

And, if you look at the last few comments on "Never allow unwinding from Drop impls", they're from people who are concerned about the proposed change because they want things that panic on drop.


Yes, I see some kind of a debate there about the usefulness of panic on drop. I don't want to minimise the issues, but on the other hand I do think the problems could be described as "somewhat theoretical" ( but nevertheless deserving of attention ). I cannot imagine my code panicking in drop ( I only ever implemented drop once, and the drop in question doesn't even occur during a caught panic ).

Of course, others may have other experiences, I would be interested to hear about them.

As an example of experts catching panics wrong, Tokio has some code where it catches panics, but I'm pretty sure we don't catch panics from the destructor of the panic payload of a previously-caught panic.

It wouldn't cause memory unsafety if it happened, but you would probably lose one of your worker threads.


Are you saying there was a bug in the tokio code? What was the mistake?

If a Tokio task panics with a payload that panics again when dropped, then you will lose the worker thread it happened, is my guess based on how I recall the code was written.


Ah ok. I wouldn't call that a bug, it seems to me that a panic during an unwind is always going to be problematic and should be avoided ( if you want the panic to be caught ). Maybe it's possibly to deal with the situation, but I wouldn't want to depend on that.

But if it was in a situation where it caused UB, then that would certainly be a bug. Safe code causing memory errors/other UB is supposed to be impossible in safe Rust; any code that doesn't uphold that guarantee is considered unsound (i.e., buggy).

1 Like

From the thread I also linked to, there's the issues @kpreid pointed out (bugs in rustc/stdlib -- experts getting it wrong), followed by some conversations around the RUDRA paper.

To quote the Lang team lead in #97,

Code using catch_unwind is not typically prepared to handle an object that panics in its Drop impl. Even the standard library has had various bugs in this regard, and if the standard library doesn't consistently get it right, we can hardly expect others to do so.

That quote is in the context of removing that particular footgun, but there is no consensus yet and the footgun still exists.

Those are the citations I have on hand, but I've seen other examples via TWIR, changesets, blog posts, and the like.

I want to highlight this as well. Protecting an FFI boundary from unwind UB is one of the niche circumstances where catching panics is not only applicable, but necessary. And because it's a soundness issue, "eh, esoteric circumstances" doesn't cut it in a language like Rust, where safety is a core tenant.

It can be a struggle to recognize when coming from C/C++ say, where a lot of UB is "turned off" with compiler flags or just culturally ignored.

I do recognize that if there's not a risk of UB, it may not be worth one's effort, and slapping a good-enough sticker on it is a viable approach. However, catch_unwind does not protect you from unexpectedly broken logical invariants (as panics break the library API), so trying to recover can still be risky (prone to bugs).


In addition to the panic-on-drop stuff already discussed, writing unsafe code which can correctly handle the possibility of a panic is hard.

Any time you slightly bend the rules (e.g. I'm reallocating a buffer and temporarily have a dangling pointer which my Drop impl might free) you need to worry about how an untimely panic might let your users observe this transient state. This gets even harder when you realise that any functions provided by the user (e.g. via methods on a trait) may panic.

There used to be a problem with Vec's Drain iterator where forgetting the Drain would mean we never shift the items at the end down over the items that were already removed, letting you read uninitialised memory.

In practice you see the Pre-Pooping Your Pants pattern being used a lot, where you put the original object in a safe-but-neutered state and move the unsafe logic into some drop guard which will restore correctness when it is dropped.


Yes, it is necessary to make sure any persistent data structures (that is data structures that still exist after the unwind) are in a consistent state (rolled back) after an error, but I don't see that the use of catch_unwind rather than Result makes any difference there, it's still necessary either way. Maybe I am wrong about this though.

Can you please tell me how I can do that with "cargo test"
I don't want to set it in my Cargo.toml

panic = 'abort'

I would like to use my "cargo test" command hopefully with flags to check my test function that catches a panic to prove it fails when panics abort.
Is there a way around the

warning: `panic` setting is ignored for `test` profile

Can test profiles be run with panic=abort?

Baby steps toward expert.

In Android, we compile tests with panic = abort, but we use our own build system rather than cargo. I don't know if cargo has an option for it.

I guess I just needed to ask. Usually as soon as I ask someone a question the answer comes to me.

RUSTFLAGS='-C panic=abort -Zpanic_abort_tests' cargo +nightly test

Will run tests with panic=about.

Baby steps toward being an expert....

1 Like

Now I need to figure out how to check if panic=abort is set for the compile. Maybe a cfg flag test of some sort is available. I don't want to include the catch code if catching will not work.

It's unstable, unfortunately:

Thanks, I will look. Unstable is good enough for testing.

Thinks to self before looking at link ...
"so then because cfg_panic is 'unstable' I probably need to set a cfg test to check if cfg_panic is enabled before I use the cfg_panic config test in my code."

Is this a fair summary:

(1) Writing unsafe code that allows for panics is difficult ( or at least it is easy to overlook that a panic might leave memory in an unsafe state ), and experts have made mistakes in this area. Panic safety is one of three mistakes that the Rudra project found lead to memory bugs in unsafe Rust.
( Static Analyzer Rudra Found over 200 Memory Safety Issues in Rust Crates )

(2) Catching panics is not difficult, although you need to make sure any state is restored to correct values, and avoid using panic in any drops that could occur during unwinding. Typically this will mean discarding any changes that have been made, making sure any cached data is correct. However this needs to be done with conventional Result-based error handling as well, if program execution is not to be aborted.

I don't really see where this is going? The whole point of Result-based error handling is that you don't need to perform additional manual cleanup, since Result is just a value, ? is equivalent with a regular return, and RAII solves the rest.

If an error occurs, then typically you will want to undo any changes that were made prior to the error ( or perhaps more typically stop them becoming permanent ). This needs to be done regardless of the error-handling scheme involved ( as far as I can see ). Of course, it all depends on the general context, what is going on. I am thinking of a database transaction type scenario, where some changes to data are prepared ( but not committed to permanent storage), an error occurs, and the transaction is therefore "rolled back".

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.