Merge pull request #71280 from RandomShaper/fix_deadlock_windows

Enhance thread-safety of loaders and importers (a.k.a. fix editor deadlock)
This commit is contained in:
Rémi Verschelde 2023-05-12 20:12:32 +02:00
commit ca26d9dc48
No known key found for this signature in database
GPG Key ID: C3336907360768E1
6 changed files with 20 additions and 37 deletions

View File

@ -1136,10 +1136,7 @@ void ResourceLoader::initialize() {}
void ResourceLoader::finalize() {} void ResourceLoader::finalize() {}
ResourceLoadErrorNotify ResourceLoader::err_notify = nullptr; ResourceLoadErrorNotify ResourceLoader::err_notify = nullptr;
void *ResourceLoader::err_notify_ud = nullptr;
DependencyErrorNotify ResourceLoader::dep_err_notify = 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::create_missing_resources_if_class_unavailable = false;
bool ResourceLoader::abort_on_missing_resource = true; bool ResourceLoader::abort_on_missing_resource = true;

View File

@ -89,8 +89,8 @@ public:
VARIANT_ENUM_CAST(ResourceFormatLoader::CacheMode) VARIANT_ENUM_CAST(ResourceFormatLoader::CacheMode)
typedef void (*ResourceLoadErrorNotify)(void *p_ud, const String &p_text); typedef void (*ResourceLoadErrorNotify)(const String &p_text);
typedef void (*DependencyErrorNotify)(void *p_ud, const String &p_loading, const String &p_which, const String &p_type); typedef void (*DependencyErrorNotify)(const String &p_loading, const String &p_which, const String &p_type);
typedef Error (*ResourceLoaderImport)(const String &p_path); typedef Error (*ResourceLoaderImport)(const String &p_path);
typedef void (*ResourceLoadedCallback)(Ref<Resource> p_resource, 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 void set_timestamp_on_load(bool p_timestamp) { timestamp_on_load = p_timestamp; }
static bool get_timestamp_on_load() { return timestamp_on_load; } 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) { static void notify_load_error(const String &p_err) {
if (err_notify) { 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 = 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) { static void notify_dependency_error(const String &p_path, const String &p_dependency, const String &p_type) {
if (dep_err_notify) { 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 = 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; } static void set_abort_on_missing_resources(bool p_abort) { abort_on_missing_resource = p_abort; }

View File

@ -4115,16 +4115,10 @@ void EditorNode::notify_all_debug_sessions_exited() {
} }
void EditorNode::add_io_error(const String &p_error) { void EditorNode::add_io_error(const String &p_error) {
_load_error_notify(singleton, p_error); 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");
void EditorNode::_load_error_notify(void *p_ud, const String &p_text) { singleton->load_error_dialog->attach_and_popup_centered_ratio(0.5);
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);
}
} }
bool EditorNode::_find_scene_in_use(Node *p_node, const String &p_path) const { 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_abort_on_missing_resources(false);
ResourceLoader::set_error_notify_func(this, _load_error_notify); ResourceLoader::set_error_notify_func(&EditorNode::add_io_error);
ResourceLoader::set_dependency_error_notify_func(this, _dependency_error_report); ResourceLoader::set_dependency_error_notify_func(&EditorNode::_dependency_error_report);
{ {
// Register importers at the beginning, so dialogs are created with the right extensions. // Register importers at the beginning, so dialogs are created with the right extensions.

View File

@ -504,12 +504,12 @@ private:
static int plugin_init_callback_count; static int plugin_init_callback_count;
static Vector<EditorNodeInitCallback> _init_callbacks; static Vector<EditorNodeInitCallback> _init_callbacks;
static void _dependency_error_report(void *ud, const String &p_path, const String &p_dep, const String &p_type) { static void _dependency_error_report(const String &p_path, const String &p_dep, const String &p_type) {
EditorNode *en = static_cast<EditorNode *>(ud); DEV_ASSERT(Thread::get_caller_id() == Thread::get_main_id());
if (!en->dependency_errors.has(p_path)) { if (!singleton->dependency_errors.has(p_path)) {
en->dependency_errors[p_path] = HashSet<String>(); 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); 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_register(EditorFileDialog *p_dialog);
static void _editor_file_dialog_unregister(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 _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); static void _print_handler(void *p_this, const String &p_string, bool p_error, bool p_rich);

View File

@ -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 + "'."); ERR_FAIL_COND_V_MSG(err != OK, nullptr, "Cannot load scene from file '" + p_path + "'.");
if (state.missing_textures.size()) { 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) { if (r_missing_deps) {
for (int i = 0; i < state.missing_textures.size(); i++) { 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]); r_missing_deps->push_back(state.missing_textures[i]);
} }
} }

View File

@ -106,7 +106,7 @@ Error ResourceImporterShaderFile::import(const String &p_source_file, const Stri
if (err != OK) { if (err != OK) {
if (!ShaderFileEditor::singleton->is_visible_in_tree()) { 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();
} }
} }