# Design meeting 2021-11-24 - panic in drop

https://gist.github.com/Amanieu/e30b9d6ad36b011125d8d3a01e0e622b

## Questions

- `drain_filter` is given as an example of a problematic API, but is unstable, so could still be removed. Are there any known examples of stable ones, or is that basically the only one?

- No stable APIs are affected.
- `drain_filter` stabilization is blocked on figuring out what to do when the iterator is dropped.

- Clarifying: This is about making panics that *escape* `Drop` abort, right? One could still call something that panics from inside `drop`, and use `catch_unwind` in the drop impl?

- Yes.
- But be careful of double-panics which use a thread-local counter.
- I was pretty sure, but the doc starts "Panics which happen inside a Drop impl"

# Q

* What alternatives do users have if they "need" the current behavior?
* Or, are there no known cases of actual reliance on panic! catching (i.e., where the current behavior is actually interesting)?
* "scope guard" style APIs, maybe?
* Note that drain_filter is not really this: panics there generally still end the relevant code, I suspect.
* (closed) RFC for a "close trait" https://github.com/rust-lang/rfcs/pull/2677
* conversation discusses calling it `TryDrop` or similar.

- Do we anticipate changing the default in a future edition?
- Change the default now without an edition change.
- All crates must use the same `-Z panic-in-drop` settings.
- Do we anticipate eliminating the option in a future edition?

Amanieu: I think this is unlikely to happen in practice. Someone has to be panicking in a drop (which is already very rare) and then relying on the behavior of unwinding from that panic.

Mark: It seems like many drop impls *could* panic, they just don't. (For instance, a call to `unwrap`.)

Amanieu: Even if you make your code safe against panic in drop, you will still leak memory.

ScottMcM: The usual web server example is the place I might disagree with that --- it may want to stop accepting new requests but allow its currently-running ones to complete before restarting the process instead of aborting the process immediately and having those in-flight requests just die.

- Could we still call the panic hook? (Could we notify it that this will abort due to panic in drop?)

Amanieu: Yes, we call the panic hook. But then we eventually abort before we escape the drop.

- How much of the advantage could we get here from scoping how much we do this? The container versions of this feels like definitely worth doing; in general it's less obvious to me.

(try below means C++‍-style exception handling, not a `try` block for `?`)

impl Drop {
    fn drop(&mut self) {
        try {
            ...
        } catch {
            abort()
        }
    }
}

^ This is the same mechanism as panics in `extern "C" fn`.

Escape hatch: use the rustc option to re-enable panic in drop.
Problem: you'd have to rebuild the standard library.

Could we have a hook that happens right before the `abort`?
Amanieu: C++ does this; it doesn't abort immediately, it calls `terminate` which you can hook.

panic_aborting hook, taking an opaque PanicAbortingInfo that may be interrogatable?
Possibly do this for unwind through `extern "C"` as well?

When can (and should) crates remove support for handling panic-in-drop reasonably?

## Offline comment from nikomatsakis
The Rudra project -- great paper! -- pointed out that panic safety is a particular source of problems in unsafe code:

Due to their subtlety, panic safety violations have caused several memory safety bugs in popular Rust packages [1, 4, 5] and the Rust standard library [7, 10, 12]

My experience with Rayon was that while it was plausible for me to audit the sources of panics from explicit closure calls or generic operations, identifying panics that might result from drops was beyond my capabilities. I wound up just adding a "abort on panic" guard. It's possible that a lint against "possible panics" would have helped, but I think that in general such a lint + disallowing panic in drop would help even more.