Panics in destructors can cause the tail expression of a block to be leaked

[arielb1]

View all comments

STR

struct NoisyDrop;
impl Drop for NoisyDrop {
    fn drop(&mut self) {
        println!("dropping a NoisyDrop");
    }
}

impl NoisyDrop {
    fn new() -> Self {
        println!("creating a NoisyDrop");
        NoisyDrop
    }
}

struct PanickyDrop;
impl Drop for PanickyDrop {
    fn drop(&mut self) {
        panic!()
    }
}

fn foo() -> NoisyDrop {
    let p = PanickyDrop;
    NoisyDrop::new()
}

fn main() {
    foo();
}

Expected Result

"creating a NoisyDrop" and "dropping a NoisyDrop" should appear the same number of times

Actual Result

creating a NoisyDrop
thread 'main' panicked at 'explicit panic', src/main.rs:18:8
note: Run with `RUST_BACKTRACE=1` for a backtrace.

The destructor is ignored.

[jonas-schievink]

Heh, this even happens on Rust 1.0

[arielb1]

@jonas-schievink

Rust 1.0 is far worse than this - it leaks everything when a destructor panics.

[solb]

@arielb1, could you explain why this is tagged as a soundness bug? The language makes no guarantee that destructors will ever be invoked.

[gnzlbg]

ping @rust-lang/compiler this should be triagged, even if it is not a soundness bug, unwinding working properly is something that a lot of code relies on.

[nikomatsakis]

So, to be clear, it's not that we leak all destructors when panic in drop occurs. For example, this function drops both p and n as expected:

fn foo() {
    let n = NoisyDrop::new();
    let p = PanickyDrop;
}

The specific scenario here is that:

• we are in the middle of returning the NoisyDrop to our caller (so our caller "owns it")
• but we panic before our return completes (so they don't realize they own it)

I don't think it's quite right to say that "we make no guarantee that destructors will ever be invoked". We don't guarantee that all destructors are always invoked, but we do guarantee some things: for example, that if you have a local variable whose scope includes a call, it will be dropped when the call panics (here, n would be dropped):

let n = ...;
foo();

So the question is what we should guarantee in this particular scenario. Perhaps the answer is that we should just be very clear to document the current behavior, given how long it has existed.