From 91f7b776ca51b06f33e72c92854a960c2fbaad8b Mon Sep 17 00:00:00 2001 From: Dentomologist Date: Tue, 24 Sep 2024 12:55:43 -0700 Subject: [PATCH 1/3] GraphicsWindow: Fix crash when opening during emulation startup Fix a crash when opening the Graphics window for the first time during emulation startup when the backend is Vulkan, D3D11, or D3D12. Don't call PopulateBackendInfo() from the Host thread when the core is starting up. First, the function has already been called in Core::Init() so we don't need to again. More importantly, PopulateBackendInfo() calls g_video_backend->InitBackendInfo(), and the Vulkan and D3D implementations of those functions load and then unload libraries (and their associated function pointers) which are potentially in use by other threads. This crash was reliably reproducible with the following steps: 1) Select an affected backend. 2) Enable "Compile Shaders Before Starting" 3) Delete the cached shaders (but not the .uidcache file) for the game you're testing. 4) Close and reopen Dolphin. 5) Start the game. 6) While the game is still booting or compiling shaders, open the Graphics window for the first time in that Dolphin session. Fixes https://bugs.dolphin-emu.org/issues/13634. --- Source/Core/VideoCommon/VideoBackendBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp index 1db7fb2ab6..0bbc734e3b 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.cpp +++ b/Source/Core/VideoCommon/VideoBackendBase.cpp @@ -300,7 +300,7 @@ void VideoBackendBase::PopulateBackendInfoFromUI(const WindowSystemInfo& wsi) { // If the core is running, the backend info will have been populated already. // If we did it here, the UI thread can race with the with the GPU thread. - if (!Core::IsRunning(Core::System::GetInstance())) + if (!Core::IsRunningOrStarting(Core::System::GetInstance())) PopulateBackendInfo(wsi); } From 2b82c34ea81c673da4cac8227277a01021730c4d Mon Sep 17 00:00:00 2001 From: Dentomologist Date: Thu, 26 Sep 2024 13:33:16 -0700 Subject: [PATCH 2/3] Core: Remove redundant call to PopulateBackendInfo Remove the second of two calls to PopulateBackendInfo during emulation startup. This call was originally the only one, but b214e0e added an earlier call to handle the backend being changed by the GameINI. The PR discussion doesn't explain why the original call was left in; I suspect it was just overlooked. As a bonus, this removes one of the extra copies of the "Video Info" On Screen Display message at startup when using OpenGL. --- Source/Core/Core/Core.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index bf850a253d..13508621b0 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -573,8 +573,6 @@ static void EmuThread(Core::System& system, std::unique_ptr boot system.GetPowerPC().GetDebugInterface().Clear(guard); }}; - VideoBackendBase::PopulateBackendInfo(wsi); - if (!g_video_backend->Initialize(wsi)) { PanicAlertFmt("Failed to initialize video backend!"); From 0a1084fad58ca6e340802021bd2e0d48ad7180ed Mon Sep 17 00:00:00 2001 From: Dentomologist Date: Thu, 26 Sep 2024 15:11:26 -0700 Subject: [PATCH 3/3] VideoBackendBase: Check Core state in PopulateBackendInfo Remove the PopulateBackendInfoFromUI function, which had a single caller (GraphicsWindow::OnBackendChanged) and checked that the core wasn't running or starting before calling PopulateBackendInfo. Move the core state check into PopulateBackendInfo and have OnBackendChanged call that instead. This guarantees the check is performed by all callers of PopulateBackendInfo, preventing potential reintroduction of the crash fixed in 3d4ae63f if another call to PopulateBackendInfo is added. As of the previous commit the only other caller of PopulateBackendInfo is Core::Init shortly before s_state is set to Starting, so it will always pass the check and so maintain its current behavior. --- .../DolphinQt/Config/Graphics/GraphicsWindow.cpp | 2 +- Source/Core/VideoCommon/VideoBackendBase.cpp | 13 +++++-------- Source/Core/VideoCommon/VideoBackendBase.h | 2 -- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp b/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp index 4edbea1594..627dd11b75 100644 --- a/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp +++ b/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp @@ -67,7 +67,7 @@ void GraphicsWindow::CreateMainLayout() void GraphicsWindow::OnBackendChanged(const QString& backend_name) { - VideoBackendBase::PopulateBackendInfoFromUI(m_main_window->GetWindowSystemInfo()); + VideoBackendBase::PopulateBackendInfo(m_main_window->GetWindowSystemInfo()); setWindowTitle( tr("%1 Graphics Configuration").arg(tr(g_video_backend->GetDisplayName().c_str()))); diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp index 0bbc734e3b..ab5e6c9fe6 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.cpp +++ b/Source/Core/VideoCommon/VideoBackendBase.cpp @@ -284,6 +284,11 @@ void VideoBackendBase::ActivateBackend(const std::string& name) void VideoBackendBase::PopulateBackendInfo(const WindowSystemInfo& wsi) { + // If the core is running, the backend info will have been populated already. If we did it here, + // the UI thread could race with the GPU thread. + if (Core::IsRunningOrStarting(Core::System::GetInstance())) + return; + g_Config.Refresh(); // Reset backend_info so if the backend forgets to initialize something it doesn't end up using // a value from the previously used renderer @@ -296,14 +301,6 @@ void VideoBackendBase::PopulateBackendInfo(const WindowSystemInfo& wsi) g_Config.VerifyValidity(); } -void VideoBackendBase::PopulateBackendInfoFromUI(const WindowSystemInfo& wsi) -{ - // If the core is running, the backend info will have been populated already. - // If we did it here, the UI thread can race with the with the GPU thread. - if (!Core::IsRunningOrStarting(Core::System::GetInstance())) - PopulateBackendInfo(wsi); -} - void VideoBackendBase::DoState(PointerWrap& p) { auto& system = Core::System::GetInstance(); diff --git a/Source/Core/VideoCommon/VideoBackendBase.h b/Source/Core/VideoCommon/VideoBackendBase.h index f5222a1936..5d9ab020d2 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.h +++ b/Source/Core/VideoCommon/VideoBackendBase.h @@ -70,8 +70,6 @@ public: // Fills the backend_info fields with the capabilities of the selected backend/device. static void PopulateBackendInfo(const WindowSystemInfo& wsi); - // Called by the UI thread when the graphics config is opened. - static void PopulateBackendInfoFromUI(const WindowSystemInfo& wsi); // Wrapper function which pushes the event to the GPU thread. void DoState(PointerWrap& p);