Using `queue_free` after `Tween.Stop` causes a memory leak

[dillyo09]

Godot version

v4.1.1.stable.official [bd6af8e]

System information

Godot v4.1.1.stable - Windows 10.0.22621 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 Ti (NVIDIA; 31.0.15.3640) - Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz (16 Threads)

Issue description

Running the following code causes a memory leak:

var tween = get_tree().create_tween()
tween.stop()
queue_free()

Steps to reproduce

1. Create a Tween
2. Use tween.stop()
3. queue_free() the Node that created the Tween

Minimal reproduction project

TweenMemoryLeak.zip

[Calinou]

cc @KoBeWi

[Marigem]

var tween = get_tree().create_tween()

This creates a tween attached to the tree instead of the node you are queue_free'ing.

var tween = create_tween() would be correct. Try and see if it still causes a memory leak with that.

[kleonc]

var tween = create_tween() would be correct.

Or var tween = get_tree().create_tween().bind_node(self) which is equivalent.

So there's no bug in here indeed. Edit: Nevermind, binded to a Node or not these seem to indeed be not freed.
Everything below is pre-edit too so might not hold:
I guess what could be done is maybe adding some warning in the docs saying that paused/stopped tween not binded to a node will not be freed unless it will be killed manually or it will finish the tweening (it would need to be resumed/played again).

Another improvement could be changing this line in the in-docs examples:

var tween = get_tree().create_tween()

to something like:

var tween = create_tween() # Or `get_tree().create_tween()` if auto-binding to the node is not wanted.

so both ways would be mentioned in each example. Hopefully this would make more users be aware of the difference between SceneTree.create_tween and Node.create_tween methods.

[KoBeWi]

Note that you can still retrieve the Tween using get_processed_tweens(), so this is not a leak.

[kleonc]

@KoBeWi From Tween.bind_node's docs:

the [Tween] will be automatically killed when the bound object is freed

godot/doc/classes/Tween.xml

Line 102 in f618701

Binds this [Tween] with the given [param node]. [Tween]s are processed directly by the [SceneTree], so they run independently of the animated nodes. When you bind a [Node] with the [Tween], the [Tween] will halt the animation when the object is not inside tree and the [Tween] will be automatically killed when the bound object is freed. Also [constant TWEEN_PAUSE_BOUND] will make the pausing behavior dependent on the bound node.

This is currently not the case, paused/stopped tweens are not freed. I think moving if (!running) after if (is_bound) should do the thing (it needs to be able to reach return false; before early returning if not running):

godot/scene/animation/tween.cpp

Lines 287 to 300 in f618701

	if (!running) {
	return true;
	}
	if (is_bound) {
	Node *node = get_bound_node();
	if (node) {
	if (!node->is_inside_tree()) {
	return true;
	}
	} else {
	return false;
	}
	}