Engine crashes if you free a node with an incomplete yield

[tcoxon]

Godot version

3.5.1, also 3.5.2-stable (git tag)

System information

Arch Linux (rolling release)

Issue description

I've been encountering seemingly random crashes in my project--on one particular NDA'd platform quite often, but sometimes, very rarely, on Windows and Linux as well. No errors are logged, and crash dumps don't provide enough information to debug. With the help of GDB and address sanitizer I managed to create a reproducer, attached below.

The code is quite simple. It starts in the YieldSandwichTest.gd script, which creates a node with a script and then frees the node while that script is yielding (I know that this used to cause leaks, but I think that is unrelated to the issue here):

func _ready():
	var elem = preload("YieldSandwichTestElement.tscn").instance()
	add_child(elem)
	elem.queue_free()

That node's script sets up a tween and then yields until completion of another function in an autoload script. It doesn't have to be a tween here, anything with a signal that fires after a while will do.

func _ready():
	tween = Tween.new()
	add_child(tween)

	tween.interpolate_property(self, "rect_position", null, Vector2(1920, 1080), 1.0)
	tween.start()
	yield(Co.wait_for_tween(tween), "completed")
	print("arrived")

The autoload script Co.gd:

func wait_for_tween(tween: Tween):
	yield(tween, "tween_all_completed")

If you run this in the editor or in release builds, 99.999% of the time it will run without issue. Run it in an engine built with address sanitizer though, and it'll crash immediately.

The issue seems to be that GDScriptFunctionState::_clear_stack indirectly causes the same instance of GDScriptFunctionState to be freed before it finishes. Check the value of the this pointer in frames 0, 1 and 29, below. When control flow eventually returns to _clear_stack in frame 29, it'll begin writing memory that has already been freed:

#0  GDScriptFunctionState::_clear_stack (this=0x614000063e40) at modules/gdscript/gdscript_function.cpp:1849
#1  GDScriptFunctionState::~GDScriptFunctionState (this=0x614000063e40, __in_chrg=<optimized out>) at modules/gdscript/gdscript_function.cpp:1873
#2  0x0000000012804557 in memdelete<Reference> (p_class=0x614000063e40) at ./core/os/memory.h:110
#3  memdelete<Reference> (p_class=0x614000063e40) at ./core/os/memory.h:110
#4  Ref<Reference>::unref (this=0x604000243ef0) at ./core/reference.h:258
#5  RefPtr::unref (this=this@entry=0x604000243ef0) at core/ref_ptr.cpp:85
#6  0x0000000012ba8408 in Variant::clear (this=this@entry=0x604000243ee0) at core/variant.cpp:1052
#7  0x0000000000e41c0a in Variant::~Variant (this=0x604000243ee0, __in_chrg=<optimized out>) at ./core/variant.h:438
#8  CowData<Variant>::_unref (this=<optimized out>, p_data=0x604000243ee0) at ./core/cowdata.h:205
#9  0x0000000012711df8 in CowData<Variant>::~CowData (this=<optimized out>, __in_chrg=<optimized out>) at ./core/cowdata.h:375
#10 Vector<Variant>::~Vector (this=<optimized out>, __in_chrg=<optimized out>) at ./core/vector.h:162
#11 Object::Connection::~Connection (this=<optimized out>, __in_chrg=<optimized out>) at core/object.h:416
#12 Object::Signal::Slot::~Slot (this=<optimized out>, __in_chrg=<optimized out>) at core/object.h:459
#13 VMap<Object::Signal::Target, Object::Signal::Slot>::Pair::~Pair (this=0x60d000378b10, __in_chrg=<optimized out>) at ./core/vmap.h:40
#14 CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::_unref (this=<optimized out>, p_data=0x60d000378b10) at ./core/cowdata.h:205
#15 0x00000000126a07df in CowData<VMap<Object::Signal::Target, Object::Signal::Slot>::Pair>::~CowData (this=0x60c000078b30, __in_chrg=<optimized out>) at ./core/cowdata.h:376
#16 VMap<Object::Signal::Target, Object::Signal::Slot>::~VMap (this=0x60c000078b30, __in_chrg=<optimized out>) at ./core/vmap.h:38
#17 Object::Signal::~Signal (this=0x60c000078ad8, __in_chrg=<optimized out>) at core/object.h:445
#18 HashMap<StringName, Object::Signal, HashMapHasherDefault, HashMapComparatorDefault<StringName>, (unsigned char)3, (unsigned char)8>::Pair::~Pair (this=0x60c000078ad0, __in_chrg=<optimized out>) at ./core/hash_map.h:61
#19 HashMap<StringName, Object::Signal, HashMapHasherDefault, HashMapComparatorDefault<StringName>, (unsigned char)3, (unsigned char)8>::Element::~Element (this=0x60c000078ac0, __in_chrg=<optimized out>) at ./core/hash_map.h:74
#20 memdelete<HashMap<StringName, Object::Signal, HashMapHasherDefault, HashMapComparatorDefault<StringName>, (unsigned char)3, (unsigned char)8>::Element> (p_class=0x60c000078ac0) at ./core/os/memory.h:115
#21 HashMap<StringName, Object::Signal, HashMapHasherDefault, HashMapComparatorDefault<StringName>, (unsigned char)3, (unsigned char)8>::erase (p_key=..., this=0x614000063c48) at ./core/hash_map.h:407
#22 Object::~Object (this=<optimized out>, __in_chrg=<optimized out>) at core/object.cpp:1958
#23 0x0000000012804557 in memdelete<Reference> (p_class=0x614000063c40) at ./core/os/memory.h:110
#24 memdelete<Reference> (p_class=0x614000063c40) at ./core/os/memory.h:110
#25 Ref<Reference>::unref (this=0x60d000378a50) at ./core/reference.h:258
#26 RefPtr::unref (this=this@entry=0x60d000378a50) at core/ref_ptr.cpp:85
#27 0x0000000012ba8408 in Variant::clear (this=this@entry=0x60d000378a40) at core/variant.cpp:1052
#28 0x0000000002b04c45 in Variant::~Variant (this=0x60d000378a40, __in_chrg=<optimized out>) at ./core/variant.h:438
#29 GDScriptFunctionState::_clear_stack (this=this@entry=0x614000063e40) at modules/gdscript/gdscript_function.cpp:1852
#30 0x0000000002910800 in GDScriptInstance::~GDScriptInstance (this=0x6060000abbc0, __in_chrg=<optimized out>) at modules/gdscript/gdscript.cpp:1358

I think it goes something like this:

1. Freeing YieldSandwichTestElement causes the stack of its _ready function state to be cleared (frame 29)
2. This clears a reference to the GDScriptFunctionState returned from Co.wait_for_tween (frame 25?), which causes that function state to be freed.
3. The signal connections of the GDScriptFunctionState returned from Co.wait_for_tween are disconnected (frame ~12).
4. One of the signal connections has a bind that references the GDScriptFunctionState of the _ready function. That reference is cleared (frame 6).
5. Clearing that reference causes the _ready function state to be deleted (frame 1).

Steps to reproduce

1. Download the attached reproducer project.
2. Build the engine with use_asan=yes
3. Run the attached project using that build of the engine.

Address sanitizer will print an error and then the program will crash.

Minimal reproduction project

yield-sandwich.zip

[lawnjelly]

Closed by #97464 .