Merge pull request #71280 from RandomShaper/fix_deadlock_windows

Enhance thread-safety of loaders and importers (a.k.a. fix editor deadlock)

6 files changed, 20 insertions, 37 deletions

diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp
index a27341dd2c..f852e8d382 100644
--- a/core/io/resource_loader.cpp
+++ b/core/io/resource_loader.cpp
@@ -1136,10 +1136,7 @@ void ResourceLoader::initialize() {}
 void ResourceLoader::finalize() {}
 
 ResourceLoadErrorNotify ResourceLoader::err_notify = nullptr;
-void *ResourceLoader::err_notify_ud = nullptr;
-
 DependencyErrorNotify ResourceLoader::dep_err_notify = nullptr;
-void *ResourceLoader::dep_err_notify_ud = nullptr;
 
 bool ResourceLoader::create_missing_resources_if_class_unavailable = false;
 bool ResourceLoader::abort_on_missing_resource = true;
diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h
index ffe9d5de9a..592befb603 100644
--- a/core/io/resource_loader.h
+++ b/core/io/resource_loader.h
@@ -89,8 +89,8 @@ public:
 
 VARIANT_ENUM_CAST(ResourceFormatLoader::CacheMode)
 
-typedef void (*ResourceLoadErrorNotify)(void *p_ud, const String &p_text);
-typedef void (*DependencyErrorNotify)(void *p_ud, const String &p_loading, const String &p_which, const String &p_type);
+typedef void (*ResourceLoadErrorNotify)(const String &p_text);
+typedef void (*DependencyErrorNotify)(const String &p_loading, const String &p_which, const String &p_type);
 
 typedef Error (*ResourceLoaderImport)(const String &p_path);
 typedef void (*ResourceLoadedCallback)(Ref<Resource> p_resource, const String &p_path);
@@ -218,24 +218,24 @@ public:
 	static void set_timestamp_on_load(bool p_timestamp) { timestamp_on_load = p_timestamp; }
 	static bool get_timestamp_on_load() { return timestamp_on_load; }
 
+	// Loaders can safely use this regardless which thread they are running on.
 	static void notify_load_error(const String &p_err) {
 		if (err_notify) {
-			err_notify(err_notify_ud, p_err);
+			callable_mp_static(err_notify).bind(p_err).call_deferred();
 		}
 	}
