Rename unsafe_ignore_trace and unsafe_empty_trace

[mystor]

Omitting fields from all of the trace methods is actually safe, due to the design of this garbage collector. It just means that any Gc<T>s which are hidden by these impls will not ever be unrooted, and thus will not be collected if they participate in a cycle.

As leaking is safe, these should be renamed from unsafe_ignore_trace to just ignore_trace and unsafe_empty_trace to just empty_trace.

[andersk]

One can cause the collector to panic with inappropriate use of #[unsafe_ignore_trace]. Although panicking is safe, it’s probably not what the developer expects. So perhaps it’s for the best that the name inspires some level of caution.

use gc::{Finalize, Gc, Trace};

#[derive(Finalize, Trace)]
struct S(#[unsafe_ignore_trace] Gc<()>);

fn main() {
    Gc::new(S(Gc::new(())));
}

thread 'main' panicked at 'assertion failed: finalizer_safe()', /home/anders/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gc-0.4.1/src/lib.rs:125:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
Aborted (core dumped)

[Ekleog]

I'm trying to understand what the panic message from #52 (comment) is about. It seems to me like the two Gc here should simply be independent?

In other words, I'm thinking that having #[unsafe_ignore_trace] should raise issues only when the field with this tag actually contains references that cause a circular dependency with the outside Gc. Because so long as the inner and outer Gc live in different worlds, I don't see how that could raise problems.

I guess, based on the error message the issue somehow stems from the fact that Gc needs to Finalize and not Drop? But I must say I'm not sure I understand what exactly Finalize is used for, so I don't get the issue.

In other words, I currently think this panic is a bug in Gc. But is it?

(For some context, I'm coming from my first blog post and this is something I would not be able to guarantee, if it must assert that no Gc ever are under this field, as opposed to no Gc under this field ever looping with Gcs outside — and no one using a library could either, as the library could start using gc internally in later minor versions. BTW if you have thoughts about the post I'm willing to hear them :))

[andersk]

@Ekleog The inner Gc is created in the rooted state. It would ordinarily be unrooted upon being moved into the outer Gc; however, that’s skipped by #[unsafe_ignore_trace]. So the inner Gc is still rooted until the outer Gc is collected, at which time the destructor of the inner Gc unroots itself, which in particular involves dereferencing itself. It’s generally not safe to dereference a Gc at this stage during collection, because some Gc pointers might be dangling if we’re in the middle of collecting a cycle. The assertion prevents this unsafety and panics instead.

In the particular case of the destructor of a rooted Gc, there may be an argument that, even if we’re in the middle of collection, this allocation must have been marked live, so it should be safe to skip the assertion and dereference it anyway. I’m not entirely confident that this argument is correct (especially in the presence of a known bug #124), but if it is, perhaps we could indeed arrange for ignore_trace to have no adverse consequences beyond potential leaks.

[Ekleog]

Hmm… do you know of a place where I could read about what Finalize is supposed to do? I guess I need to read some more than the one-line doc-comment of Finalize and Trace in order to understand why this assertion is necessary for finalizing unrooted pointers :)