Godot crashes when using cast_to on a node that precedes GDNative node in tree

[and3rson]

First of all, sorry for no stack trace: this is reproducible only with release builds for some reason...

Here's my code:

// goapplanner.cpp
void GOAPPlanner::initialize(Array actions) {
    Godot::print(Object::cast_to<Node>(actions[0])->get("name").operator String());
}

Here's the tree:

- parent
  - agent
    - empty_node (empty node of type Node with nothing inside)
    - goapplanner (with GDNS running previous code)

Within agent, I have the following code:

func _ready():
    $goapplanner.initialize([$empty_node])

The above code succeeds and prints empty_node. However, when I unload the scene to load another (by using parent.queue_free()), the game segfaults.

If I move empty_node AFTER goapplanner, the crash does not happen.

This is strange since I don't even keep the reference to Node* anywhere, I just cast actions[0] to Node* and then throw it away. Do cast_to-ed objects have to be marked/freed somehow?

Any ideas about what's going on here?

[and3rson]

Okay, I really wish this was documented somewhere, but it turns out that the result of cast_to must be free()-d. Once I added free, things stopped crashing. I didn't expect a result of cast_to to require free-ing.

Besides, I thought calling Object::cast_to<Object>(node)->free() would free the underlying node. Or am I still wrong? I'm really curious about what's going on there.

[MGilleronFJ]

It isn't a rule that the result of cast_to needs to be freed. It depends on what you are casting. Things stopping to crash doesn't mean they work as intented. In your case, freeing the node shouldn't be necessary, because the node is owned by the scene tree. The scene tree is in charge of doing that. Something else is buggy.

---

Side note, might not be related to the crash:

Godot::print(Object::cast_to<Node>(actions[0])->get("name").operator String());

A lot of things can go wrong in this line. If actions is empty, or if the cast fails (for whatever reason, user mistake...), there is no check at all to protect against an invalid memory access with ->get. C++ is less forgiving than GDScript, stuff like that doesnt necessarily crash, it can end up doing random stuff. A safer, more explicit-when-failing code would be:

ERR_FAIL_COND(actions.size() == 0)
Node *node = Object::cast_to<Node>(actions[0]);
ERR_FAIL_COND(node == nullptr);
Godot::print(node->get_name()); // Also changed this one because a function exists for this

[and3rson]

@MGilleronFJ thanks for the info!

I don't think the issue is with actions being empty or node being null, since I can see the node's name printed.
Still, thanks for the suggestion about ERR_FAIL_COND and get_name function

Update: I can see my code crashing even when doing just this:

void GOAPPlanner::initialize(Node *action) {
    // Do nothing
}

# gdscript:
goapplanner.initialize($empty_node)

Replacing Node *action with Variant action works though. But casting Variant action to Node via cast_to still causes crash during queue_free.

[and3rson]

Seems like godotengine/godot#48295 might be related to this.
This seems to fix my issue: godotengine/godot#48295 (comment)