From a095c5e3fabf98ff4b571d4ad0c97a2e8fe6872d Mon Sep 17 00:00:00 2001 From: Serhii Snitsaruk Date: Fri, 23 May 2025 19:32:01 +0200 Subject: [PATCH] GDScript call stack as reverse linked list with fixed coroutines * GDScript call stack as reverse linked list with issues fixed (originally proposed in 91006). * Fix coroutine issues with call stack by resuming async call chain inside `GDScriptFunction::call()`. * This fixes corrupted line numbers for coroutines in the debugger and backtrace (106489). Co-authored-by: Juan Linietsky --- doc/classes/ProjectSettings.xml | 1 - modules/gdscript/gdscript.cpp | 16 +++++- modules/gdscript/gdscript.h | 77 ++++++++++++-------------- modules/gdscript/gdscript_editor.cpp | 42 ++++++-------- modules/gdscript/gdscript_function.cpp | 9 +-- modules/gdscript/gdscript_function.h | 1 + modules/gdscript/gdscript_vm.cpp | 23 +++++++- 7 files changed, 90 insertions(+), 79 deletions(-) diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index 0e0cfee3189..b3077c4536c 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -661,7 +661,6 @@ Whether GDScript call stacks will be tracked in release builds, thus allowing [method Engine.capture_script_backtraces] to function. - Enabling this comes at the cost of roughly 40 KiB for every thread that runs any GDScript code. [b]Note:[/b] This setting has no effect on editor builds or debug builds, where GDScript call stacks are tracked regardless. diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 864efc67b95..6e60e1528f7 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -2329,8 +2329,6 @@ void GDScriptLanguage::finish() { } finishing = true; - _call_stack.free(); - // Clear the cache before parsing the script_list GDScriptCache::clear(); @@ -2922,7 +2920,19 @@ String GDScriptLanguage::get_global_class_name(const String &p_path, String *r_b return c->identifier != nullptr ? String(c->identifier->name) : String(); } -thread_local GDScriptLanguage::CallStack GDScriptLanguage::_call_stack; +thread_local GDScriptLanguage::CallLevel *GDScriptLanguage::_call_stack = nullptr; +thread_local uint32_t GDScriptLanguage::_call_stack_size = 0; + +GDScriptLanguage::CallLevel *GDScriptLanguage::_get_stack_level(uint32_t p_level) { + ERR_FAIL_UNSIGNED_INDEX_V(p_level, _call_stack_size, nullptr); + CallLevel *level = _call_stack; // Start from top + uint32_t level_index = 0; + while (p_level > level_index) { + level_index++; + level = level->prev; + } + return level; +} GDScriptLanguage::GDScriptLanguage() { ERR_FAIL_COND(singleton); diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h index 58ea47c5989..d74f0449090 100644 --- a/modules/gdscript/gdscript.h +++ b/modules/gdscript/gdscript.h @@ -437,31 +437,22 @@ class GDScriptLanguage : public ScriptLanguage { GDScriptInstance *instance = nullptr; int *ip = nullptr; int *line = nullptr; + CallLevel *prev = nullptr; // Reverse linked list (stack). }; static thread_local int _debug_parse_err_line; static thread_local String _debug_parse_err_file; static thread_local String _debug_error; - struct CallStack { - CallLevel *levels = nullptr; - int stack_pos = 0; - void free() { - if (levels) { - memdelete_arr(levels); - levels = nullptr; - } - } - ~CallStack() { - free(); - } - }; + static thread_local CallLevel *_call_stack; + static thread_local uint32_t _call_stack_size; + uint32_t _debug_max_call_stack = 0; - static thread_local CallStack _call_stack; - int _debug_max_call_stack = 0; bool track_call_stack = false; bool track_locals = false; + static CallLevel *_get_stack_level(uint32_t p_level); + void _add_global(const StringName &p_name, const Variant &p_value); void _remove_global(const StringName &p_name); @@ -492,15 +483,11 @@ public: bool debug_break(const String &p_error, bool p_allow_continue = true); bool debug_break_parse(const String &p_file, int p_line, const String &p_error); - _FORCE_INLINE_ void enter_function(GDScriptInstance *p_instance, GDScriptFunction *p_function, Variant *p_stack, int *p_ip, int *p_line) { + _FORCE_INLINE_ void enter_function(CallLevel *call_level, GDScriptInstance *p_instance, GDScriptFunction *p_function, Variant *p_stack, int *p_ip, int *p_line) { if (!track_call_stack) { return; } - if (unlikely(_call_stack.levels == nullptr)) { - _call_stack.levels = memnew_arr(CallLevel, _debug_max_call_stack + 1); - } - #ifdef DEBUG_ENABLED ScriptDebugger *script_debugger = EngineDebugger::get_script_debugger(); if (script_debugger != nullptr && script_debugger->get_lines_left() > 0 && script_debugger->get_depth() >= 0) { @@ -508,7 +495,7 @@ public: } #endif - if (unlikely(_call_stack.stack_pos >= _debug_max_call_stack)) { + if (unlikely(_call_stack_size >= _debug_max_call_stack)) { _debug_error = vformat("Stack overflow (stack size: %s). Check for infinite recursion in your script.", _debug_max_call_stack); #ifdef DEBUG_ENABLED @@ -520,13 +507,14 @@ public: return; } - CallLevel &call_level = _call_stack.levels[_call_stack.stack_pos]; - call_level.stack = p_stack; - call_level.instance = p_instance; - call_level.function = p_function; - call_level.ip = p_ip; - call_level.line = p_line; - _call_stack.stack_pos++; + call_level->prev = _call_stack; + _call_stack = call_level; + call_level->stack = p_stack; + call_level->instance = p_instance; + call_level->function = p_function; + call_level->ip = p_ip; + call_level->line = p_line; + _call_stack_size++; } _FORCE_INLINE_ void exit_function() { @@ -536,35 +524,42 @@ public: #ifdef DEBUG_ENABLED ScriptDebugger *script_debugger = EngineDebugger::get_script_debugger(); - if (script_debugger != nullptr && script_debugger->get_lines_left() > 0 && script_debugger->get_depth() >= 0) { + if (script_debugger && script_debugger->get_lines_left() > 0 && script_debugger->get_depth() >= 0) { script_debugger->set_depth(script_debugger->get_depth() - 1); } #endif - if (unlikely(_call_stack.stack_pos == 0)) { - _debug_error = "Stack Underflow (Engine Bug)"; - + if (unlikely(_call_stack_size == 0)) { #ifdef DEBUG_ENABLED - if (script_debugger != nullptr) { + if (script_debugger) { + _debug_error = "Stack Underflow (Engine Bug)"; script_debugger->debug(this); + } else { + ERR_PRINT("Stack underflow! (Engine Bug)"); } +#else // !DEBUG_ENABLED + ERR_PRINT("Stack underflow! (Engine Bug)"); #endif - return; } - _call_stack.stack_pos--; + _call_stack_size--; + _call_stack = _call_stack->prev; } virtual Vector debug_get_current_stack_info() override { Vector csi; - csi.resize(_call_stack.stack_pos); - for (int i = 0; i < _call_stack.stack_pos; i++) { - csi.write[_call_stack.stack_pos - i - 1].line = _call_stack.levels[i].line ? *_call_stack.levels[i].line : 0; - if (_call_stack.levels[i].function) { - csi.write[_call_stack.stack_pos - i - 1].func = _call_stack.levels[i].function->get_name(); - csi.write[_call_stack.stack_pos - i - 1].file = _call_stack.levels[i].function->get_script()->get_script_path(); + csi.resize(_call_stack_size); + CallLevel *cl = _call_stack; + uint32_t idx = 0; + while (cl) { + csi.write[idx].line = *cl->line; + if (cl->function) { + csi.write[idx].func = cl->function->get_name(); + csi.write[idx].file = cl->function->get_script()->get_script_path(); } + idx++; + cl = cl->prev; } return csi; } diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp index 3340b9df25c..7cdf4804f9b 100644 --- a/modules/gdscript/gdscript_editor.cpp +++ b/modules/gdscript/gdscript_editor.cpp @@ -297,7 +297,7 @@ int GDScriptLanguage::debug_get_stack_level_count() const { return 1; } - return _call_stack.stack_pos; + return _call_stack_size; } int GDScriptLanguage::debug_get_stack_level_line(int p_level) const { @@ -305,11 +305,9 @@ int GDScriptLanguage::debug_get_stack_level_line(int p_level) const { return _debug_parse_err_line; } - ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, -1); + ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, -1); - int l = _call_stack.stack_pos - p_level - 1; - - return *(_call_stack.levels[l].line); + return *(_get_stack_level(p_level)->line); } String GDScriptLanguage::debug_get_stack_level_function(int p_level) const { @@ -317,9 +315,9 @@ String GDScriptLanguage::debug_get_stack_level_function(int p_level) const { return ""; } - ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, ""); - int l = _call_stack.stack_pos - p_level - 1; - return _call_stack.levels[l].function->get_name(); + ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, ""); + GDScriptFunction *func = _get_stack_level(p_level)->function; + return func ? func->get_name().operator String() : ""; } String GDScriptLanguage::debug_get_stack_level_source(int p_level) const { @@ -327,9 +325,8 @@ String GDScriptLanguage::debug_get_stack_level_source(int p_level) const { return _debug_parse_err_file; } - ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, ""); - int l = _call_stack.stack_pos - p_level - 1; - return _call_stack.levels[l].function->get_source(); + ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, ""); + return _get_stack_level(p_level)->function->get_source(); } void GDScriptLanguage::debug_get_stack_level_locals(int p_level, List *p_locals, List *p_values, int p_max_subitems, int p_max_depth) { @@ -337,17 +334,17 @@ void GDScriptLanguage::debug_get_stack_level_locals(int p_level, List *p return; } - ERR_FAIL_INDEX(p_level, _call_stack.stack_pos); - int l = _call_stack.stack_pos - p_level - 1; + ERR_FAIL_INDEX(p_level, (int)_call_stack_size); - GDScriptFunction *f = _call_stack.levels[l].function; + CallLevel *cl = _get_stack_level(p_level); + GDScriptFunction *f = cl->function; List> locals; - f->debug_get_stack_member_state(*_call_stack.levels[l].line, &locals); + f->debug_get_stack_member_state(*cl->line, &locals); for (const Pair &E : locals) { p_locals->push_back(E.first); - p_values->push_back(_call_stack.levels[l].stack[E.second]); + p_values->push_back(cl->stack[E.second]); } } @@ -356,10 +353,10 @@ void GDScriptLanguage::debug_get_stack_level_members(int p_level, List * return; } - ERR_FAIL_INDEX(p_level, _call_stack.stack_pos); - int l = _call_stack.stack_pos - p_level - 1; + ERR_FAIL_INDEX(p_level, (int)_call_stack_size); - GDScriptInstance *instance = _call_stack.levels[l].instance; + CallLevel *cl = _get_stack_level(p_level); + GDScriptInstance *instance = cl->instance; if (!instance) { return; @@ -381,12 +378,9 @@ ScriptInstance *GDScriptLanguage::debug_get_stack_level_instance(int p_level) { return nullptr; } - ERR_FAIL_INDEX_V(p_level, _call_stack.stack_pos, nullptr); + ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, nullptr); - int l = _call_stack.stack_pos - p_level - 1; - ScriptInstance *instance = _call_stack.levels[l].instance; - - return instance; + return _get_stack_level(p_level)->instance; } void GDScriptLanguage::debug_get_globals(List *p_globals, List *p_values, int p_max_subitems, int p_max_depth) { diff --git a/modules/gdscript/gdscript_function.cpp b/modules/gdscript/gdscript_function.cpp index e089b1b9c14..03c7e7df866 100644 --- a/modules/gdscript/gdscript_function.cpp +++ b/modules/gdscript/gdscript_function.cpp @@ -223,6 +223,7 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) { 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); } } @@ -231,14 +232,6 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) { state.result = Variant(); if (completed) { - if (first_state.is_valid()) { - first_state->emit_signal(SNAME("completed"), ret); - } else { - emit_signal(SNAME("completed"), ret); - } - - GDScriptLanguage::get_singleton()->exit_function(); - _clear_stack(); } diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index 438249d579c..7b454b856a1 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -575,6 +575,7 @@ public: static constexpr int MAX_CALL_DEPTH = 2048; // Limit to try to avoid crash because of a stack overflow. struct CallState { + Signal completed; GDScript *script = nullptr; GDScriptInstance *instance = nullptr; #ifdef DEBUG_ENABLED diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp index 00922560419..715505d8313 100644 --- a/modules/gdscript/gdscript_vm.cpp +++ b/modules/gdscript/gdscript_vm.cpp @@ -656,7 +656,8 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a String err_text; - GDScriptLanguage::get_singleton()->enter_function(p_instance, this, stack, &ip, &line); + GDScriptLanguage::CallLevel call_level; + GDScriptLanguage::get_singleton()->enter_function(&call_level, p_instance, this, stack, &ip, &line); #ifdef DEBUG_ENABLED #define GD_ERR_BREAK(m_cond) \ @@ -2548,7 +2549,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a // Is this even possible to be null at this point? if (obj) { if (obj->is_class_ptr(GDScriptFunctionState::get_class_ptr_static())) { - result = Signal(obj, "completed"); + result = Signal(obj, SNAME("completed")); } } } @@ -2595,6 +2596,13 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a gdfs->state.defarg = defarg; gdfs->function = this; + if (p_state) { + // Pass down the signal from the first state. + gdfs->state.completed = p_state->completed; + } else { + gdfs->state.completed = Signal(gdfs.ptr(), SNAME("completed")); + } + retvalue = gdfs; Error err = sig.connect(Callable(gdfs.ptr(), "_signal_callback").bind(retvalue), Object::CONNECT_ONE_SHOT); @@ -3980,5 +3988,16 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a call_depth--; + if (p_state && !awaited) { + // This means we have finished executing a resumed function and it was not awaited again. + + // Signal the next function-state to resume. + const Variant *args[1] = { &retvalue }; + p_state->completed.emit(args, 1); + + // Exit function only after executing the remaining function states to preserve async call stack. + GDScriptLanguage::get_singleton()->exit_function(); + } + return retvalue; }