Problems with reference count on using custom singletons inheriting from `RefCounted`

[StatisMike]

Lately I've found a problem with GodotClasses thats are to be saved as singleton. During discussion with @lilizoey we were able to pinpoint the possible problem: it seems that gdext decreases the reference count too eagerly. Below is an example:

#[itest(focus)]
fn test_refcount_singleton() {
    #[derive(godot::prelude::GodotClass)]
    #[class(init, base=RefCounted)]
    struct SingletonTest;

    use godot::obj::UserClass;

    let mut engine = godot::engine::Engine::singleton();
    let singleton = SingletonTest::new_gd();
    engine.register_singleton("SingletonTest".into(), singleton.clone().upcast());
    
    // Uncommenting line below makes the test pass.
    // std::mem::forget(singleton);
    engine.get_singleton("SingletonTest".into());
}

[Bromeon]

Oh yes, thanks. I started working on a fix a few weeks ago (branch bugfix/singleton-uaf), but got sidetracked.

[TitanNano]

I created a reproduction project for this bug in #549. I'm copying it over to this issue incase it is useful.

---

When registering a recounted object as a singleton, the ref-count of the object is decremented after the registration while the engine keeps holding on to a pointer to the object.

This is demonstrated by the log output of the example project:

refcount (initial): 1 # the object has been created and we hold the only reference to it.
refcount (post-clone): 2 # smart pointer was cloned. There are now two references.
# second smart pointer is moved into Engine::register_singleton.
ref-count (post-register): 1 # Second smart pointer was dropped once the function returned. The engine did not increment the ref-count.
# original ref will be droped at the end of the scope.

Once the library tries to access the singleton again during shutdown, the engine crashes because the object has already been freed.

ref-count-bug.zip

[TitanNano]

My current workaround for this is a manually managed wrapper for ref-counted singletons.

#[derive(GodotClass)]
#[class(base = Object, init)]
struct RefCountedSingleton {
    inner: Gd<RefCounted>,
}

impl RefCountedSingleton {
    fn new(object: Gd<RefCounted>) -> Gd<Self> {
        Gd::from_object(Self { inner: object })
    }

    fn get(&self) -> Gd<RefCounted> {
        self.inner.clone()
    }
}

[lokimckay]

Currently RefCounted is required for signals to work according to the docs

So if a user wants a singleton with signals, this issue.. is an issue 😆

But as the docs here allude to, another workaround is to use Object's emit_signal instead

self.base_mut().emit_signal("my_signal", &[]);

[TitanNano]

Currently RefCounted is required for signals to work according to the docs

I think the documentation is misleading. Only the base: Base<T> field is required, not the base type RefCounted.