Core: Let any thread call previously host-thread-only functions

By letting threads other than the host thread use things like
CPUThreadGuard, we can do a significant cleanup in AchievementsManager
in a later commit of this pull request.

Note: Some functions still can't be called from the CPU thread (or
threads the CPU thread might block on, like the GPU thread), but can
be called from any other thread.

Source/Core/Core/Core.cpp

@@ -105,6 +105,11 @@ static bool s_is_throttler_temp_disabled = false;
 static bool s_frame_step = false;
 static std::atomic<bool> s_stop_frame_step;
 
+// Threads other than the CPU thread must hold this when taking on the role of the CPU thread.
+// The CPU thread is not required to hold this when doing normal work, but must hold it if writing
+// to s_state.
+static std::recursive_mutex s_core_mutex;
+
 // The value Paused is never stored in this variable. The core is considered to be in
 // the Paused state if this variable is Running and the CPU reports that it's stepping.
 static std::atomic<State> s_state = State::Uninitialized;
@@ -221,6 +226,8 @@ bool WantsDeterminism()
 // BootManager.cpp
 bool Init(Core::System& system, std::unique_ptr<BootParameters> boot, const WindowSystemInfo& wsi)
 {
+  std::lock_guard lock(s_core_mutex);
+
   if (s_emu_thread.joinable())
   {
     if (!IsUninitialized(system))
@@ -266,16 +273,20 @@ static void ResetRumble()
 // Called from GUI thread
 void Stop(Core::System& system)  // - Hammertime!
 {
-  const State state = s_state.load();
-  if (state == State::Stopping || state == State::Uninitialized)
-    return;
+  {
+    std::lock_guard lock(s_core_mutex);
 
-  AchievementManager::GetInstance().CloseGame();
+    const State state = s_state.load();
+    if (state == State::Stopping || state == State::Uninitialized)
+      return;
 
-  s_state.store(State::Stopping);
+    s_state.store(State::Stopping);
+  }
 
   NotifyStateChanged(State::Stopping);
 
+  AchievementManager::GetInstance().CloseGame();
+
   // Dump left over jobs
   HostDispatchJobs(system);
 
@@ -322,7 +333,7 @@ void UndeclareAsHostThread()
 static void CPUSetInitialExecutionState(bool force_paused = false)
 {
   // The CPU starts in stepping state, and will wait until a new state is set before executing.
-  // SetState must be called on the host thread, so we defer it for later.
+  // SetState isn't safe to call from the CPU thread, so we ask the host thread to call it.
   QueueHostJob([force_paused](Core::System& system) {
     bool paused = SConfig::GetInstance().bBootToPause || force_paused;
     SetState(system, paused ? State::Paused : State::Running, true, true);
@@ -362,10 +373,14 @@ static void CpuThread(Core::System& system, const std::optional<std::string>& sa
       File::Delete(*savestate_path);
   }
 
-  // If s_state is Starting, change it to Running. But if it's already been set to Stopping
-  // by the host thread, don't change it.
-  State expected = State::Starting;
-  s_state.compare_exchange_strong(expected, State::Running);
+  {
+    std::unique_lock core_lock(s_core_mutex);
+
+    // If s_state is Starting, change it to Running. But if it's already been set to Stopping
+    // because another thread called Stop, don't change it.
+    State expected = State::Starting;
+    s_state.compare_exchange_strong(expected, State::Running);
+  }
 
   {
 #ifndef _WIN32
@@ -423,12 +438,17 @@ static void FifoPlayerThread(Core::System& system, const std::optional<std::stri
   {
     system.GetPowerPC().InjectExternalCPUCore(cpu_core.get());
 
-    // If s_state is Starting, change it to Running. But if it's already been set to Stopping
-    // by the host thread, don't change it.
-    State expected = State::Starting;
-    s_state.compare_exchange_strong(expected, State::Running);
+    {
+      std::lock_guard core_lock(s_core_mutex);
+
+      // If s_state is Starting, change it to Running. But if it's already been set to Stopping
+      // because another thread called Stop, don't change it.
+      State expected = State::Starting;
+      s_state.compare_exchange_strong(expected, State::Running);
+    }
 
     CPUSetInitialExecutionState();
+
     system.GetCPU().Run();
 
     system.GetPowerPC().InjectExternalCPUCore(nullptr);
@@ -517,7 +537,10 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
 {
   NotifyStateChanged(State::Starting);
   Common::ScopeGuard flag_guard{[] {
-    s_state.store(State::Uninitialized);
+    {
+      std::lock_guard lock(s_core_mutex);
+      s_state.store(State::Uninitialized);
+    }
 
     NotifyStateChanged(State::Uninitialized);
 
@@ -682,35 +705,39 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
 void SetState(Core::System& system, State state, bool report_state_change,
               bool override_achievement_restrictions)
 {
-  // State cannot be controlled until the CPU Thread is operational
-  if (s_state.load() != State::Running)
-    return;
+  {
+    std::lock_guard lock(s_core_mutex);
 
-  switch (state)
-  {
-  case State::Paused:
-#ifdef USE_RETRO_ACHIEVEMENTS
-    if (!override_achievement_restrictions && !AchievementManager::GetInstance().CanPause())
+    // State cannot be controlled until the CPU Thread is operational
+    if (s_state.load() != State::Running)
       return;
-#endif  // USE_RETRO_ACHIEVEMENTS
-    // NOTE: GetState() will return State::Paused immediately, even before anything has
-    //   stopped (including the CPU).
-    system.GetCPU().SetStepping(true);  // Break
-    Wiimote::Pause();
-    ResetRumble();
+
+    switch (state)
+    {
+    case State::Paused:
 #ifdef USE_RETRO_ACHIEVEMENTS
-    AchievementManager::GetInstance().DoIdle();
+      if (!override_achievement_restrictions && !AchievementManager::GetInstance().CanPause())
+        return;
 #endif  // USE_RETRO_ACHIEVEMENTS
-    break;
-  case State::Running:
-  {
-    system.GetCPU().SetStepping(false);
-    Wiimote::Resume();
-    break;
-  }
-  default:
-    PanicAlertFmt("Invalid state");
-    break;
+      // NOTE: GetState() will return State::Paused immediately, even before anything has
+      //   stopped (including the CPU).
+      system.GetCPU().SetStepping(true);  // Break
+      Wiimote::Pause();
+      ResetRumble();
+#ifdef USE_RETRO_ACHIEVEMENTS
+      AchievementManager::GetInstance().DoIdle();
+#endif  // USE_RETRO_ACHIEVEMENTS
+      break;
+    case State::Running:
+    {
+      system.GetCPU().SetStepping(false);
+      Wiimote::Resume();
+      break;
+    }
+    default:
+      PanicAlertFmt("Invalid state");
+      break;
+    }
   }
 
   // Certain callers only change the state momentarily. Sending a callback for them causes
@@ -781,7 +808,7 @@ void SaveScreenShot(std::string_view name)
 
 static bool PauseAndLock(Core::System& system)
 {
-  // WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread
+  s_core_mutex.lock();
 
   if (!IsRunning(system))
     return true;
@@ -804,7 +831,7 @@ static bool PauseAndLock(Core::System& system)
 
 static void RestoreStateAndUnlock(Core::System& system, const bool unpause_on_unlock)
 {
-  // WARNING: RestoreStateAndUnlock is not fully threadsafe so is only valid on the Host Thread
+  Common::ScopeGuard scope_guard([] { s_core_mutex.unlock(); });
 
   if (!IsRunning(system))
     return;
@@ -824,8 +851,7 @@ static void RestoreStateAndUnlock(Core::System& system, const bool unpause_on_un
 void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> function,
                     bool wait_for_completion)
 {
-  // If the CPU thread is not running, assume there is no active CPU thread we can race against.
-  if (!IsRunning(system) || IsCPUThread())
+  if (IsCPUThread())
   {
     function();
     return;
@@ -834,10 +860,15 @@ void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> funct
   // Pause the CPU (set it to stepping mode).
   const bool was_running = PauseAndLock(system);
 
-  // Queue the job function.
-  if (wait_for_completion)
+  if (!IsRunning(system))
   {
-    // Trigger the event after executing the function.
+    // If the core hasn't been started, there is no active CPU thread we can race against.
+    function();
+    wait_for_completion = false;
+  }
+  else if (wait_for_completion)
+  {
+    // Queue the job function followed by triggering the event.
     s_cpu_thread_job_finished.Reset();
     system.GetCPU().AddCPUThreadJob([&function] {
       function();
@@ -846,6 +877,7 @@ void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> funct
   }
   else
   {
+    // Queue the job function.
     system.GetCPU().AddCPUThreadJob(std::move(function));
   }
 
@@ -967,6 +999,8 @@ void NotifyStateChanged(Core::State state)
 
 void UpdateWantDeterminism(Core::System& system, bool initial)
 {
+  const Core::CPUThreadGuard guard(system);
+
   // For now, this value is not itself configurable.  Instead, individual
   // settings that depend on it, such as GPU determinism mode. should have
   // override options for testing,
@@ -975,7 +1009,6 @@ void UpdateWantDeterminism(Core::System& system, bool initial)
   {
     NOTICE_LOG_FMT(COMMON, "Want determinism <- {}", new_want_determinism ? "true" : "false");
 
-    const Core::CPUThreadGuard guard(system);
     s_wants_determinism = new_want_determinism;
     const auto ios = system.GetIOS();
     if (ios)
@@ -1037,6 +1070,9 @@ void DoFrameStep(Core::System& system)
     OSD::AddMessage("Frame stepping is disabled in RetroAchievements hardcore mode");
     return;
   }
+
+  std::lock_guard lock(s_core_mutex);
+
   if (GetState(system) == State::Paused)
   {
     // if already paused, frame advance for 1 frame

Source/Core/Core/Core.h

@@ -91,10 +91,9 @@ enum class ConsoleType : u32
   ReservedTDEVSystem = 0x20000007,
 };
 
-// This is an RAII alternative to using PauseAndLock. If constructed from the host thread, the CPU
-// thread is paused, and the current thread temporarily becomes the CPU thread. If constructed from
-// the CPU thread, nothing special happens. This should only be constructed on the CPU thread or the
-// host thread.
+// This is an RAII alternative to using PauseAndLock. If constructed from any thread other than the
+// CPU thread, the CPU thread is paused, and the current thread temporarily becomes the CPU thread.
+// If constructed from the CPU thread, nothing special happens.
 //
 // Some functions use a parameter of this type to indicate that the function should only be called
 // from the CPU thread. If the parameter is a pointer, the function has a fallback for being called
@@ -118,6 +117,8 @@ private:
   bool m_was_unpaused = false;
 };
 
+// These three are normally called from the Host thread. However, they can be called from any thread
+// that isn't launched by the emulator core.
 bool Init(Core::System& system, std::unique_ptr<BootParameters> boot, const WindowSystemInfo& wsi);
 void Stop(Core::System& system);
 void Shutdown(Core::System& system);
@@ -144,7 +145,8 @@ bool IsHostThread();
 
 bool WantsDeterminism();
 
-// [NOT THREADSAFE] For use by Host only
+// SetState can't be called by the CPU thread, but can be called by any thread that isn't launched
+// by the emulator core.
 void SetState(Core::System& system, State state, bool report_state_change = true,
               bool override_achievement_restrictions = false);
 State GetState(Core::System& system);
@@ -159,7 +161,6 @@ void FrameUpdateOnCPUThread();
 void OnFrameEnd(Core::System& system);
 
 // Run a function on the CPU thread, asynchronously.
-// This is only valid to call from the host thread, since it uses PauseAndLock() internally.
 void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> function,
                     bool wait_for_completion);
 
@@ -171,7 +172,6 @@ int AddOnStateChangedCallback(StateChangedCallbackFunc callback);
 bool RemoveOnStateChangedCallback(int* handle);
 void NotifyStateChanged(Core::State state);
 
-// Run on the Host thread when the factors change. [NOT THREADSAFE]
 void UpdateWantDeterminism(Core::System& system, bool initial = false);
 
 // Queue an arbitrary function to asynchronously run once on the Host thread later.

Source/Core/DolphinQt/Host.cpp

@@ -8,6 +8,7 @@
 #include <QAbstractEventDispatcher>
 #include <QApplication>
 #include <QLocale>
+#include <QThread>
 
 #include <imgui.h>
 
@@ -245,7 +246,8 @@ bool Host_TASInputHasFocus()
 
 void Host_YieldToUI()
 {
-  qApp->processEvents(QEventLoop::ExcludeUserInputEvents);
+  if (qApp->thread() == QThread::currentThread())
+    qApp->processEvents(QEventLoop::ExcludeUserInputEvents);
 }
 
 void Host_UpdateDisasmDialog()