Support threads in the script debugger

* This implementation adds threads on the side of the client (script debugger).
* Some functions of the debugger are optimized.
* The profile is also now thread safe using atomics.
* The editor can switch between multiple threads when debugging.

This PR adds threaded support for the script language debugger. Every thread has its own thread local data and it will connect to the debugger using multiple thread IDs.
This means that, now, the editor can receive multiple threads entering debug mode at the same time.

core/debugger/debugger_marshalls.cpp

@@ -67,14 +67,15 @@ Array DebuggerMarshalls::ScriptStackVariable::serialize(int max_size) {
 	Array arr;
 	arr.push_back(name);
 	arr.push_back(type);
+	arr.push_back(value.get_type());
 
 	Variant var = value;
 	if (value.get_type() == Variant::OBJECT && value.get_validated_object() == nullptr) {
 		var = Variant();
 	}
 
 	int len = 0;
-	Error err = encode_variant(var, nullptr, len, true);
+	Error err = encode_variant(var, nullptr, len, false);
 	if (err != OK) {
 		ERR_PRINT("Failed to encode variant.");
 	}
@@ -88,11 +89,12 @@ Array DebuggerMarshalls::ScriptStackVariable::serialize(int max_size) {
 }
 
 bool DebuggerMarshalls::ScriptStackVariable::deserialize(const Array &p_arr) {
-	CHECK_SIZE(p_arr, 3, "ScriptStackVariable");
+	CHECK_SIZE(p_arr, 4, "ScriptStackVariable");
 	name = p_arr[0];
 	type = p_arr[1];
-	value = p_arr[2];
-	CHECK_END(p_arr, 3, "ScriptStackVariable");
+	var_type = p_arr[2];
+	value = p_arr[3];
+	CHECK_END(p_arr, 4, "ScriptStackVariable");
 	return true;
 }
 

core/debugger/debugger_marshalls.h

@@ -38,6 +38,7 @@ struct DebuggerMarshalls {
 		String name;
 		Variant value;
 		int type = -1;
+		int var_type = -1;
 
 		Array serialize(int max_size = 1 << 20); // 1 MiB default.
 		bool deserialize(const Array &p_arr);

core/debugger/engine_debugger.cpp

@@ -111,14 +111,6 @@ Error EngineDebugger::capture_parse(const StringName &p_name, const String &p_ms
 	return cap.capture(cap.data, p_msg, p_args, r_captured);
 }
 
-void EngineDebugger::line_poll() {
-	// The purpose of this is just processing events every now and then when the script might get too busy otherwise bugs like infinite loops can't be caught
-	if (poll_every % 2048 == 0) {
-		poll_events(false);
-	}
-	poll_every++;
-}
-
 void EngineDebugger::iteration(uint64_t p_frame_ticks, uint64_t p_process_ticks, uint64_t p_physics_ticks, double p_physics_frame_time) {
 	frame_time = USEC_TO_SEC(p_frame_ticks);
 	process_time = USEC_TO_SEC(p_process_ticks);

core/debugger/engine_debugger.h

@@ -126,7 +126,13 @@ class EngineDebugger {
 	void profiler_enable(const StringName &p_name, bool p_enabled, const Array &p_opts = Array());
 	Error capture_parse(const StringName &p_name, const String &p_msg, const Array &p_args, bool &r_captured);
 
-	void line_poll();
+	void line_poll() {
+		// The purpose of this is just processing events every now and then when the script might get too busy otherwise bugs like infinite loops can't be caught.
+		if (unlikely(poll_every % 2048) == 0) {
+			poll_events(false);
+		}
+		poll_every++;
+	}
 
 	virtual void poll_events(bool p_is_idle) {}
 	virtual void send_message(const String &p_msg, const Array &p_data) = 0;

core/debugger/remote_debugger.cpp

@@ -94,6 +94,7 @@ class RemoteDebugger::PerformanceProfiler : public EngineProfiler {
 Error RemoteDebugger::_put_msg(String p_message, Array p_data) {
 	Array msg;
 	msg.push_back(p_message);
+	msg.push_back(Thread::get_caller_id());
 	msg.push_back(p_data);
 	Error err = peer->put_message(msg);
 	if (err != OK) {
@@ -185,9 +186,9 @@ RemoteDebugger::ErrorMessage RemoteDebugger::_create_overflow_error(const String
 }
 
 void RemoteDebugger::flush_output() {
+	MutexLock lock(mutex);
 	flush_thread = Thread::get_caller_id();
 	flushing = true;
-	MutexLock lock(mutex);
 	if (!is_peer_connected()) {
 		return;
 	}
@@ -348,18 +349,65 @@ Error RemoteDebugger::_try_capture(const String &p_msg, const Array &p_data, boo
 	return capture_parse(cap, msg, p_data, r_captured);
 }
 
+void RemoteDebugger::_poll_messages() {
+	MutexLock mutex_lock(mutex);
+
+	peer->poll();
+	while (peer->has_message()) {
+		Array cmd = peer->get_message();
+		ERR_CONTINUE(cmd.size() != 3);
+		ERR_CONTINUE(cmd[0].get_type() != Variant::STRING);
+		ERR_CONTINUE(cmd[1].get_type() != Variant::INT);
+		ERR_CONTINUE(cmd[2].get_type() != Variant::ARRAY);
+
+		Thread::ID thread = cmd[1];
+
+		if (!messages.has(thread)) {
+			continue; // This thread is not around to receive the messages
+		}
+
+		Message msg;
+		msg.message = cmd[0];
+		msg.data = cmd[2];
+		messages[thread].push_back(msg);
+	}
+}
+
+bool RemoteDebugger::_has_messages() {
+	MutexLock mutex_lock(mutex);
+	return messages.has(Thread::get_caller_id()) && !messages[Thread::get_caller_id()].is_empty();
+}
+
+Array RemoteDebugger::_get_message() {
+	MutexLock mutex_lock(mutex);
+	ERR_FAIL_COND_V(!messages.has(Thread::get_caller_id()), Array());
+	List<Message> &message_list = messages[Thread::get_caller_id()];
+	ERR_FAIL_COND_V(message_list.is_empty(), Array());
+
+	Array msg;
+	msg.resize(2);
+	msg[0] = message_list.front()->get().message;
+	msg[1] = message_list.front()->get().data;
+	message_list.pop_front();
+	return msg;
+}
+
 void RemoteDebugger::debug(bool p_can_continue, bool p_is_error_breakpoint) {
 	//this function is called when there is a debugger break (bug on script)
 	//or when execution is paused from editor
 
-	if (script_debugger->is_skipping_breakpoints() && !p_is_error_breakpoint) {
-		return;
-	}
+	{
+		MutexLock lock(mutex);
+		// Tests that require mutex.
+		if (script_debugger->is_skipping_breakpoints() && !p_is_error_breakpoint) {
+			return;
+		}
 
-	ERR_FAIL_COND_MSG(!is_peer_connected(), "Script Debugger failed to connect, but being used anyway.");
+		ERR_FAIL_COND_MSG(!is_peer_connected(), "Script Debugger failed to connect, but being used anyway.");
 
-	if (!peer->can_block()) {
-		return; // Peer does not support blocking IO. We could at least send the error though.
+		if (!peer->can_block()) {
+			return; // Peer does not support blocking IO. We could at least send the error though.
+		}
 	}
 
 	ScriptLanguage *script_lang = script_debugger->get_break_language();
@@ -369,22 +417,33 @@ void RemoteDebugger::debug(bool p_can_continue, bool p_is_error_breakpoint) {
 	msg.push_back(error_str);
 	ERR_FAIL_COND(!script_lang);
 	msg.push_back(script_lang->debug_get_stack_level_count() > 0);
+	msg.push_back(Thread::get_caller_id() == Thread::get_main_id() ? String(RTR("Main Thread")) : itos(Thread::get_caller_id()));
 	if (allow_focus_steal_fn) {
 		allow_focus_steal_fn();
 	}
 	send_message("debug_enter", msg);
 
-	Input::MouseMode mouse_mode = Input::get_singleton()->get_mouse_mode();
-	if (mouse_mode != Input::MOUSE_MODE_VISIBLE) {
-		Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_VISIBLE);
+	Input::MouseMode mouse_mode = Input::MOUSE_MODE_VISIBLE;
+
+	if (Thread::get_caller_id() == Thread::get_main_id()) {
+		mouse_mode = Input::get_singleton()->get_mouse_mode();
+		if (mouse_mode != Input::MOUSE_MODE_VISIBLE) {
+			Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_VISIBLE);
+		}
+	} else {
+		MutexLock mutex_lock(mutex);
+		messages.insert(Thread::get_caller_id(), List<Message>());
 	}
 
+	mutex.lock();
 	while (is_peer_connected()) {
+		mutex.unlock();
 		flush_output();
-		peer->poll();
 
-		if (peer->has_message()) {
-			Array cmd = peer->get_message();
+		_poll_messages();
+
+		if (_has_messages()) {
+			Array cmd = _get_message();
 
 			ERR_CONTINUE(cmd.size() != 2);
 			ERR_CONTINUE(cmd[0].get_type() != Variant::STRING);
@@ -479,14 +538,22 @@ void RemoteDebugger::debug(bool p_can_continue, bool p_is_error_breakpoint) {
 			}
 		} else {
 			OS::get_singleton()->delay_usec(10000);
-			OS::get_singleton()->process_and_drop_events();
+			if (Thread::get_caller_id() == Thread::get_main_id()) {
+				// If this is a busy loop on the main thread, events still need to be processed.
+				OS::get_singleton()->process_and_drop_events();
+			}
 		}
 	}
 
 	send_message("debug_exit", Array());
 
-	if (mouse_mode != Input::MOUSE_MODE_VISIBLE) {
-		Input::get_singleton()->set_mouse_mode(mouse_mode);
+	if (Thread::get_caller_id() == Thread::get_main_id()) {
+		if (mouse_mode != Input::MOUSE_MODE_VISIBLE) {
+			Input::get_singleton()->set_mouse_mode(mouse_mode);
+		}
+	} else {
+		MutexLock mutex_lock(mutex);
+		messages.erase(Thread::get_caller_id());
 	}
 }
 
@@ -496,9 +563,11 @@ void RemoteDebugger::poll_events(bool p_is_idle) {
 	}
 
 	flush_output();
-	peer->poll();
-	while (peer->has_message()) {
-		Array arr = peer->get_message();
+
+	_poll_messages();
+
+	while (_has_messages()) {
+		Array arr = _get_message();
 
 		ERR_CONTINUE(arr.size() != 2);
 		ERR_CONTINUE(arr[0].get_type() != Variant::STRING);
@@ -604,6 +673,8 @@ RemoteDebugger::RemoteDebugger(Ref<RemoteDebuggerPeer> p_peer) {
 	eh.errfunc = _err_handler;
 	eh.userdata = this;
 	add_error_handler(&eh);
+
+	messages.insert(Thread::get_main_id(), List<Message>());
 }
 
 RemoteDebugger::~RemoteDebugger() {

core/debugger/remote_debugger.h

@@ -80,6 +80,17 @@ class RemoteDebugger : public EngineDebugger {
 	bool flushing = false;
 	Thread::ID flush_thread = 0;
 
+	struct Message {
+		String message;
+		Array data;
+	};
+
+	HashMap<Thread::ID, List<Message>> messages;
+
+	void _poll_messages();
+	bool _has_messages();
+	Array _get_message();
+
 	PrintHandlerList phl;
 	static void _print_handler(void *p_this, const String &p_string, bool p_error, bool p_rich);
 	ErrorHandlerList eh;

core/debugger/script_debugger.cpp

@@ -32,22 +32,19 @@
 
 #include "core/debugger/engine_debugger.h"
 
+thread_local int ScriptDebugger::lines_left = -1;
+thread_local int ScriptDebugger::depth = -1;
+thread_local ScriptLanguage *ScriptDebugger::break_lang = nullptr;
+thread_local Vector<ScriptDebugger::StackInfo> ScriptDebugger::error_stack_info;
+
 void ScriptDebugger::set_lines_left(int p_left) {
 	lines_left = p_left;
 }
 
-int ScriptDebugger::get_lines_left() const {
-	return lines_left;
-}
-
 void ScriptDebugger::set_depth(int p_depth) {
 	depth = p_depth;
 }
 
-int ScriptDebugger::get_depth() const {
-	return depth;
-}
-
 void ScriptDebugger::insert_breakpoint(int p_line, const StringName &p_source) {
 	if (!breakpoints.has(p_line)) {
 		breakpoints[p_line] = HashSet<StringName>();
@@ -66,13 +63,6 @@ void ScriptDebugger::remove_breakpoint(int p_line, const StringName &p_source) {
 	}
 }
 
-bool ScriptDebugger::is_breakpoint(int p_line, const StringName &p_source) const {
-	if (!breakpoints.has(p_line)) {
-		return false;
-	}
-	return breakpoints[p_line].has(p_source);
-}
-
 String ScriptDebugger::breakpoint_find_source(const String &p_source) const {
 	return p_source;
 }
@@ -100,7 +90,7 @@ void ScriptDebugger::send_error(const String &p_func, const String &p_file, int
 	// Store stack info, this is ugly, but allows us to separate EngineDebugger and ScriptDebugger. There might be a better way.
 	error_stack_info.append_array(p_stack_info);
 	EngineDebugger::get_singleton()->send_error(p_func, p_file, p_line, p_err, p_descr, p_editor_notify, p_type);
-	error_stack_info.clear();
+	error_stack_info.clear(); // Clear because this is thread local
 }
 
 Vector<ScriptLanguage::StackInfo> ScriptDebugger::get_error_stack_info() const {

core/debugger/script_debugger.h

@@ -40,21 +40,25 @@
 class ScriptDebugger {
 	typedef ScriptLanguage::StackInfo StackInfo;
 
-	int lines_left = -1;
-	int depth = -1;
 	bool skip_breakpoints = false;
 
 	HashMap<int, HashSet<StringName>> breakpoints;
 
-	ScriptLanguage *break_lang = nullptr;
-	Vector<StackInfo> error_stack_info;
+	static thread_local int lines_left;
+	static thread_local int depth;
+	static thread_local ScriptLanguage *break_lang;
+	static thread_local Vector<StackInfo> error_stack_info;
 
 public:
 	void set_lines_left(int p_left);
-	int get_lines_left() const;
+	_ALWAYS_INLINE_ int get_lines_left() const {
+		return lines_left;
+	}
 
 	void set_depth(int p_depth);
-	int get_depth() const;
+	_ALWAYS_INLINE_ int get_depth() const {
+		return depth;
+	}
 
 	String breakpoint_find_source(const String &p_source) const;
 	void set_break_language(ScriptLanguage *p_lang) { break_lang = p_lang; }
@@ -63,7 +67,12 @@ class ScriptDebugger {
 	bool is_skipping_breakpoints();
 	void insert_breakpoint(int p_line, const StringName &p_source);
 	void remove_breakpoint(int p_line, const StringName &p_source);
-	bool is_breakpoint(int p_line, const StringName &p_source) const;
+	_ALWAYS_INLINE_ bool is_breakpoint(int p_line, const StringName &p_source) const {
+		if (likely(!breakpoints.has(p_line))) {
+			return false;
+		}
+		return breakpoints[p_line].has(p_source);
+	}
 	void clear_breakpoints();
 	const HashMap<int, HashSet<StringName>> &get_breakpoints() const { return breakpoints; }
 

doc/classes/Thread.xml

@@ -5,7 +5,6 @@
 	</brief_description>
 	<description>
 		A unit of execution in a process. Can run methods on [Object]s simultaneously. The use of synchronization via [Mutex] or [Semaphore] is advised if working with shared objects.
-		[b]Note:[/b] Breakpoints won't break on code if it's running in a thread. This is a current limitation of the GDScript debugger.
 		[b]Warning:[/b]
 		To ensure proper cleanup without crashes or deadlocks, when a [Thread]'s reference count reaches zero and it is therefore destroyed, the following conditions must be met:
 		- It must not have any [Mutex] objects locked.

editor/debugger/editor_debugger_inspector.cpp

@@ -232,7 +232,7 @@ void EditorDebuggerInspector::add_stack_variable(const Array &p_array) {
 	PropertyHint h = PROPERTY_HINT_NONE;
 	String hs;
 
-	if (v.get_type() == Variant::OBJECT) {
+	if (var.var_type == Variant::OBJECT) {
 		v = Object::cast_to<EncodedObjectAsID>(v)->get_object_id();
 		h = PROPERTY_HINT_OBJECT_ID;
 		hs = "Object";