Skip to content
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

Passing an rvalue as ~self causes double-free #4355

Closed
thestinger opened this issue Jan 5, 2013 · 6 comments · Fixed by #7452
Closed

Passing an rvalue as ~self causes double-free #4355

thestinger opened this issue Jan 5, 2013 · 6 comments · Fixed by #7452
Labels
A-codegen Area: Code generation A-destructors Area: Destructors (`Drop`, …) A-type-system Area: Type system I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@thestinger
Copy link
Contributor

This somehow causes a destructor to be called twice:

struct Foo {
    data: ~[int]
}

impl Foo {
    fn bar(~self) -> ~Foo { self }
}

fn main() {
    (~Foo{data: ~[1,2,3]}).bar();
}
@bblum
Copy link
Contributor

bblum commented Feb 22, 2013

6644da5 also points to this, which I ran into independently, though that problem might be something different? Looks like using v.consume() or v.partition() (possibly among others) always crash, and consume(v) and partition(v) work fine.

@nikomatsakis
Copy link
Contributor

Not critical for 0.6; removing milestone

@erickt
Copy link
Contributor

erickt commented Apr 14, 2013

Here's an even simpler version of this bug:

trait A {
    fn a(self);
}

impl A for ~int {
    fn a(self) { }
}

fn main() {
    (~1).a();
}

However, if we use ~self it works fine:

trait A {
    fn a(~self);
}

impl A for int {
    fn a(~self) { }
}

fn main() {
    (~1).a();
}

@Blei
Copy link
Contributor

Blei commented Jun 15, 2013

This seems to be fixed on master.

@Blei
Copy link
Contributor

Blei commented Jun 15, 2013

Correction: the "simpler version" now runs correctly, the original code still segfaults.

@bblum
Copy link
Contributor

bblum commented Jun 28, 2013

I believe the problem is only for rvalues.

use std::util;

struct Foo;

impl Foo {
    fn eat(~self) { util::ignore(self); }
}

fn main() {
    (~Foo).eat();
}

This dies, but works fine if you change it to:

let x = ~Foo;
x.eat();

bblum referenced this issue in brson/rust Jun 28, 2013
This is supposed to be an efficient way to link the lifetimes
of tasks into a tree. JoinLatches form a tree and when `release`
is called they wait on children then signal the parent.

This structure creates zombie tasks which currently keep the entire
task allocated. Zombie tasks are supposed to be tombstoned but that
code does not work correctly.
bors added a commit that referenced this issue Jun 29, 2013
Currently we pass all "self" arguments by reference, for the pointer
variants this means that we end up with double indirection which causes
a unnecessary performance hit.

The fix itself is pretty straight-forward and just means that "self"
needs to be handled like any other argument, except for by-value "self"
which still needs to be passed by reference. This is because
non-pointer types can't just be stuffed into the environment slot which
is used to pass "self".

What made things tricky is that there was also a bug in the typechecker
where the method map entries are created. For type impls, that stored
the base type instead of the actual self-type in the method map, e.g.
Foo instead of &Foo for &self. That worked with pass-by-reference, but
fails with pass-by-value which needs the real type.

Code that makes use of methods seems to be about 10% faster with this
change. Also, build times are reduced by about 4%.

Fixes #4355, #4402, #5280, #4406 and #7285
@bors bors closed this as completed in 765a290 Jun 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-destructors Area: Destructors (`Drop`, …) A-type-system Area: Type system I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants