Use of GC inside destructors aborts when destructor called from GC

[bblum]

Updated example

use std::task;
use std::gc::{GC, Gc};
use std::cell::RefCell;

struct Foo;
impl Drop for Foo {
    fn drop(&mut self) {
        allocate();
    }
}

fn allocate() {
    struct A {
        inner: RefCell<Option<Gc<A>>>,
        other: Foo,
    }
    let a = box(GC) A {
        inner: RefCell::new(None),
        other: Foo,
    };
    *a.inner.borrow_mut() = Some(a.clone());
}

fn main() {
    allocate();
}

failed at 'assertion failed: self.live_allocs.is_null()', /home/rustbuild/src/rust-buildbot/slave/nightly-linux/build/src/librustrt/local_heap.rs:173
run with `RUST_BACKTRACE=1` to see a backtrace

You've met with a terrible fate, haven't you?

fatal runtime error: Could not unwind stack, error = 5
zsh: illegal hardware instruction (core dumped)  ./foo

Original description

This program crashes:

struct Foo { x: int } // surprisingly, doesn't crash with an empty struct

impl Drop for Foo {
    fn finalize(&self) {
        let _x = @Foo { x: self.x + 1 };
    }
}

fn main() { let _x = @Foo { x: 0 }; }

With the current implementation, the crash message is:

rust: task f46140 ran out of stack
rust: domain main @0xf44a40 root task failed
rust: task f46140 ran out of stack during unwinding
terminate called after throwing an instance of 'rust_task*'
Aborted

But when we switch to tracing GC for @-pointers, I predict a new problem: the destructor won't run until an attempt to allocate another @-pointer fails, at which point @-pointers could no longer be allocatable... and the allocation would fail (even though the amount of live memory in use might be reasonably small). This raises some questions:

• Should we ban creating new @-pointers in destructors, or does "disable recursive GC" during GC" suffice?
• Depending on the allocator design, would that also make it unsafe to allocate ~-pointers in destructors?

Related #910.

[bblum]

nominating for well-defined milestone

[thestinger]

This is still an issue.

[catamorphism]

Accepted for milestone 1, well-defined

[emberian]

Visiting for triage. Re-nominating. I think this should just be a "don't do this" in Gc<T>, and notabug.

[nikomatsakis]

cc me

[pnkfelix]

there are a number of ways to handle this, e.g. not running user-defined destructors from within the GC coroutine (and instead deferring them to e.g. a guardian, or a concurrent finalization tasks).

Not closing as not-a-bug, at least not yet, I want more time to investigate the design here.

[pcwalton]

Nominating for closing, as this is a GC issue and GC is not in scope for 1.0. The corresponding TLS issue, #14807, is more important.

[pnkfelix]

Do we not allow for open bugs on feature-gated things?

(Having said that, I would not object to taking this off the milestone, and in fact I will nominate it for that right now.)

[thestinger]

I think it would be reasonable to simply migrate the users of Gc<T> to Rc<T> and remove the stub until it actually provides tracing garbage collection. The Rust compiler itself has no use case for tracing garbage collection because it has serious memory consumption issues and Valgrind will report any Rc cycles as errors on the buildbots.

[alexcrichton]

Updated comment/title to be relevant for today.

[pcwalton]

Sorry, I meant nominating for removal from P-backcompat-lang.

[brson]

P-high, not 1.0

[ghost]

Gc is gone so closing.