Merge pull request #8763 from JosJuice/panic-alert-deadlock-gpu

DolphinQt: Fix the panic alert deadlock, dual core edition

Source/Core/Core/Core.cpp

@@ -127,6 +127,7 @@ static std::queue<HostJob> s_host_jobs_queue;
 static Common::Event s_cpu_thread_job_finished;
 
 static thread_local bool tls_is_cpu_thread = false;
+static thread_local bool tls_is_gpu_thread = false;
 
 static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi);
 
@@ -203,14 +204,7 @@ bool IsCPUThread()
 
 bool IsGPUThread()
 {
-  if (Core::System::GetInstance().IsDualCoreMode())
-  {
-    return (s_emu_thread.joinable() && (s_emu_thread.get_id() == std::this_thread::get_id()));
-  }
-  else
-  {
-    return IsCPUThread();
-  }
+  return tls_is_gpu_thread;
 }
 
 bool WantsDeterminism()
@@ -313,6 +307,16 @@ void UndeclareAsCPUThread()
   tls_is_cpu_thread = false;
 }
 
+void DeclareAsGPUThread()
+{
+  tls_is_gpu_thread = true;
+}
+
+void UndeclareAsGPUThread()
+{
+  tls_is_gpu_thread = false;
+}
+
 // For the CPU Thread only.
 static void CPUSetInitialExecutionState(bool force_paused = false)
 {
@@ -459,6 +463,8 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
 
   Common::SetCurrentThreadName("Emuthread - Starting");
 
+  DeclareAsGPUThread();
+
   // For a time this acts as the CPU thread...
   DeclareAsCPUThread();
   s_frame_step = false;

Source/Core/Core/Core.h

@@ -98,6 +98,8 @@ void Shutdown();
 
 void DeclareAsCPUThread();
 void UndeclareAsCPUThread();
+void DeclareAsGPUThread();
+void UndeclareAsGPUThread();
 
 std::string StopMessage(bool main_thread, std::string_view message);
 

Source/Core/DolphinQt/Host.cpp

@@ -3,6 +3,8 @@
 
 #include "DolphinQt/Host.h"
 
+#include <functional>
+
 #include <QAbstractEventDispatcher>
 #include <QApplication>
 #include <QLocale>
@@ -34,6 +36,7 @@
 
 #include "UICommon/DiscordPresence.h"
 
+#include "VideoCommon/Fifo.cpp"
 #include "VideoCommon/RenderBase.h"
 #include "VideoCommon/VideoConfig.h"
 
@@ -85,6 +88,44 @@ void Host::SetMainWindowHandle(void* handle)
   m_main_window_handle = handle;
 }
 
+static void RunWithGPUThreadInactive(std::function<void()> f)
+{
+  // Potentially any thread which shows panic alerts can be blocked on this returning.
+  // This means that, in order to avoid deadlocks, we need to be careful with how we
+  // synchronize with other threads. Note that the panic alert handler temporarily declares
+  // us as the CPU and/or GPU thread if the panic alert was requested by that thread.
+
+  // TODO: What about the unlikely case where the GPU thread calls the panic alert handler
+  // while the panic alert handler is processing a panic alert from the CPU thread?
+
+  if (Core::IsGPUThread())
+  {
+    // If we are the GPU thread, we can't call Core::PauseAndLock without getting a deadlock,
+    // since it would try to pause the GPU thread while that thread is waiting for us.
+    // However, since we know that the GPU thread is inactive, we can just run f directly.
+
+    f();
+  }
+  else if (Core::IsCPUThread())
+  {
+    // If we are the CPU thread in dual core mode, we can't call Core::PauseAndLock, for the
+    // same reason as above. Instead, we use Fifo::PauseAndLock to pause the GPU thread only.
+    // (Note that this case cannot be reached in single core mode, because in single core mode,
+    // the CPU and GPU threads are the same thread, and we already checked for the GPU thread.)
+
+    const bool was_running = Core::GetState() == Core::State::Running;
+    Fifo::PauseAndLock(true, was_running);
+    f();
+    Fifo::PauseAndLock(false, was_running);
+  }
+  else
+  {
+    // If we reach here, we can call Core::PauseAndLock (which we do using RunAsCPUThread).
+
+    Core::RunAsCPUThread(std::move(f));
+  }
+}
+
 bool Host::GetRenderFocus()
 {
 #ifdef _WIN32
@@ -107,10 +148,12 @@ void Host::SetRenderFocus(bool focus)
 {
   m_render_focus = focus;
   if (g_renderer && m_render_fullscreen && g_ActiveConfig.ExclusiveFullscreenEnabled())
-    Core::RunAsCPUThread([focus] {
+  {
+    RunWithGPUThreadInactive([focus] {
       if (!Config::Get(Config::MAIN_RENDER_TO_MAIN))
         g_renderer->SetFullscreen(focus);
     });
+  }
 }
 
 void Host::SetRenderFullFocus(bool focus)
@@ -138,7 +181,9 @@ void Host::SetRenderFullscreen(bool fullscreen)
 
   if (g_renderer && g_renderer->IsFullscreen() != fullscreen &&
       g_ActiveConfig.ExclusiveFullscreenEnabled())
-    Core::RunAsCPUThread([fullscreen] { g_renderer->SetFullscreen(fullscreen); });
+  {
+    RunWithGPUThreadInactive([fullscreen] { g_renderer->SetFullscreen(fullscreen); });
+  }
 }
 
 void Host::ResizeSurface(int new_width, int new_height)

Source/Core/DolphinQt/Main.cpp

@@ -41,21 +41,26 @@ static bool QtMsgAlertHandler(const char* caption, const char* text, bool yes_no
                               Common::MsgType style)
 {
   const bool called_from_cpu_thread = Core::IsCPUThread();
+  const bool called_from_gpu_thread = Core::IsGPUThread();
 
   std::optional<bool> r = RunOnObject(QApplication::instance(), [&] {
-    Common::ScopeGuard scope_guard(&Core::UndeclareAsCPUThread);
+    // If we were called from the CPU/GPU thread, set us as the CPU/GPU thread.
+    // This information is used in order to avoid deadlocks when calling e.g.
+    // Host::SetRenderFocus or Core::RunAsCPUThread. (Host::SetRenderFocus
+    // can get called automatically when a dialog steals the focus.)
+
+    Common::ScopeGuard cpu_scope_guard(&Core::UndeclareAsCPUThread);
+    Common::ScopeGuard gpu_scope_guard(&Core::UndeclareAsGPUThread);
+
+    if (!called_from_cpu_thread)
+      cpu_scope_guard.Dismiss();
+    if (!called_from_gpu_thread)
+      gpu_scope_guard.Dismiss();
+
     if (called_from_cpu_thread)
-    {
-      // Temporarily declare this as the CPU thread to avoid getting a deadlock if any DolphinQt
-      // code calls RunAsCPUThread while the CPU thread is blocked on this function returning.
-      // Notably, if the panic alert steals focus from RenderWidget, Host::SetRenderFocus gets
-      // called, which can attempt to use RunAsCPUThread to get us out of exclusive fullscreen.
       Core::DeclareAsCPUThread();
-    }
-    else
-    {
-      scope_guard.Dismiss();
-    }
+    if (called_from_gpu_thread)
+      Core::DeclareAsGPUThread();
 
     ModalMessageBox message_box(QApplication::activeWindow(), Qt::ApplicationModal);
     message_box.setWindowTitle(QString::fromUtf8(caption));

Source/Core/DolphinQt/RenderWidget.cpp

@@ -421,10 +421,10 @@ bool RenderWidget::event(QEvent* event)
 
     if (Config::Get(Config::MAIN_PAUSE_ON_FOCUS_LOST) && Core::GetState() == Core::State::Running)
     {
-      // If we are declared as the CPU thread, it means that the real CPU thread is waiting
-      // for us to finish showing a panic alert (with that panic alert likely being the cause
-      // of this event), so trying to pause the real CPU thread would cause a deadlock
-      if (!Core::IsCPUThread())
+      // If we are declared as the CPU or GPU thread, it means that the real CPU or GPU thread
+      // is waiting for us to finish showing a panic alert (with that panic alert likely being
+      // the cause of this event), so trying to pause the core would cause a deadlock
+      if (!Core::IsCPUThread() && !Core::IsGPUThread())
         Core::SetState(Core::State::Paused);
     }