-	static void set_error_notify_func(void *p_ud, ResourceLoadErrorNotify p_err_notify) {
+	static void set_error_notify_func(ResourceLoadErrorNotify p_err_notify) {
 		err_notify = p_err_notify;
-		err_notify_ud = p_ud;
 	}
 
+	// Loaders can safely use this regardless which thread they are running on.
 	static void notify_dependency_error(const String &p_path, const String &p_dependency, const String &p_type) {
 		if (dep_err_notify) {
-			dep_err_notify(dep_err_notify_ud, p_path, p_dependency, p_type);
+			callable_mp_static(dep_err_notify).bind(p_path, p_dependency, p_type).call_deferred();
 		}
 	}
-	static void set_dependency_error_notify_func(void *p_ud, DependencyErrorNotify p_err_notify) {
+	static void set_dependency_error_notify_func(DependencyErrorNotify p_err_notify) {
 		dep_err_notify = p_err_notify;
-		dep_err_notify_ud = p_ud;
 	}
 
 	static void set_abort_on_missing_resources(bool p_abort) { abort_on_missing_resource = p_abort; }
diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp
index 16128603b5..2d9f9645b8 100644
--- a/editor/editor_node.cpp
+++ b/editor/editor_node.cpp
@@ -4115,16 +4115,10 @@ void EditorNode::notify_all_debug_sessions_exited() {
 }
 
 void EditorNode::add_io_error(const String &p_error) {
-	_load_error_notify(singleton, p_error);
-}
-
-void EditorNode::_load_error_notify(void *p_ud, const String &p_text) {
-	EditorNode *en = static_cast<EditorNode *>(p_ud);
-	if (en && en->load_error_dialog) {
-		en->load_errors->add_image(en->gui_base->get_theme_icon(SNAME("Error"), SNAME("EditorIcons")));
-		en->load_errors->add_text(p_text + "\n");
-		en->load_error_dialog->attach_and_popup_centered_ratio(0.5);
-	}
+	DEV_ASSERT(Thread::get_caller_id() == Thread::get_main_id());
+	singleton->load_errors->add_image(singleton->gui_base->get_theme_icon(SNAME("Error"), SNAME("EditorIcons")));
+	singleton->load_errors->add_text(p_error + "\n");
+	singleton->load_error_dialog->attach_and_popup_centered_ratio(0.5);
 }
 
 bool EditorNode::_find_scene_in_use(Node *p_node, const String &p_path) const {
@@ -6731,8 +6725,8 @@ EditorNode::EditorNode() {
 	}
 
 	ResourceLoader::set_abort_on_missing_resources(false);
-	ResourceLoader::set_error_notify_func(this, _load_error_notify);
-	ResourceLoader::set_dependency_error_notify_func(this, _dependency_error_report);
+	ResourceLoader::set_error_notify_func(&EditorNode::add_io_error);
+	ResourceLoader::set_dependency_error_notify_func(&EditorNode::_dependency_error_report);
 
 	{
 		// Register importers at the beginning, so dialogs are created with the right extensions.
diff --git a/editor/editor_node.h b/editor/editor_node.h
index 9917fa16bc..221637be1c 100644
--- a/editor/editor_node.h
+++ b/editor/editor_node.h
@@ -504,12 +504,12 @@ private:
 	static int plugin_init_callback_count;
 	static Vector<EditorNodeInitCallback> _init_callbacks;
 
-	static void _dependency_error_report(void *ud, const String &p_path, const String &p_dep, const String &p_type) {
-		EditorNode *en = static_cast<EditorNode *>(ud);
-		if (!en->dependency_errors.has(p_path)) {
-			en->dependency_errors[p_path] = HashSet<String>();
+	static void _dependency_error_report(const String &p_path, const String &p_dep, const String &p_type) {
+		DEV_ASSERT(Thread::get_caller_id() == Thread::get_main_id());
+		if (!singleton->dependency_errors.has(p_path)) {
+			singleton->dependency_errors[p_path] = HashSet<String>();
 		}
-		en->dependency_errors[p_path].insert(p_dep + "::" + p_type);
+		singleton->dependency_errors[p_path].insert(p_dep + "::" + p_type);
 	}
 
 	static Ref<Texture2D> _file_dialog_get_icon(const String &p_path);
@@ -518,7 +518,6 @@ private:
 	static void _editor_file_dialog_register(EditorFileDialog *p_dialog);
 	static void _editor_file_dialog_unregister(EditorFileDialog *p_dialog);
 
-	static void _load_error_notify(void *p_ud, const String &p_text);
 	static void _file_access_close_error_notify(const String &p_str);
 
 	static void _print_handler(void *p_this, const String &p_string, bool p_error, bool p_rich);
diff --git a/editor/import/editor_import_collada.cpp b/editor/import/editor_import_collada.cpp
index 1ffede6502..5f714e4488 100644
--- a/editor/import/editor_import_collada.cpp
+++ b/editor/import/editor_import_collada.cpp
@@ -1782,15 +1782,8 @@ Node *EditorSceneFormatImporterCollada::import_scene(const String &p_path, uint3
 	ERR_FAIL_COND_V_MSG(err != OK, nullptr, "Cannot load scene from file '" + p_path + "'.");
 
 	if (state.missing_textures.size()) {
-		/*
-	for(int i=0;i<state.missing_textures.size();i++) {
-		EditorNode::add_io_error("Texture Not Found: "+state.missing_textures[i]);
-	}
-	*/
-
 		if (r_missing_deps) {
 			for (int i = 0; i < state.missing_textures.size(); i++) {
-				//EditorNode::add_io_error("Texture Not Found: "+state.missing_textures[i]);
 				r_missing_deps->push_back(state.missing_textures[i]);
 			}
 		}
diff --git a/editor/import/resource_importer_shader_file.cpp b/editor/import/resource_importer_shader_file.cpp
index ba48fc9029..1275e5b85a 100644
--- a/editor/import/resource_importer_shader_file.cpp
+++ b/editor/import/resource_importer_shader_file.cpp
@@ -106,7 +106,7 @@ Error ResourceImporterShaderFile::import(const String &p_source_file, const Stri
 
 	if (err != OK) {
 		if (!ShaderFileEditor::singleton->is_visible_in_tree()) {
-			EditorNode::get_singleton()->add_io_error(vformat(TTR("Error importing GLSL shader file: '%s'. Open the file in the filesystem dock in order to see the reason."), p_source_file));
+			callable_mp_static(&EditorNode::add_io_error).bind(vformat(TTR("Error importing GLSL shader file: '%s'. Open the file in the filesystem dock in order to see the reason."), p_source_file)).call_deferred();
 		}
 	}