rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.21k stars 12.81k forks source link

Vec dtor might want to try harder on failure #16135

Closed brson closed 7 years ago

brson commented 10 years ago

Although dtors should not fail (by convention), it can happen. The Vec dtor doesn't do anything to defend against one of its elements' failing.

Since there's no winning when dtors fail, it's not obvious that Vec should try to do anything, but for comparison, the I/O process and stream types both take defensive measures to try to make the best of similar situations.

This program leaks 4 boxes, the buffer, and fails to run 3 dtors:

struct F(Box<()>);

impl Drop for F {
    fn drop(&mut self) {
        println!("drop");
        fail!()
    }
}

fn main() {
    let _v = vec!(F(box () ()), F(box () ()), F(box () ()), F(box () ()));
}
alexcrichton commented 10 years ago

This is... disturbing. We also discovered today that Rc and Arc leak their allocations on failure of the contained object's destructor.

In general it sounds like our "clean up on destructor failure" story isn't so hot when considering issues like #14875 as well.

thestinger commented 10 years ago

I don't think it makes sense to take a further performance and complexity hit to improve (but not fix) support for something that's already broken.

steveklabnik commented 9 years ago

Triage: this sitll only prints drop once, with some trivial updates to today's syntax.

steveklabnik commented 8 years ago

Triage:

#![feature(box_syntax)]
struct F(Box<()>);

impl Drop for F {
    fn drop(&mut self) {
        println!("drop");
        panic!()
    }
}

fn main() {
    let _v = vec!(F(box ()), F(box ()), F(box ()), F(box ()));
}
steve@warmachine:~/tmp$ ./main 
drop
thread '<main>' panicked at 'explicit panic', main.rs:7
note: Run with `RUST_BACKTRACE=1` for a backtrace.
bluss commented 8 years ago

Since we did the Vec / RawVec split, it should now be the case that the RawVec allocation itself is always free'd even if one of the elements panic in Vec's drop.

Confirmed that it does deallocate the RawVec allocation using rustc 1.9.0-nightly (7b0b80ae2 2016-03-02).

Mark-Simulacrum commented 7 years ago

I believe we handle this correctly today, in that we attempt to deallocate all elements regardless of failure:

struct F(u32);

impl Drop for F {
    fn drop(&mut self) {
        eprintln!("drop");
        if self.0 == 0 { panic!() }
    }
}

fn main() {
    let _v = vec!(F(1), F(0), F(1), F(1));
}
   Compiling playground v0.0.1 (file:///playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.66 secs
     Running `target/debug/playground`
drop
drop
thread 'main' panicked at 'explicit panic', src/main.rs:7:25
note: Run with `RUST_BACKTRACE=1` for a backtrace.
drop
drop