godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.22k stars 21.22k forks source link

Deadlock during Godot startup #85202

Closed Malcolmnixon closed 12 months ago

Malcolmnixon commented 12 months ago

Godot version

7022271291a3d2a9cbd6a223d22a29fd775dfc5d

System information

Windows 11, gl_compatibility, NVidia RTX 3070 TI

Issue description

A deadlock can occur during application startup with the following:

The Worker Thread owns the DisplayServerWindows mutex and is trying to do a blocking SetWindowTextW call - a call which requires the Main Thread message pump to process; however the Main Thread is blocked trying to get hold of the very mutex locked by the worker thread.

Main Thread Call Stack:

godot.windows.editor.dev.x86_64.exe!MutexLock<MutexImpl<std::recursive_mutex>>::MutexLock<MutexImpl<std::recursive_mutex>>(const MutexImpl<std::recursive_mutex> & p_mutex) Line 129    C++
godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::_get_screens_origin() Line 663    C++
godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::mouse_get_position() Line 429 C++
godot.windows.editor.dev.x86_64.exe!Viewport::get_mouse_position() Line 1391    C++
godot.windows.editor.dev.x86_64.exe!CanvasItem::get_global_mouse_position() Line 1047   C++
godot.windows.editor.dev.x86_64.exe!CanvasItem::get_local_mouse_position() Line 1054    C++
godot.windows.editor.dev.x86_64.exe!EditorToaster::_notification(int p_what) Line 51    C++
godot.windows.editor.dev.x86_64.exe!EditorToaster::_notificationv(int p_notification, bool p_reversed) Line 43  C++
godot.windows.editor.dev.x86_64.exe!Object::notification(int p_notification, bool p_reversed) Line 840  C++
godot.windows.editor.dev.x86_64.exe!SceneTree::_process_group(SceneTree::ProcessGroup * p_group, bool p_physics) Line 950   C++
godot.windows.editor.dev.x86_64.exe!SceneTree::_process(bool p_physics) Line 1031   C++
godot.windows.editor.dev.x86_64.exe!SceneTree::process(double p_time) Line 510  C++
godot.windows.editor.dev.x86_64.exe!Main::iteration() Line 3636 C++
godot.windows.editor.dev.x86_64.exe!OS_Windows::run() Line 1474 C++
godot.windows.editor.dev.x86_64.exe!widechar_main(int argc, wchar_t * * argv) Line 182  C++
godot.windows.editor.dev.x86_64.exe!_main() Line 204    C++
godot.windows.editor.dev.x86_64.exe!main(int argc, char * * argv) Line 218  C++
godot.windows.editor.dev.x86_64.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 232  C++
[External Code] 

Worker Thread Call Stack:

godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::window_set_title(const String & p_title, int p_window) Line 1228  C++
godot.windows.editor.dev.x86_64.exe!EditorNode::_update_title() Line 303    C++
godot.windows.editor.dev.x86_64.exe!EditorNode::_update_from_settings() Line 349    C++
godot.windows.editor.dev.x86_64.exe!call_with_variant_args_helper<EditorNode>(EditorNode * p_instance, void(EditorNode::*)() p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<> __formal) Line 308  C++
godot.windows.editor.dev.x86_64.exe!call_with_variant_args<EditorNode>(EditorNode * p_instance, void(EditorNode::*)() p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 418   C++
godot.windows.editor.dev.x86_64.exe!CallableCustomMethodPointer<EditorNode>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105 C++
godot.windows.editor.dev.x86_64.exe!Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 58    C++
godot.windows.editor.dev.x86_64.exe!Object::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 1128 C++
godot.windows.editor.dev.x86_64.exe!Object::emit_signal<>(const StringName & p_name) Line 922   C++
godot.windows.editor.dev.x86_64.exe!ProjectSettings::_emit_changed() Line 458   C++
godot.windows.editor.dev.x86_64.exe!call_with_variant_args_helper<ProjectSettings>(ProjectSettings * p_instance, void(ProjectSettings::*)() p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<> __formal) Line 308   C++
godot.windows.editor.dev.x86_64.exe!call_with_variant_args<ProjectSettings>(ProjectSettings * p_instance, void(ProjectSettings::*)() p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 418    C++
godot.windows.editor.dev.x86_64.exe!CallableCustomMethodPointer<ProjectSettings>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105    C++
godot.windows.editor.dev.x86_64.exe!Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 58    C++
godot.windows.editor.dev.x86_64.exe!CallQueue::_call_function(const Callable & p_callable, const Variant * p_args, int p_argcount, bool p_show_error) Line 220  C++
godot.windows.editor.dev.x86_64.exe!CallQueue::flush() Line 326 C++
godot.windows.editor.dev.x86_64.exe!Main::iteration() Line 3615 C++
godot.windows.editor.dev.x86_64.exe!EditorNode::immediate_confirmation_dialog(const String & p_text, const String & p_ok_text, const String & p_cancel_text, unsigned int p_wrap_width) Line 5579   C++
godot.windows.editor.dev.x86_64.exe!SurfaceUpgradeTool::_show_popup() Line 89   C++
godot.windows.editor.dev.x86_64.exe!SurfaceUpgradeTool::_try_show_popup() Line 75   C++
godot.windows.editor.dev.x86_64.exe!RenderingServer::fix_surface_compatibility(RenderingServer::SurfaceData & p_surface, const String & p_path) Line 2138   C++
godot.windows.editor.dev.x86_64.exe!ArrayMesh::_set_surfaces(const Array & p_surfaces) Line 1643    C++
godot.windows.editor.dev.x86_64.exe!call_with_variant_args_helper<ArrayMesh,Array const &,0>(ArrayMesh * p_instance, void(ArrayMesh::*)(const Array &) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 303   C++
godot.windows.editor.dev.x86_64.exe!call_with_variant_args_dv<ArrayMesh,Array const &>(ArrayMesh * p_instance, void(ArrayMesh::*)(const Array &) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error, const Vector<Variant> & default_values) Line 451    C++
godot.windows.editor.dev.x86_64.exe!MethodBindT<ArrayMesh,Array const &>::call(Object * p_object, const Variant * * p_args, int p_arg_count, Callable::CallError & r_error) Line 337    C++
godot.windows.editor.dev.x86_64.exe!ClassDB::set_property(Object * p_object, const StringName & p_property, const Variant & p_value, bool * r_valid) Line 1235  C++
godot.windows.editor.dev.x86_64.exe!Object::set(const StringName & p_name, const Variant & p_value, bool * r_valid) Line 257    C++
godot.windows.editor.dev.x86_64.exe!ResourceLoaderBinary::load() Line 836   C++
godot.windows.editor.dev.x86_64.exe!ResourceFormatLoaderBinary::load(const String & p_path, const String & p_original_path, Error * r_error, bool p_use_sub_threads, float * r_progress, ResourceFormatLoader::CacheMode p_cache_mode) Line 1198    C++
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_load(const String & p_path, const String & p_original_path, const String & p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error * r_error, bool p_use_sub_threads, float * r_progress) Line 261    C++
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_thread_load_function(void * p_userdata) Line 319   C++
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_load_start(const String & p_path, const String & p_type_hint, ResourceLoader::LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode) Line 501 C++
godot.windows.editor.dev.x86_64.exe!ResourceLoader::load(const String & p_path, const String & p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error * r_error) Line 418 C++
godot.windows.editor.dev.x86_64.exe!EditorResourcePreviewGenerator::generate_from_path(const String & p_path, const Vector2 & p_size, Dictionary & p_metadata) Line 68  C++
godot.windows.editor.dev.x86_64.exe!EditorResourcePreview::_generate_preview(Ref<ImageTexture> & r_texture, Ref<ImageTexture> & r_small_texture, const EditorResourcePreview::QueueItem & p_item, const String & cache_base, Dictionary & p_metadata) Line 159  C++
godot.windows.editor.dev.x86_64.exe!EditorResourcePreview::_iterate() Line 251  C++
godot.windows.editor.dev.x86_64.exe!EditorResourcePreview::_thread() Line 338   C++
godot.windows.editor.dev.x86_64.exe!EditorResourcePreview::_thread_func(void * ud) Line 103 C++
godot.windows.editor.dev.x86_64.exe!Thread::callback(unsigned __int64 p_caller_id, const Thread::Settings & p_settings, void(*)(void *) p_callback, void * p_userdata) Line 63  C++
[External Code] 
godot.windows.editor.dev.x86_64.exe!thread_start<unsigned int (__cdecl*)(void *),1>(void * const parameter) Line 97 C++

Steps to reproduce

Happens when opening a project that has many meshes. This occurred opening https://github.com/Malcolmnixon/TiltFiveMapTest

Minimal reproduction project

It seems to be a russian-roulette with large projects - the project in the reproduce steps causes it.

akien-mga commented 12 months ago

CC @YuriSizov @bruvzg

More woes with EditorResourcePreview triggering the SurfaceUpgradeTool possibly too early?

bruvzg commented 12 months ago

trying to do a blocking SetWindowTextW call - a call which requires the Main Thread message pump to process

I guess we can defer the call if it's done for non-main thread, but there are likely other WinAPI functions that have the same issue, so this should be investigated further.

Something like:

diff --git a/platform/windows/display_server_windows.cpp b/platform/windows/display_server_windows.cpp
index e8d81405f0..b217a906f3 100644
--- a/platform/windows/display_server_windows.cpp
+++ b/platform/windows/display_server_windows.cpp
@@ -1221,13 +1221,19 @@ void DisplayServerWindows::window_set_drop_files_callback(const Callable &p_call
    windows[p_window].drop_files_callback = p_callable;
 }

-void DisplayServerWindows::window_set_title(const String &p_title, WindowID p_window) {
-   _THREAD_SAFE_METHOD_
-
+void DisplayServerWindows::_window_set_title(const String &p_title, WindowID p_window) {
    ERR_FAIL_COND(!windows.has(p_window));
    SetWindowTextW(windows[p_window].hWnd, (LPCWSTR)(p_title.utf16().get_data()));
 }

+void DisplayServerWindows::window_set_title(const String &p_title, WindowID p_window) {
+   if (!Thread::is_main_thread()) {
+       callable_mp((DisplayServerWindows *)get_singleton(), &DisplayServerWindows::_window_set_title).call_deferred(p_title, p_window);
+   } else {
+       _window_set_title(p_title, p_window);
+   }
+}
+
 Size2i DisplayServerWindows::window_get_title_size(const String &p_title, WindowID p_window) const {
    _THREAD_SAFE_METHOD_

diff --git a/platform/windows/display_server_windows.h b/platform/windows/display_server_windows.h
index 48c8c20280..2ab0af671c 100644
--- a/platform/windows/display_server_windows.h
+++ b/platform/windows/display_server_windows.h
@@ -492,6 +492,8 @@ class DisplayServerWindows : public DisplayServer {
    LRESULT _handle_early_window_message(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
    Point2i _get_screens_origin() const;

+   void _window_set_title(const String &p_title, WindowID p_window = MAIN_WINDOW_ID);
+
 public:
    LRESULT WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
    LRESULT MouseProc(int code, WPARAM wParam, LPARAM lParam);

Edit: This seems to be fixing deadlock, but test project seems to be always crashing (after a lot of physics server related errors).