소스 검색

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 <[email protected]>
Serhii Snitsaruk 3 달 전
부모
커밋
a095c5e3fa

+ 0 - 1
doc/classes/ProjectSettings.xml

@@ -661,7 +661,6 @@
 		</member>
 		<member name="debug/settings/gdscript/always_track_call_stacks" type="bool" setter="" getter="" default="false">
 			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.
 		</member>
 		<member name="debug/settings/gdscript/always_track_local_variables" type="bool" setter="" getter="" default="false">

+ 13 - 3
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);

+ 37 - 42
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 CallStack _call_stack;
-	int _debug_max_call_stack = 0;
+	static thread_local CallLevel *_call_stack;
+	static thread_local uint32_t _call_stack_size;
+	uint32_t _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<StackInfo> debug_get_current_stack_info() override {
 		Vector<StackInfo> 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;
 	}

+ 18 - 24
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<String> *p_locals, List<Variant> *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<String> *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<Pair<StringName, int>> 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<StringName, int> &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<String> *
 		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);
-
-	int l = _call_stack.stack_pos - p_level - 1;
-	ScriptInstance *instance = _call_stack.levels[l].instance;
+	ERR_FAIL_INDEX_V(p_level, (int)_call_stack_size, nullptr);
 
-	return instance;
+	return _get_stack_level(p_level)->instance;
 }
 
 void GDScriptLanguage::debug_get_globals(List<String> *p_globals, List<Variant> *p_values, int p_max_subitems, int p_max_depth) {

+ 1 - 8
modules/gdscript/gdscript_function.cpp

@@ -223,6 +223,7 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) {
 		GDScriptFunctionState *gdfs = Object::cast_to<GDScriptFunctionState>(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<GDScriptFunctionState>(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();
 	}
 

+ 1 - 0
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

+ 21 - 2
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;
 }