Godot: Reference to freed object can return a random different one

[jaykyburz]

Godot version & OS/device including version:
3.1.1 Windows 10.

Issue description:
I think this is a simple explanation of an issue I have on my project. I have raised this issue without an example project because it will take some time to reproduce in a simple form and I would like some godot experts to look at what I am describing, and let me know if the issue is already logged and known (and perhaps fixed) before I spend more time working on a reproduction cases

---

Say I have a RobotFactory class and it can instantiate Robots, but it is only allowed to have one active in the world at once. It saves the instantiated robot in a var.

var my_robot = RobotScene.instance()

The robot walks around the world for a while an then is destroyed by something so in its own script it calls queue_free() on itself.

Sometime later in the RobotFactory we call is_instance_valid(my_robot) to see if we should create a new robot.

In my project I have found that sometimes is_instance_valid(my_robot) returns true after my_robot has been freed, but that it is pointing to some other random object in the scene that has been instantiated. Perhaps sometime after the queue_free, but before is_instance_valid call.

The only thing I think could be happening is that there is some kind of underlying pooling system and my_robot is pointing to somewhere that is being reused after the robot is freed.

Please let me know if this is a known issue, is already logged, or if I should do some more work to explain the issue better.

---

Bugsquad edit: Keyword: previously freed instance

[KoBeWi]

Oooh, it had totally the same thing. Good to know I'm not alone ;_;

In my case, I had a Control node with script that calls queue_free() inside _ready() on some condition. Then another node referenced it in an onready variable. Later in code I had something like:

if is_instance_valid(my_control):
    my_control.do_stuff()

And it raised an error ~ "No method do_stuff in class ParticleMaterial". Like, what XD
It just sometimes takes random object or resource from the scene. But now always, it depends on when I use it. I tried to make reproduction project, but I couldn't >_> Although I'm instancing lots of objects in my scene using ResourceInteractiveLoader and I didn't test it with minimal project yet.

Ultimately I ended with this ;_;

if is_instance_valid(my_control) and my_control is Control:
    my_control.do_stuff()

[Zylann]

Could it be that the object ID got re-used? Also, are there threads involved? (I mean, anywhere, not just scripts).

[bojidar-bg]

Can confirm on a game I made some time ago without threads.
In it, I had enemies with the following snipped of code:

func ....():
        if target == null or not is_instance_valid(target):
                var players = get_tree().get_nodes_in_group("player")
                if players.size() == 0: return
                target = players[randi() % players.size()]

It worked -- enemies targeted random players, and switched when their target died.
But, when all players died, and a new enemy spawned, all other enemies targeted that enemy, due to the reference being reused.

[jaykyburz]

Attached is a sample project the demonstrates the issue. It's basically what I described above.

One factory produces red cubes and keeps a reference to the cube it last created. It points to its reference using the pink squashed sphere in the screen shot below. The red factory produces cubes every 4 seconds but they are freed after 1 second so there is 3 seconds where the is_instance_valid(my_instance) should return false.

There is a second factory producing blue cubes every 1 second.

We find that every so often, is_instance_valid(my_instance) on the red factory returns true when it should not, but is pointing to one of the blue cubes.

IsInstanceValid.zip

Update: on reading my comment above I realise that saying is_instance_valid should not return true is wrong, because it is pointing to a valid instance, its just not the instance I put in the variable. So strictly speaking is_instance_valid is working. But, I discovered is_instance_valid command when reading here on git hub about ways to detect if an instance we have has been freed. I don't know how to do that if is_instance_valid is not the correct way.

[reduz]

This is a known problem, but it requires some major changes to fix. 4.0 will change the way objects are referenced so this (and other similar issues) can be properly fixed.

[jaykyburz]

Thanks for the update.

I think if this is going to be a 4.0 issue I had better remove is_instance_valid from my project. I've only used it 11 times so far. Normally its AI's keeping track of enemy its following or shooting at. It could lead to very subtle wired bugs with AI's walking off in random directions or shooting at the wrong thing. I might try and implement a "about_to_free" signal and emit it before calling queue_free. Then anybody with a reference can listen for the signal and set the var holding the instance to null.

We might consider removing is_instance_valid in 3.2, or having a warning when you use it, or at the very least update the help. All it says at the moment is "Returns whether instance is a valid object (e.g. has not been deleted from memory)." which is not true.

[vnen]

Pretty sure there's already a report about this.

Edit: it's this one: #22395

[KoBeWi]

I found a workaround for some cases. Let's say you have an object stored in a variable called my_instance. You can just do

my_instance.connect("tree_exited", self, "set", ["my_instance", null])

Then instead of using is_instance_valid() you can just do if my_instance, which will now reliably point to null if object is freed. This of course breaks if you use remove_child.

[jaykyburz]

I wrote a little helper class and now try never to save a reference to a node directly.

extends Node
class_name NodeRefrence

var __ref:Node

func get_ref()->Node:
    return __ref

func set_ref(ref:Node):
    clear_ref()
    __ref = ref
    __ref.connect("tree_exiting", self, "ref_tree_exiting")

func clear_ref():
    if __ref:
        __ref.disconnect("tree_exiting", self, "ref_tree_exiting")
    __ref = null

func ref_tree_exiting():
    __ref = null

Then to use this you have some code that looks something like this.

var target_ref = NodeRefrence.new()
func _ready():
    target_ref.set_ref(some_node)
    ....
    var target = target_ref.get_ref()

[akien-mga]

See #22395 for other example projects that reproduce it.

It sucks that we can't have a proper fix for this before 4.0 :/

[lawnjelly]

When I tried jaykyburz's test project I found that using WeakRef instead of a raw object pointer also fixed it for me.

i.e.

var my_robot_ref : WeakRef

my_robot_ref = weakref(bot)

    if my_robot_ref:
        var ref = my_robot_ref.get_ref()
        if ref:
            $RedPointer.global_transform = ref.global_transform

WeakRef internally uses an ObjectID rather than pointer, which is looked up when you call get_ref().

If you store both the original pointer and a weakref in the test project, if you remote debug it, you can clearly see the point at which the pointer starts pointing to the wrong object.