From 8a09a2f88c1691fa5a8bf1a4854d66b22694a31d Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Wed, 4 Mar 2026 10:24:08 +0300 Subject: [PATCH] GDScript: Fix and simplify coroutine stack clearing --- modules/gdscript/gdscript_function.cpp | 21 +------ modules/gdscript/gdscript_function.h | 1 - modules/gdscript/gdscript_vm.cpp | 17 +++--- .../runtime/features/coroutine_clearing.gd | 55 +++++++++++++++++-- .../runtime/features/coroutine_clearing.out | 14 ++++- 5 files changed, 72 insertions(+), 36 deletions(-) diff --git a/modules/gdscript/gdscript_function.cpp b/modules/gdscript/gdscript_function.cpp index e81d4ca8a7..2925e2d59e 100644 --- a/modules/gdscript/gdscript_function.cpp +++ b/modules/gdscript/gdscript_function.cpp @@ -325,7 +325,7 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) { return Variant(); #endif } - // Do these now to avoid locking again after the call + // Do these now to avoid locking again after the call. scripts_list.remove_from_list(); instances_list.remove_from_list(); } @@ -334,26 +334,9 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) { Callable::CallError err; Variant ret = function->call(nullptr, nullptr, 0, err, &state); - bool completed = true; - - // If the return value is a GDScriptFunctionState reference, - // then the function did await again after resuming. - if (ret.is_ref_counted()) { - GDScriptFunctionState *gdfs = Object::cast_to(ret); - if (gdfs && gdfs->function == function) { - completed = false; - // Keep the first state alive via reference. - gdfs->first_state = first_state.is_valid() ? first_state : Ref(this); - } - } - - function = nullptr; //cleaned up; + function = nullptr; // Cleaned up. state.result = Variant(); - if (completed) { - _clear_stack(); - } - return ret; } diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index df758e80e2..ee6f2515a9 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -505,7 +505,6 @@ class GDScriptFunctionState : public RefCounted { GDScriptFunction *function = nullptr; GDScriptFunction::CallState state; Variant _signal_callback(const Variant **p_args, int p_argcount, Callable::CallError &r_error); - Ref first_state; SelfList scripts_list; SelfList instances_list; diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp index f56397c308..377262b51f 100644 --- a/modules/gdscript/gdscript_vm.cpp +++ b/modules/gdscript/gdscript_vm.cpp @@ -543,9 +543,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a int line = _initial_line; if (p_state) { - //use existing (supplied) state (awaited) + // Use existing (supplied) state (awaited). stack = (Variant *)p_state->stack.ptr(); - instruction_args = (Variant **)&p_state->stack.ptr()[sizeof(Variant) * p_state->stack_size]; //ptr() to avoid bounds check + instruction_args = (Variant **)&p_state->stack.ptr()[sizeof(Variant) * p_state->stack_size]; // `ptr()` to avoid bounds check. line = p_state->line; ip = p_state->ip; alloca_size = p_state->stack.size(); @@ -553,6 +553,11 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a p_instance = p_state->instance; defarg = p_state->defarg; + // Responsibility for the stack is moved from `GDScriptFunctionState` to this method. So, we reset `p_state->stack_size` + // to prevent `GDScriptFunctionState::_clear_stack()` from clearing the stack again. + // NOTE: Strictly speaking, ownership doesn't move. However, we can be sure that `p_state->stack` won't be cleared + // before the current call completes, and that `p_state` won't be resumed again. + p_state->stack_size = 0; } else { if (p_argcount != _argument_count) { if (p_argcount > _argument_count) { @@ -4000,15 +4005,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a // This ensures the call stack can be properly shown when using `await`, showing what resumed the function. if (!p_state || awaited) { GDScriptLanguage::get_singleton()->exit_function(); - - // Free stack, except reserved addresses. - for (int i = FIXED_ADDRESSES_MAX; i < _stack_size; i++) { - stack[i].~Variant(); - } } - // Always free reserved addresses, since they are never copied. - for (int i = 0; i < FIXED_ADDRESSES_MAX; i++) { + for (int i = 0; i < _stack_size; i++) { stack[i].~Variant(); } diff --git a/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.gd b/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.gd index 4440a25083..e6bdd2dc3e 100644 --- a/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.gd +++ b/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.gd @@ -1,13 +1,16 @@ -# GH-116706 - class Instance: func _init() -> void: print("Instance _init") + func _to_string() -> String: + return "" + func _notification(what: int) -> void: if what == NOTIFICATION_PREDELETE: print("Instance predelete") +# GH-116706 + class LocalOwner: signal never_emitted() @@ -24,10 +27,52 @@ class LocalOwner: await never_emitted print("interrupted_coroutine end") -func test(): - print("test begin") +func subtest_order(): + print("subtest_order begin") var local_owner := LocalOwner.new() @warning_ignore("missing_await") local_owner.interrupted_coroutine() local_owner = null - print("test end") + print("subtest_order end") + +# GH-117049 + +signal tick() + +func await_before_and_after() -> void: + await tick + var packed_array: PackedStringArray = ["abc"] + var instance := Instance.new() + await tick + prints(packed_array, instance) + +func await_two_after() -> void: + var packed_array: PackedStringArray = ["abc"] + var instance := Instance.new() + await tick + await tick + prints(packed_array, instance) + +func subtest_resume(): + print("subtest_resume begin") + + @warning_ignore("missing_await") + await_before_and_after() + tick.emit() + tick.emit() + + print("---") + + @warning_ignore("missing_await") + await_two_after() + tick.emit() + tick.emit() + + print("subtest_resume end") + +# === + +func test(): + subtest_order() + print("===") + subtest_resume() diff --git a/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.out b/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.out index fb4f93bdcf..abd9a9696d 100644 --- a/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.out +++ b/modules/gdscript/tests/scripts/runtime/features/coroutine_clearing.out @@ -1,8 +1,18 @@ GDTEST_OK -test begin +subtest_order begin LocalOwner _init interrupted_coroutine begin Instance _init LocalOwner predelete Instance predelete -test end +subtest_order end +=== +subtest_resume begin +Instance _init +["abc"] +Instance predelete +--- +Instance _init +["abc"] +Instance predelete +subtest_resume end