Merge pull request #11873 from AdmiralCurtiss/pause-and-lock-host

Core: Assert that only the Host thread may call PauseAndLock().

9 files changed, 65 insertions, 32 deletions

diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp
index 14084c0406..30f0f5995e 100644
--- a/Source/Android/jni/MainAndroid.cpp
+++ b/Source/Android/jni/MainAndroid.cpp
@@ -74,6 +74,21 @@ ANativeWindow* s_surf;
 // If multiple threads want to call host functions then they need to queue
 // sequentially for access.
 std::mutex s_host_identity_lock;
+template <typename T>
+struct HostThreadWrapper
+{
+  T lock;
+
+  explicit HostThreadWrapper(auto&&... args) : lock(std::forward<decltype(args)>(args)...)
+  {
+    Core::DeclareAsHostThread();
+  }
+  HostThreadWrapper(const HostThreadWrapper& other) = delete;
+  HostThreadWrapper(HostThreadWrapper&& other) = delete;
+  HostThreadWrapper& operator=(const HostThreadWrapper& other) = delete;
+  HostThreadWrapper& operator=(HostThreadWrapper&& other) = delete;
+  ~HostThreadWrapper() { Core::UndeclareAsHostThread(); }
+};
 Common::Event s_update_main_frame_event;
 
 // This exists to prevent surfaces from being destroyed during the boot process,
@@ -248,19 +263,19 @@ extern "C" {
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_UnPauseEmulation(JNIEnv*,
                                                                                      jclass)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   Core::SetState(Core::State::Running);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulation(JNIEnv*, jclass)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   Core::SetState(Core::State::Paused);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv*, jclass)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   Core::Stop();
 
   // Kick the waiting event
@@ -303,7 +318,7 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetGitRev
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveScreenShot(JNIEnv*, jclass)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   Core::SaveScreenShot();
 }
 
@@ -317,7 +332,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveState(JN
                                                                               jint slot,
                                                                               jboolean wait)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   State::Save(slot, wait);
 }
 
@@ -325,21 +340,21 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveStateAs(
                                                                                 jstring path,
                                                                                 jboolean wait)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   State::SaveAs(GetJString(env, path), wait);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadState(JNIEnv*, jclass,
                                                                               jint slot)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   State::Load(slot);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadStateAs(JNIEnv* env, jclass,
                                                                                 jstring path)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   State::LoadAs(GetJString(env, path));
 }
 
@@ -359,7 +374,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_utils_DirectoryInitializat
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetUserDirectory(
     JNIEnv* env, jclass, jstring jDirectory)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   UICommon::SetUserDirectory(GetJString(env, jDirectory));
 }
 
@@ -372,7 +387,7 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetUserDi
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetCacheDirectory(
     JNIEnv* env, jclass, jstring jDirectory)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   File::SetUserPath(D_CACHE_IDX, GetJString(env, jDirectory));
 }
 
@@ -395,7 +410,7 @@ JNIEXPORT jint JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetMaxLogLev
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling(JNIEnv*, jclass,
                                                                                  jboolean enable)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   Core::SetState(Core::State::Paused);
   auto& jit_interface = Core::System::GetInstance().GetJitInterface();
   jit_interface.ClearCache();
@@ -407,7 +422,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_WriteProfileResults(JNIEnv*,
                                                                                         jclass)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   std::string filename = File::GetUserPath(D_DUMP_IDX) + "Debug/profiler.txt";
   File::CreateFullPath(filename);
   auto& jit_interface = Core::System::GetInstance().GetJitInterface();
@@ -436,14 +451,14 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestr
     // If emulation continues running without a valid surface, we will probably crash,
     // so pause emulation until we get a valid surface again. EmulationFragment handles resuming.
 
-    std::unique_lock host_identity_guard(s_host_identity_lock);
+    HostThreadWrapper<std::unique_lock<std::mutex>> host_identity_guard(s_host_identity_lock);
 
     while (s_is_booting.IsSet())
     {
       // Need to wait for boot to finish before we can pause
-      host_identity_guard.unlock();
+      host_identity_guard.lock.unlock();
       std::this_thread::sleep_for(std::chrono::milliseconds(1));
-      host_identity_guard.lock();
+      host_identity_guard.lock.lock();
     }
 
     if (Core::GetState() == Core::State::Running)
@@ -477,7 +492,7 @@ JNIEXPORT jfloat JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetGameAsp
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_RefreshWiimotes(JNIEnv*, jclass)
 {
-  std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
   WiimoteReal::Refresh();
 }
 
@@ -535,7 +550,7 @@ static float GetRenderSurfaceScale(JNIEnv* env)
 
 static void Run(JNIEnv* env, std::unique_ptr<BootParameters>&& boot, bool riivolution)
 {
-  std::unique_lock<std::mutex> host_identity_guard(s_host_identity_lock);
+  HostThreadWrapper<std::unique_lock<std::mutex>> host_identity_guard(s_host_identity_lock);
 
   if (riivolution && std::holds_alternative<BootParameters::Disc>(boot->parameters))
   {
@@ -566,15 +581,15 @@ static void Run(JNIEnv* env, std::unique_ptr<BootParameters>&& boot, bool riivol
 
   while (Core::IsRunning())
   {
-    host_identity_guard.unlock();
+    host_identity_guard.lock.unlock();
     s_update_main_frame_event.Wait();
-    host_identity_guard.lock();
+    host_identity_guard.lock.lock();
     Core::HostDispatchJobs();
   }
 
   s_game_metadata_is_valid = false;
   Core::Shutdown();
-  host_identity_guard.unlock();
+  host_identity_guard.lock.unlock();
 
   env->CallStaticVoidMethod(IDCache::GetNativeLibraryClass(),
                             IDCache::GetFinishEmulationActivity());
diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp
index 514863b5f8..c73fe46fd9 100644
--- a/Source/Core/Core/Core.cpp
+++ b/Source/Core/Core/Core.cpp
@@ -130,6 +130,7 @@ 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 thread_local bool tls_is_host_thread = false;
 
 static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi);
 
@@ -224,6 +225,11 @@ bool IsGPUThread()
   return tls_is_gpu_thread;
 }
 
+bool IsHostThread()
+{
+  return tls_is_host_thread;
+}
+
 bool WantsDeterminism()
 {
   return s_wants_determinism;
@@ -338,6 +344,16 @@ void UndeclareAsGPUThread()
   tls_is_gpu_thread = false;
 }
 
+void DeclareAsHostThread()
+{
+  tls_is_host_thread = true;
+}
+
+void UndeclareAsHostThread()
+{
+  tls_is_host_thread = false;
+}
+
 // For the CPU Thread only.
 static void CPUSetInitialExecutionState(bool force_paused = false)
 {
@@ -777,6 +793,8 @@ void SaveScreenShot(std::string_view name)
 static bool PauseAndLock(Core::System& system, bool do_lock, bool unpause_on_unlock)
 {
   // WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread
+  ASSERT(IsHostThread());
+
   if (!IsRunningAndStarted())
     return true;
 
diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h
index e237e25711..0ff8c32b5c 100644
--- a/Source/Core/Core/Core.h
+++ b/Source/Core/Core/Core.h
@@ -132,6 +132,8 @@ void DeclareAsCPUThread();
 void UndeclareAsCPUThread();
 void DeclareAsGPUThread();
 void UndeclareAsGPUThread();
+void DeclareAsHostThread();
+void UndeclareAsHostThread();
 
 std::string StopMessage(bool main_thread, std::string_view message);
 
@@ -140,6 +142,7 @@ bool IsRunningAndStarted();       // is running and the CPU loop has been entere
 bool IsRunningInCurrentThread();  // this tells us whether we are running in the CPU thread.
 bool IsCPUThread();               // this tells us whether we are the CPU thread.
 bool IsGPUThread();
+bool IsHostThread();
 
 bool WantsDeterminism();
 
diff --git a/Source/Core/DolphinNoGUI/MainNoGUI.cpp b/Source/Core/DolphinNoGUI/MainNoGUI.cpp
index 887181acb2..ff69e6bf7a 100644
--- a/Source/Core/DolphinNoGUI/MainNoGUI.cpp
+++ b/Source/Core/DolphinNoGUI/MainNoGUI.cpp
@@ -185,6 +185,8 @@ static std::unique_ptr<Platform> GetPlatform(const optparse::Values& options)
 
 int main(int argc, char* argv[])
 {
+  Core::DeclareAsHostThread();
+
   auto parser = CommandLineParse::CreateParser(CommandLineParse::ParserOptions::OmitGUIOptions);
   parser->add_option("-p", "--platform")
       .action("store")
diff --git a/Source/Core/DolphinQt/Host.cpp b/Source/Core/DolphinQt/Host.cpp
index 036dc2cffa..6a4939b106 100644
--- a/Source/Core/DolphinQt/Host.cpp
+++ b/Source/Core/DolphinQt/Host.cpp
@@ -59,16 +59,6 @@ Host* Host::GetInstance()
   return s_instance;
 }
 
-void Host::DeclareAsHostThread()
-{
-  tls_is_host_thread = true;
-}
-
-bool Host::IsHostThread()
-{
-  return tls_is_host_thread;
-}
-
 void Host::SetRenderHandle(void* handle)
 {
   m_render_to_main = Config::Get(Config::MAIN_RENDER_TO_MAIN);
diff --git a/Source/Core/DolphinQt/Main.cpp b/Source/Core/DolphinQt/Main.cpp
index 8fa1afff19..1c658e8d62 100644
--- a/Source/Core/DolphinQt/Main.cpp
+++ b/Source/Core/DolphinQt/Main.cpp
@@ -123,7 +123,7 @@ int main(int argc, char* argv[])
   }
 #endif
 
-  Host::GetInstance()->DeclareAsHostThread();
+  Core::DeclareAsHostThread();
 
 #ifdef __APPLE__
   // On macOS, a command line option matching the format "-psn_X_XXXXXX" is passed when
diff --git a/Source/Core/DolphinQt/Settings.cpp b/Source/Core/DolphinQt/Settings.cpp
index d915f4e916..c6a06336e2 100644
--- a/Source/Core/DolphinQt/Settings.cpp
+++ b/Source/Core/DolphinQt/Settings.cpp
@@ -71,7 +71,7 @@ Settings::Settings()
   });
 
   m_hotplug_callback_handle = g_controller_interface.RegisterDevicesChangedCallback([this] {
-    if (Host::GetInstance()->IsHostThread())
+    if (Core::IsHostThread())
     {
       emit DevicesChanged();
     }
diff --git a/Source/Core/DolphinTool/ToolMain.cpp b/Source/Core/DolphinTool/ToolMain.cpp
index 59613a17f6..263104a976 100644
--- a/Source/Core/DolphinTool/ToolMain.cpp
+++ b/Source/Core/DolphinTool/ToolMain.cpp
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "Common/Version.h"
+#include "Core/Core.h"
 #include "DolphinTool/Command.h"
 #include "DolphinTool/ConvertCommand.h"
 #include "DolphinTool/HeaderCommand.h"
@@ -27,6 +28,8 @@ static int PrintUsage(int code)
 
 int main(int argc, char* argv[])
 {
+  Core::DeclareAsHostThread();
+
   if (argc < 2)
     return PrintUsage(1);
 
diff --git a/Source/UnitTests/UnitTestsMain.cpp b/Source/UnitTests/UnitTestsMain.cpp
index aa4df43f48..4eed0c6099 100644
--- a/Source/UnitTests/UnitTestsMain.cpp
+++ b/Source/UnitTests/UnitTestsMain.cpp
@@ -6,6 +6,7 @@
 #include <fmt/format.h>
 
 #include "Common/MsgHandler.h"
+#include "Core/Core.h"
 
 #include "gtest/gtest.h"
 
@@ -24,6 +25,7 @@ int main(int argc, char** argv)
 {
   fmt::print(stderr, "Running main() from UnitTestsMain.cpp\n");
   Common::RegisterMsgAlertHandler(TestMsgHandler);
+  Core::DeclareAsHostThread();
 
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();