It is possible to have a Base pointing to a dead object

[lilizoey]

Using this code:

use godot::prelude::*;
use std::cell::RefCell;

struct Test;

#[gdextension]
unsafe impl ExtensionLibrary for Test {}

thread_local! {
    pub static FOO: RefCell<Option<Base<RefCounted>>> = RefCell::new(None);
}

#[derive(GodotClass)]
struct Foo {}

#[godot_api]
impl INode for Foo {
    fn init(base: Base<RefCounted>) -> Self {
        FOO.with_borrow_mut(|foo| *foo = Some(base));
        Self {}
    }
}

#[derive(GodotClass)]
#[class(base = Node, init)]
struct Bar {}

#[godot_api]
impl INode for Bar {
    fn ready(&mut self) {
        let foo = Foo::new_gd();
        std::mem::drop(foo);
        FOO.with_borrow(|foo| {
            // This foo is a `Base` pointing to a dead object.
        });
    }
}

We can get a hold of a Base pointing to a dead object. It is unclear to me if this is a soundness issue or not, it seems like any case that could cause unsoundness due to this has proper runtime checks in place. But i do know we generally assume a Base will always point to a live object.

Is this an issue we need to worry about? If so then what should we do, it's unclear to me if it's even possible to fix this issue. Best thing we could do is maybe just documenting explicitly that this is possible, and thus not something we can rely on as a safety invariant.

[Bromeon]

This sounds very similar to #23 -- which is about subtyping but should also cover dead objects.

I'll need to look into it a bit deeper 🤔

[Bromeon]

It's also possible to do this without thread-locals:

#[derive(GodotClass)]
#[class(init)]
struct Foo {}

impl Foo {
    fn smuggle_out(other_base: &mut Option<Base<RefCounted>>) -> Gd<Self> {
        Gd::from_init_fn(|base| {
            *other_base = Some(base);
            Self {}
        })
    }
}

#[derive(GodotClass)]
#[class(base = Node, init)]
struct Bar {}

#[godot_api]
impl INode for Bar {
    fn ready(&mut self) {
        let mut option = None;
        let foo = Foo::smuggle_out(&mut option);
        let extracted_base = option.unwrap();
    }
}

And of course, you can swap two objects' base field after construction, similarly to #23.

---

I don't think it's possible to protect fully against this (at least not without major ergonomic impediments), so we should probably double-check that we don't rely on bases being always valid, or pointing to the original object.