I am working with the following function in GDScript:

func remove_entity(ent: Sprite):
    sprite_lst[ent].queue_free()
    sprite_lst.erase(ent)
    draw_entities()

sprite_lst is a Dictionary that holds all of the Sprites on the HUD. The keys for these Sprites are the actual object that they're representing. This function is called to remove one of those sprites from the HUD.

This mostly works alright, however, it seems to have a race condition preventing it from always working as intended. Sometimes, this function will throw an exception when trying to call queue_free() on the intended object, because ent is not found as a key in sprite_lst.

To troubleshoot, I tried printing out the object ID before erasing it. However, once I added a print statement to this function, it stopped throwing the exception. So then I changed the function to this:

func remove_entity(ent: Sprite):
    var this_sprite = sprite_lst[ent]
    this_sprite.queue_free()
    sprite_lst.erase(ent)
    draw_entities()

When storing the entity from sprite_lst, the function always works. It seems to me like sometimes the erase() method is called on the dictionary, before queue_free() is called on the Sprite.

Is there any way to prevent this race condition, or force these two lines to run sequentially?

CodePudding user response：

It seems to me like sometimes the erase() method is called on the dictionary, before queue_free() is called on the Sprite.

In your code, you always call queue_free() first. However, that queues a free. It is similar to call_deferred("free") (not exactly the same, but this gets the idea across).

We can apply plenty of defensive programming to your code.

First of all, we can check if the entry is there:

func remove_entity(ent: Sprite):
    if sprite_lst.has(ent):
        # …
        pass

Or we can use get which will return null if it isn't:

func remove_entity(ent: Sprite):
    var this_sprite = sprite_lst.get(ent)
    # …

Second we can check if we got a valid instance (not null and not freed):

func remove_entity(ent: Sprite):
    var this_sprite = sprite_lst.get(ent)
    if is_instance_valid(this_sprite):
        this_sprite.queue_free()

    # …

That way there should be no exception on the queue_free call.

I suppose you want to remove any invalid entry, too, right? Thankfully erase won't complain about a key not being there, so we don't need a check to guard it.

func remove_entity(ent: Sprite):
    var this_sprite = sprite_lst.get(ent)
    if is_instance_valid(this_sprite):
        this_sprite.queue_free()

    sprite_lst.erase(ent)
    draw_entities()

Alright, that takes care of the error. However, why do you care about when the node gets freed? I'll venture to guess that the reason you care is because you rely on it to remove the node from the scene tree. We could address that directly:

func remove_entity(ent: Sprite):
    var this_sprite:Node = sprite_lst.get(ent)
    if is_instance_valid(this_sprite):
        if this_sprite.is_inside_tree():
            this_sprite.get_parent().remove_child(this_sprite)

        this_sprite.queue_free()

    sprite_lst.erase(ent)
    draw_entities()

I suppose you could do this instead (which isn't the same, because this also works outside of the scene tree, but I don't think we need to worry about that case):

func remove_entity(ent: Sprite):
    var this_sprite:Node = sprite_lst.get(ent)
    if is_instance_valid(this_sprite):
        var parent:Node = this_sprite.get_parent()
        if parent != null:
            parent.remove_child(this_sprite)

        this_sprite.queue_free()

    sprite_lst.erase(ent)
    draw_entities()

On that note, be aware that attempting to remove a Node from another Node which isn't the parent will cause an error. So even if you know which should be parent, you still need a check:

func remove_entity(ent: Sprite):
    var this_sprite:Node = sprite_lst.get(ent)
    if is_instance_valid(this_sprite):
        if parent == this_sprite.get_parent():
            parent.remove_child(this_sprite)

        this_sprite.queue_free()

    sprite_lst.erase(ent)
    draw_entities()

Or, if it works for you, you could avoid parent shenanigans with remove_and_skip, for example:

func remove_entity(ent: Sprite):
    var this_sprite:Node = sprite_lst.get(ent)
    if is_instance_valid(this_sprite):
        if this_sprite.is_inside_tree():
            this_sprite.remove_and_skip()

        this_sprite.queue_free()

    sprite_lst.erase(ent)
    draw_entities()

Be aware that remove_and_skip will add the children of the node back to the scene tree. While remove_child would not (the Node will keep children outside the scene tree).

For completeness I'll also mention that:

• erase returns whether or not it removed something.
• You can use is_queued_for_deletion to check if you called queue_free on an Object.

And also remind you that freeing a Node does not make all the reference to it null, instead they become invalid, which is why is_instance_valid and checking if it isn't null aren't the same thing.