Docs: GDnative Reference Counting

[Cygon]

Request for documentation

I'm having a hard time figuring out the rules for dealing with references to other nodes and types in GDnative.

Documentation on this seems to be sparse and I'm worried about producing crashes (by not incrementing the reference counter when taking ownership of something) or leaking memory (because I'm not releasing references properly).

It would be very useful to provide some documentation, or even just a set of rules, on how to deal with references in GDnative.

The following 3 question cover most of what I feel is unclear right now and having answers to them would help a great deal:

1. Let's assume I had a loose class that's not a node. Should I inherit from godot::Reference (refcounted) or from godot::Object (not refcounted)?

class Move : public godot::Reference {
    GODOT_CLASS(Move, godot::Reference)
};

1a. If the class inherits from godot::Object, how is ownership handled?

# GDscript
func step1() -> void:
    var some_move = Move.new()
    cool_gdnative_object.set_move(some_move)

    # Will some_move get freed at this point?
    # Can cool_gdnative_object hold on to some_move or is ownership transfer not possible?

1b. If the class inherits from godot::Reference, when do I increment/decrement the reference counter?

# GDnative
void CoolGdnativeClass::set_move(Move *some_move) {
    this->activeMove = some_move;

    // I haven't incremented some_move's reference counter.
    // Should I, so it doesn't get freed while I'm still holding the pointer?
}

2. How do I correctly handle the reference counter when I return a new object?

# GDnative
Move *CoolGdnativeClass::create_awesome_move() {
    Move *awesomeMove = AwesomeMove::_new(); // reference counter == 1?
    return awesomeMove; // ownership transfer to caller
}

If a new object starts out with a reference count of 1, then I can't decrement it before returning the object.

Thus, it follows that returning an object must also share ownership:

• If I instead returned an object that I was holding (i.e. this->active_move), I'd have to increment the reference counter before doing so

• When I call any method that returns an object, I need to decrement the reference counter on the returned object after I'm done with it.

3. Godot nodes do not inherit from Reference

class CameraController : public godot::Node {
    GODOT_CLASS(CameraController, godot::Node)

    private: godot::Ref<godot::Spatial> target; // doesn't compile
}

Does this mean nodes always belong to the scene tree?

If so, wouldn't it be dangerous to store references to nodes because if the node gets destroyed (i.e. enemy dies, etc.), anyone holding a reference to it would have a dangling pointer?

Or do it need to get some 'instance id', 'rid' or somesuch and always look it up via Godot's scene server?

Existing documentation (or rather, attempts to find it :))

Godot Variants do call .ref.unref() on objects during clear()
https://github.com/godotengine/godot/blob/master/core/variant.cpp#L1107-L1111

Godot's reference.h contains a helper class Ref<T> which does manipulate reference counters
https://github.com/godotengine/godot/blob/master/core/reference.h#L64

godot-cpp has a Ref<T> class with a comment that states it's replicating Godot's own Ref<T> class
https://github.com/GodotNativeTools/godot-cpp/blob/master/include/core/Ref.hpp#L10

[Cygon]

After discussing this on discord, I believe to have part of my answers. There are two main ways GDnative objects can hold onto nodes:

1. Store a godot::NodePath and look up the other node (via get_node()) each time it is accessed. If the node doesn't exist, it will return nullptr. This is straightforward and doesn't need any finicky checking and cached pointer management.

2. Store the instance id of the node, obtainable via godot::Object::get_instance_id(). Also store a pointer to the node, already cast_to its desired type for maximum efficiency. Be disciplined and alway check with the non-existent godot::is_instance_id_valid() method before using the node pointer.

There is a method godot_is_instance_valid() in gdnative.h, but it's not wrapped in godot-cpp yet and takes an Object *. And, following the trail of that method into core/object.h reveals that it's checking the object pointer (baarf!) which might very well be given to another node, unlike the instance id...

Suggestion: add godot::is_instance_id_valid() to enable use case 2. Or wrap ObjectDB::get_instance() in gdnative and godot-cpp, which looks up by instance id, not pointer, and would also do the trick of checking if the node is still alive.

[follower]

Until recently there have been a number of bugs relating to Ref<T> which may have lead to crashes or memory leaks.

I've attempted to summarize the current state & suggested approach in #417.