Skip to content

Rc runs destructors twice on trait objects #25515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ftxqxd opened this issue May 17, 2015 · 9 comments
Closed

Rc runs destructors twice on trait objects #25515

ftxqxd opened this issue May 17, 2015 · 9 comments

Comments

@ftxqxd
Copy link
Contributor

ftxqxd commented May 17, 2015

use std::rc::Rc;

fn main() {
    struct Bass(u8);
    impl Drop for Bass {
        fn drop(&mut self) {
            println!("DROP THE BASS: {}", self.0);
            self.0 -= 1;
        }
    }
    let _: Rc<Send> = Rc::new(Bass(37));
}

prints:

DROP THE BASS: 37
DROP THE BASS: 36

(Playpen)

@michaelsproul
Copy link
Contributor

Doesn't occur in 1.0 stable thankfully...

@bluss
Copy link
Member

bluss commented May 17, 2015

I'll ask this just as a control question, but it's kind of important.

Is it sound that we allow simultaneous access to a value through both Rc<T> and Rc<Trait> variables? They can both be active at the same time.

@Thiez
Copy link
Contributor

Thiez commented May 17, 2015

If a 'normal' Rc is dropped last the problem goes away:

// This works:
let _: Rc<Send> = Rc::new(Bass(37)).clone();

// This also works:
let a: Rc<Bass> = Rc::new(Bass(37));
let _: Rc<Send> = a.clone();

// This triggers the bug:
let a: Rc<Bass> = Rc::new(Bass(37));
let b: Rc<Send> = a.clone();
drop(a);
drop(b);

@Thiez
Copy link
Contributor

Thiez commented May 17, 2015

The bug also occurs when you use an unsized type that isn't a trait:

// Same bug.
let _: Rc<[Bass]> = Rc::new([Bass(37)]);

@Thiez
Copy link
Contributor

Thiez commented May 17, 2015

It looks like the problem is with drop_in_place? The following program exhibits the same problems as Rc:

#![feature(core)]
struct Foo(u8);
impl Drop for Foo {
    fn drop(&mut self) { println!("Dropped Foo({})", self.0); self.0 += 1; }
}
struct Holder<T: ?Sized>(T);
fn main() {
    unsafe {
        let ptr = std::mem::transmute::<_, *mut Holder<[Foo; 1]>>(Box::new(Holder([Foo(0)])));
        let big_ptr: *mut Holder<[Foo]> = ptr;
        std::intrinsics::drop_in_place(&mut (*big_ptr).0 as *mut _);
        // leak box memory here :(
    }
}

The same thing happens when we use the following unsafe block in main:

    let ptr = std::mem::transmute::<_, *mut Holder<Foo>>(Box::new(Holder(Foo(0))));
    let big_ptr: *mut Holder<Send> = ptr;
    std::intrinsics::drop_in_place(&mut (*big_ptr).0 as *mut _);

@pnkfelix
Copy link
Member

cc me

@Thiez
Copy link
Contributor

Thiez commented May 17, 2015

This bug is hilarious:

#![feature(core)]
struct Foo(u8);
impl Drop for Foo {
    fn drop(&mut self) { println!("Dropped Foo({})", self.0); self.0 += 1; }
}
struct Holder<T: ?Sized>(T);
fn main() {
    unsafe {
        let mut foo = Holder(Foo(0));
        &(*(&mut foo as &mut Holder<Send>)).0;
        &(*(&mut foo as &mut Holder<Send>)).0;
        &(*(&mut foo as &mut Holder<Send>)).0;
        std::mem::forget(foo);
    }
}

This prints:

Dropped Foo(0)
Dropped Foo(1)
Dropped Foo(2)

Yes, we dropped the same Foo three times. The problem is not in drop_in_place, the problem is that somehow Foo gets dropped when we take a reference to it through some other object.

Edit: so the bug appeared to be in drop_in_place because there we took a reference to the value in the RcBox and triggered the actual bug.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented May 18, 2015

Looks like this also applies to Arc (since e840393).

@ftxqxd
Copy link
Contributor Author

ftxqxd commented May 18, 2015

@bluss

Is it sound that we allow simultaneous access to a value through both Rc<T> and Rc<Trait> variables?

I believe so, since it’s always been possible to do so with &T and &Trait, and Rc isn’t really any different, as far as I know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants