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

Box-enclosed destructors can't safely access other boxes. #3039

Closed
bblum opened this issue Jul 26, 2012 · 12 comments
Closed

Box-enclosed destructors can't safely access other boxes. #3039

bblum opened this issue Jul 26, 2012 · 12 comments
Labels
A-typesystem Area: The type system I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Milestone

Comments

@bblum
Copy link
Contributor

bblum commented Jul 26, 2012

Ran into this issue when trying to write a destructor for the doubly-linked-list. The following code segfaults after (i believe) one of the boxes has been annihilated and the other's destructor attempts to run.

The solution, I think, is for the box annihilator to run in two phases - one phase to run destructors, and then another to actually annihilate the boxes after that. (EDIT: this is a point of contention.)

class ticky_tacky {
    let data: uint;
    let mut neighbour: @little_box;
    new(data: uint) { self.data = data; self.neighbour = @none; }
    drop {
        alt *self.neighbour {
            some(other_box) {
                #error["Neighbour's value is %u", other_box.data];
            }
            none { }
        }
    }
}

enum little_box { none, some(ticky_tacky) }

fn main() {
    let box1: @little_box = @some(ticky_tacky(42));
    let box2: @little_box = @some(ticky_tacky(31337));

    alt check *box1 {
        some(x) { x.neighbour = box2; }
    }
    alt check *box2 {
        some(x) { x.neighbour = box1; }
    }
    fail;
}
bblum added a commit that referenced this issue Jul 26, 2012
@bblum
Copy link
Contributor Author

bblum commented Jul 26, 2012

Related: #2047 #910

@graydon
Copy link
Contributor

graydon commented Jul 26, 2012

No, this should not compile and it's not related to those bugs. Values with dtors should not have @ boxes in them, statically. They're supposed to be acyclic, precisely so they can run top-down. This is why resources were originally their own kind. Kind system bug.

@graydon
Copy link
Contributor

graydon commented Jul 26, 2012

Sorry, that sounds yell-y, I don't mean to be unduly harsh; just want to be completely clear that "running dtors in multiple passes" is how finalizers work, not dtors, and we've made a very explicit effort to avoid that up until now. If it's not working, that's a bug, not a "let's switch to finalizers" opportunity.

@bblum
Copy link
Contributor Author

bblum commented Jul 26, 2012

No problem. I'm trying to come up with a way that your proposal isn't quite complete, involving allocating new @-boxes inside of destructors, but I think it's solid.

I came up with crashing code that respects your proposed kind restriction, but it just ended up being another demonstration of why TLS deserves to be unsafe. (note that this code follows the "tls is supposedly safe if you don't use polymorphic key functions" rule.)

fn key(+_x: @ticky_tacky) { }

class ticky_tacky {
    let data: uint;
    new(data: uint) { self.data = data; }
    drop unsafe {
        do task::local_data_modify(key) |box| {
            alt box {
                some(x) { #error["My own data: %u", x.data]; }
                none    { fail ~"????" }
            }
            box
        }
    }
}

fn main() {
    unsafe { task::local_data_set(key, @ticky_tacky(42)); }
    fail;
}

@bblum
Copy link
Contributor Author

bblum commented Jul 27, 2012

However, if we forbid destructors in things that contain @-boxes, then the doubly-linked-list cannot possibly have its own auto-clean-up function. If a dlist goes out of scope with elements still inside, they'll stick around until the cycle collector runs (or until we add true GC to the language...).

There's already a dlist::clear() function for people to use to avoid this, but it'd be nicer if it could clear itself automatically.

@bblum
Copy link
Contributor Author

bblum commented Jul 27, 2012

OK, I think your proposal is right. I was able to break it with three canonical "funny businesses" - global state (TLS, above), captured closure environments, and existentials, and all three of them can be ruled out with the "no box" kind.

Crashing code with a @-closure: http://pastebin.mozilla.org/1716353
And crashing code with an existential: http://pastebin.mozilla.org/1716358

One question still: Will the "no-box" kind be equivalent to "send", or will it be able to permit region pointers too?

@bblum
Copy link
Contributor Author

bblum commented Jul 27, 2012

these crashes still happen when the 'fail' statement is removed, which convinces me that the box annihilator is orthogonal to the problem.

@catamorphism
Copy link
Contributor

I'm not totally convinced it's no longer possible to write this program, but
I tried and got stuck because I couldn't match on the @-boxes with a ref mut
pattern. So I'm going to assume this is just not allowed anymore. If you can
port the example to trunk and get the same behavior, by all means reopen!

@bblum
Copy link
Contributor Author

bblum commented Mar 23, 2013

struct TickyTacky {
    data: uint,
    neighbour: @mut Option<TickyTacky>,
}

impl Drop for TickyTacky {
    fn finalize(&self) {
        if (self.neighbour.is_some()) {
                let other_box = option::swap_unwrap(self.neighbour);
                fail_unless!(other_box.data == 42 || other_box.data == 31337);
        }
    }
}

fn main() {
    let box1 = @mut Some(TickyTacky { data: 42, neighbour: @mut None });
    let box2 = @mut Some(TickyTacky { data: 31337, neighbour: box1 });

    *box1 = Some(TickyTacky { data: 42, neighbour: box2 });
}

@bblum bblum reopened this Mar 23, 2013
@bblum
Copy link
Contributor Author

bblum commented Mar 23, 2013

Oddly, if I switch the fail_unless line in the above code for error!("Neighbour's value is %u", data);, it does not segfault. I cannot figure out why. fail_unless is not itself tripping.

@bblum
Copy link
Contributor Author

bblum commented Mar 23, 2013

I looked into the mystery a bit more, and it turned into more of a mystery, enough that I'm not confident that my above code is a reproduction of the original bug here. But I note that in the other bug, the place where it segfaulted changed (here, it segfaults in glue_drop_2475).

@thestinger
Copy link
Contributor

Is this fully fixed by d4fee24?

RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 31, 2023
tests/catch_panic: make output easier to interpret
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
## What's Changed
* Upgrade toolchain to 2024-02-14 by @zhassan-aws in
model-checking/kani#3036

**Full Changelog**:
model-checking/kani@kani-0.46.0...kani-0.47.0

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

No branches or pull requests

4 participants