Context slots are never dropped if there are Globals to the context when the isolate is dropped

[andreubotella]

Context slots, which were added in #937, let you store data associated to a context, which will be dropped when the context is GC'd. This would imply that context slots would at the latest be dropped when the corresponding isolate is dropped, but that is not the case when there are remaining global references to the context:

let _global_to_context: v8::Global<v8::Context>;
let weak_to_slot: std::rc::Weak<()>;

{
    let platform = v8::new_default_platform(0, false).make_shared();
    v8::V8::initialize_platform(platform);
    v8::V8::initialize();

    let isolate = &mut v8::Isolate::new(Default::default());
    let scope = &mut v8::HandleScope::new(isolate);
    let context = v8::Context::new(scope);
    _global_to_context = v8::Global::new(scope, &context);

    let slot = Rc::new(());
    weak_for_slot = Rc::downgrade(&slot);
    context.set_slot(scope, slot);
}

println!("{}", weak_for_slot.strong_count()); // 1

Since contexts can be GC'd, and context slots should be dropped when the context is GC'd, internally context slots are dropped with a finalizer. However, in the PR that added finalizers (#895) ended up with the quirk that finalizers would not ever run while there were Globals to the finalized object, even after the isolate is dropped. This was made necessary because finalizers take a &mut v8::Isolate, which means you could use globals to the object if any existed.

The finalizer callback used for context slots, however, do not use the &mut v8::Isolate argument at all, so it would be safe to run those callbacks as part of dropping the isolate if they have not run before. Should this be the case?

[andreubotella]

For the record, I reworked the OP to hopefully be more clear on what the issue is. This issue blocks denoland/deno#15760 because that PR indirectly stores a tokio mpsc sender in a context slot, that then ends up never being dropped, and this is causing deno bench to sleep and never wake up.

Rephrasing the OP also made me think that there could be a number of other cases where you need a finalization callback that is guaranteed to run at some point, so we might be able to make an static method v8::Weak::with_guaranteed_finalizer that doesn't take a &mut Isolate, or that takes one but is unsafe. At the time these finalizers would run, all globals that could have been dropped before the isolate is destroyed have been dropped, so the remaining finalizers might end up dropping or otherwise modifying state that might be relied by the remaining globals, so it's unsafe to call v8::Global::open on any existing global, but callbacks that don't do that should be safe.

What do you think?