Merge pull request #4439 from Armada651/yield-ui

CPU: Fix deadlocks by periodically yielding to the UI message pump.

9 files changed, 80 insertions, 18 deletions

diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp
index d2fdf30bb6..4cd975fae8 100644
--- a/Source/Android/jni/MainAndroid.cpp
+++ b/Source/Android/jni/MainAndroid.cpp
@@ -148,6 +148,10 @@ void Host_ShowVideoConfig(void*, const std::string&)
 {
 }
 
+void Host_YieldToUI()
+{
+}
+
 static bool MsgAlert(const char* caption, const char* text, bool yes_no, int /*Style*/)
 {
   __android_log_print(ANDROID_LOG_ERROR, DOLPHIN_TAG, "%s:%s", caption, text);
diff --git a/Source/Core/Common/BlockingLoop.h b/Source/Core/Common/BlockingLoop.h
index da148fccd4..a1ebe7d708 100644
--- a/Source/Core/Common/BlockingLoop.h
+++ b/Source/Core/Common/BlockingLoop.h
@@ -66,6 +66,33 @@ public:
     m_may_sleep.Set();
   }
 
+  // Wait for a complete payload run after the last Wakeup() call.
+  // This version will call a yield function every 100ms.
+  // If stopped, this returns immediately.
+  template <class Rep, class Period, typename Functor>
+  void WaitYield(const std::chrono::duration<Rep, Period>& rel_time, Functor yield_func)
+  {
+    // already done
+    if (IsDone())
+      return;
+
+    // notifying this event will only wake up one thread, so use a mutex here to
+    // allow only one waiting thread. And in this way, we get an event free wakeup
+    // but for the first thread for free
+    std::lock_guard<std::mutex> lk(m_wait_lock);
+
+    // Wait for the worker thread to finish.
+    while (!IsDone())
+    {
+      if (!m_done_event.WaitFor(rel_time))
+        yield_func();
+    }
+
+    // As we wanted to wait for the other thread, there is likely no work remaining.
+    // So there is no need for a busy loop any more.
+    m_may_sleep.Set();
+  }
+
   // Half start the worker.
   // So this object is in a running state and Wait() will block until the worker calls Run().
   // This may be called from any thread and is supposed to be called at least once before Wait() is
diff --git a/Source/Core/Core/HW/CPU.cpp b/Source/Core/Core/HW/CPU.cpp
index 2c40689c9e..a790995f70 100644
--- a/Source/Core/Core/HW/CPU.cpp
+++ b/Source/Core/Core/HW/CPU.cpp
@@ -160,11 +160,15 @@ void Stop()
   std::unique_lock<std::mutex> state_lock(s_state_change_lock);
   s_state = CPU_POWERDOWN;
   s_state_cpu_cvar.notify_one();
-  // FIXME: MsgHandler can cause this to deadlock the GUI Thread. Remove the timeout.
-  bool success = s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::seconds(5),
-                                                [] { return !s_state_cpu_thread_active; });
-  if (!success)
-    ERROR_LOG(POWERPC, "CPU Thread failed to acknowledge CPU_POWERDOWN. It may be deadlocked.");
+
+  while (s_state_cpu_thread_active)
+  {
+    std::cv_status status =
+        s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::milliseconds(100));
+    if (status == std::cv_status::timeout)
+      Host_YieldToUI();
+  }
+
   RunAdjacentSystems(false);
   FlushStepSyncEventLocked();
 }
@@ -226,12 +230,13 @@ void EnableStepping(bool stepping)
   {
     SetStateLocked(CPU_STEPPING);
 
-    // Wait for the CPU Thread to leave the run loop
-    // FIXME: MsgHandler can cause this to deadlock the GUI Thread. Remove the timeout.
-    bool success = s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::seconds(5),
-                                                  [] { return !s_state_cpu_thread_active; });
-    if (!success)
-      ERROR_LOG(POWERPC, "Abandoned waiting for CPU Thread! The Core may be deadlocked.");
+    while (s_state_cpu_thread_active)
+    {
+      std::cv_status status =
+          s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::milliseconds(100));
+      if (status == std::cv_status::timeout)
+        Host_YieldToUI();
+    }
 
     RunAdjacentSystems(false);
   }
@@ -276,12 +281,13 @@ bool PauseAndLock(bool do_lock, bool unpause_on_unlock, bool control_adjacent)
     was_unpaused = s_state == CPU_RUNNING;
     SetStateLocked(CPU_STEPPING);
 
-    // FIXME: MsgHandler can cause this to deadlock the GUI Thread. Remove the timeout.
-    bool success = s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::seconds(10),
-                                                  [] { return !s_state_cpu_thread_active; });
-    if (!success)
-      PanicAlert(
-          "Abandoned CPU Thread synchronization in CPU::PauseAndLock! We'll probably crash now.");
+    while (s_state_cpu_thread_active)
+    {
+      std::cv_status status =
+          s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::milliseconds(100));
+      if (status == std::cv_status::timeout)
+        Host_YieldToUI();
+    }
 
     if (control_adjacent)
       RunAdjacentSystems(false);
diff --git a/Source/Core/Core/Host.h b/Source/Core/Core/Host.h
index c739f0dca1..2f517c4c8e 100644
--- a/Source/Core/Core/Host.h
+++ b/Source/Core/Core/Host.h
@@ -38,6 +38,7 @@ void Host_UpdateDisasmDialog();
 void Host_UpdateMainFrame();
 void Host_UpdateTitle(const std::string& title);
 void Host_ShowVideoConfig(void* parent, const std::string& backend_name);
+void Host_YieldToUI();
 
 // TODO (neobrain): Remove this from host!
 void* Host_GetRenderHandle();
diff --git a/Source/Core/DolphinQt2/Host.cpp b/Source/Core/DolphinQt2/Host.cpp
index 62af45b489..906853bc3c 100644
--- a/Source/Core/DolphinQt2/Host.cpp
+++ b/Source/Core/DolphinQt2/Host.cpp
@@ -89,6 +89,10 @@ bool Host_RendererIsFullscreen()
 {
   return Host::GetInstance()->GetRenderFullscreen();
 }
+void Host_YieldToUI()
+{
+  qApp->processEvents(QEventLoop::ExcludeUserInputEvents);
+}
 
 // We ignore these, and their purpose should be questioned individually.
 // In particular, RequestRenderWindowSize, RequestFullscreen, and
diff --git a/Source/Core/DolphinWX/Main.cpp b/Source/Core/DolphinWX/Main.cpp
index 871e89ad28..444b270d39 100644
--- a/Source/Core/DolphinWX/Main.cpp
+++ b/Source/Core/DolphinWX/Main.cpp
@@ -10,6 +10,7 @@
 #include <wx/app.h>
 #include <wx/buffer.h>
 #include <wx/cmdline.h>
+#include <wx/evtloop.h>
 #include <wx/image.h>
 #include <wx/imagpng.h>
 #include <wx/intl.h>
@@ -547,3 +548,8 @@ void Host_ShowVideoConfig(void* parent, const std::string& backend_name)
     diag.ShowModal();
   }
 }
+
+void Host_YieldToUI()
+{
+  wxGetApp().GetMainLoop()->YieldFor(wxEVT_CATEGORY_UI);
+}
diff --git a/Source/Core/DolphinWX/MainNoGUI.cpp b/Source/Core/DolphinWX/MainNoGUI.cpp
index 216eb5a707..6c8a53b88d 100644
--- a/Source/Core/DolphinWX/MainNoGUI.cpp
+++ b/Source/Core/DolphinWX/MainNoGUI.cpp
@@ -161,6 +161,10 @@ void Host_ShowVideoConfig(void*, const std::string&)
 {
 }
 
+void Host_YieldToUI()
+{
+}
+
 #if HAVE_X11
 #include <X11/keysym.h>
 #include "DolphinWX/X11Utils.h"
diff --git a/Source/Core/VideoCommon/Fifo.cpp b/Source/Core/VideoCommon/Fifo.cpp
index 374ddb07f3..678dc77687 100644
--- a/Source/Core/VideoCommon/Fifo.cpp
+++ b/Source/Core/VideoCommon/Fifo.cpp
@@ -18,6 +18,7 @@
 #include "Core/CoreTiming.h"
 #include "Core/HW/Memmap.h"
 #include "Core/HW/SystemTimers.h"
+#include "Core/Host.h"
 #include "Core/NetPlayProto.h"
 
 #include "VideoCommon/AsyncRequests.h"
@@ -94,7 +95,13 @@ void PauseAndLock(bool doLock, bool unpauseOnUnlock)
   {
     SyncGPU(SyncGPUReason::Other);
     EmulatorState(false);
-    FlushGpu();
+
+    const SConfig& param = SConfig::GetInstance();
+
+    if (!param.bCPUThread || s_use_deterministic_gpu_thread)
+      return;
+
+    s_gpu_mainloop.WaitYield(std::chrono::milliseconds(100), Host_YieldToUI);
   }
   else
   {
diff --git a/Source/UnitTests/TestUtils/StubHost.cpp b/Source/UnitTests/TestUtils/StubHost.cpp
index c6d1cbb12e..e32561da90 100644
--- a/Source/UnitTests/TestUtils/StubHost.cpp
+++ b/Source/UnitTests/TestUtils/StubHost.cpp
@@ -63,6 +63,9 @@ void Host_SetWiiMoteConnectionState(int)
 void Host_ShowVideoConfig(void*, const std::string&)
 {
 }
+void Host_YieldToUI()
+{
+}
 std::unique_ptr<cInterfaceBase> HostGL_CreateGLInterface()
 {
   return nullptr;