Merge pull request #11926 from JosJuice/android-host-check

Android: Re-add host thread check

Source/Android/jni/CMakeLists.txt

@@ -10,6 +10,8 @@ add_library(main SHARED
   GameList/GameFile.cpp
   GameList/GameFile.h
   GameList/GameFileCache.cpp
+  Host.cpp
+  Host.h
   InfinityConfig.cpp
   Input/Control.cpp
   Input/Control.h

Source/Android/jni/Config/NativeConfig.cpp

@@ -11,6 +11,7 @@
 #include "Core/ConfigLoaders/GameConfigLoader.h"
 #include "Core/ConfigLoaders/IsSettingSaveable.h"
 #include "jni/AndroidCommon/AndroidCommon.h"
+#include "jni/Host.h"
 
 constexpr jint LAYER_BASE_OR_CURRENT = 0;
 constexpr jint LAYER_BASE = 1;
@@ -122,6 +123,7 @@ Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_loadGameInis
                                                                                  jstring jGameId,
                                                                                  jint jRevision)
 {
+  HostThreadLock guard;
   const std::string game_id = GetJString(env, jGameId);
   const u16 revision = static_cast<u16>(jRevision);
   Config::AddLayer(ConfigLoaders::GenerateGlobalGameConfigLoader(game_id, revision));
@@ -131,20 +133,23 @@ Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_loadGameInis
 JNIEXPORT void JNICALL
 Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_unloadGameInis(JNIEnv*, jclass)
 {
+  HostThreadLock guard;
   Config::RemoveLayer(Config::LayerType::GlobalGame);
   Config::RemoveLayer(Config::LayerType::LocalGame);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_save(
     JNIEnv*, jclass, jint layer)
 {
+  HostThreadLock guard;
   return GetLayer(layer, {})->Save();
 }
 
 JNIEXPORT void JNICALL
 Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_deleteAllKeys(JNIEnv*, jclass,
                                                                                   jint layer)
 {
+  HostThreadLock guard;
   return GetLayer(layer, {})->DeleteAllKeys();
 }
 
@@ -161,6 +166,7 @@ JNIEXPORT jboolean JNICALL
 Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_deleteKey(
     JNIEnv* env, jclass, jint layer, jstring file, jstring section, jstring key)
 {
+  HostThreadLock guard;
   const Config::Location location = GetLocation(env, file, section, key);
   const bool had_value = GetLayer(layer, location)->DeleteKey(location);
   if (had_value)
@@ -214,25 +220,29 @@ JNIEXPORT void JNICALL
 Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_setString(
     JNIEnv* env, jclass, jint layer, jstring file, jstring section, jstring key, jstring value)
 {
+  HostThreadLock guard;
   return Set(layer, GetLocation(env, file, section, key), GetJString(env, value));
 }
 
 JNIEXPORT void JNICALL
 Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_setBoolean(
     JNIEnv* env, jclass, jint layer, jstring file, jstring section, jstring key, jboolean value)
 {
+  HostThreadLock guard;
   return Set(layer, GetLocation(env, file, section, key), static_cast<bool>(value));
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_setInt(
     JNIEnv* env, jclass, jint layer, jstring file, jstring section, jstring key, jint value)
 {
+  HostThreadLock guard;
   return Set(layer, GetLocation(env, file, section, key), value);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_features_settings_model_NativeConfig_setFloat(
     JNIEnv* env, jclass, jint layer, jstring file, jstring section, jstring key, jfloat value)
 {
+  HostThreadLock guard;
   return Set(layer, GetLocation(env, file, section, key), value);
 }
 }

Source/Android/jni/Host.cpp

@@ -0,0 +1,8 @@
+// Copyright 2023 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "jni/Host.h"
+
+#include <mutex>
+
+std::mutex HostThreadLock::s_host_identity_mutex;

Source/Android/jni/Host.h

@@ -0,0 +1,44 @@
+// Copyright 2023 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include <mutex>
+
+#include "Core/Core.h"
+
+// The Core only supports using a single Host thread.
+// If multiple threads want to call host functions then they need to queue
+// sequentially for access.
+struct HostThreadLock
+{
+public:
+  explicit HostThreadLock() : m_lock(s_host_identity_mutex) { Core::DeclareAsHostThread(); }
+
+  ~HostThreadLock()
+  {
+    if (m_lock.owns_lock())
+      Core::UndeclareAsHostThread();
+  }
+
+  HostThreadLock(const HostThreadLock& other) = delete;
+  HostThreadLock(HostThreadLock&& other) = delete;
+  HostThreadLock& operator=(const HostThreadLock& other) = delete;
+  HostThreadLock& operator=(HostThreadLock&& other) = delete;
+
+  void Lock()
+  {
+    m_lock.lock();
+    Core::DeclareAsHostThread();
+  }
+
+  void Unlock()
+  {
+    m_lock.unlock();
+    Core::UndeclareAsHostThread();
+  }
+
+private:
+  static std::mutex s_host_identity_mutex;
+  std::unique_lock<std::mutex> m_lock;
+};

Source/Android/jni/MainAndroid.cpp

@@ -63,32 +63,14 @@
 
 #include "jni/AndroidCommon/AndroidCommon.h"
 #include "jni/AndroidCommon/IDCache.h"
+#include "jni/Host.h"
 
 namespace
 {
 constexpr char DOLPHIN_TAG[] = "DolphinEmuNative";
 
 ANativeWindow* s_surf;
 
-// The Core only supports using a single Host thread.
-// 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,
@@ -263,19 +245,19 @@ extern "C" {
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_UnPauseEmulation(JNIEnv*,
                                                                                      jclass)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   Core::SetState(Core::State::Running);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulation(JNIEnv*, jclass)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   Core::SetState(Core::State::Paused);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv*, jclass)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   Core::Stop();
 
   // Kick the waiting event
@@ -318,7 +300,7 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetGitRev
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveScreenShot(JNIEnv*, jclass)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   Core::SaveScreenShot();
 }
 
@@ -332,29 +314,29 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveState(JN
                                                                               jint slot,
                                                                               jboolean wait)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   State::Save(slot, wait);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveStateAs(JNIEnv* env, jclass,
                                                                                 jstring path,
                                                                                 jboolean wait)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   State::SaveAs(GetJString(env, path), wait);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadState(JNIEnv*, jclass,
                                                                               jint slot)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   State::Load(slot);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadStateAs(JNIEnv* env, jclass,
                                                                                 jstring path)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   State::LoadAs(GetJString(env, path));
 }
 
@@ -383,7 +365,7 @@ Java_org_dolphinemu_dolphinemu_utils_DirectoryInitialization_SetGpuDriverDirecto
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetUserDirectory(
     JNIEnv* env, jclass, jstring jDirectory)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   UICommon::SetUserDirectory(GetJString(env, jDirectory));
 }
 
@@ -396,7 +378,7 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetUserDi
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetCacheDirectory(
     JNIEnv* env, jclass, jstring jDirectory)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   File::SetUserPath(D_CACHE_IDX, GetJString(env, jDirectory));
 }
 
@@ -419,7 +401,7 @@ JNIEXPORT jint JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetMaxLogLev
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling(JNIEnv*, jclass,
                                                                                  jboolean enable)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   Core::SetState(Core::State::Paused);
   auto& jit_interface = Core::System::GetInstance().GetJitInterface();
   jit_interface.ClearCache();
@@ -431,7 +413,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_WriteProfileResults(JNIEnv*,
                                                                                         jclass)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   std::string filename = File::GetUserPath(D_DUMP_IDX) + "Debug/profiler.txt";
   File::CreateFullPath(filename);
   auto& jit_interface = Core::System::GetInstance().GetJitInterface();
@@ -460,14 +442,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.
 
-    HostThreadWrapper<std::unique_lock<std::mutex>> host_identity_guard(s_host_identity_lock);
+    HostThreadLock host_identity_guard;
 
     while (s_is_booting.IsSet())
     {
       // Need to wait for boot to finish before we can pause
-      host_identity_guard.lock.unlock();
+      host_identity_guard.Unlock();
       std::this_thread::sleep_for(std::chrono::milliseconds(1));
-      host_identity_guard.lock.lock();
+      host_identity_guard.Lock();
     }
 
     if (Core::GetState() == Core::State::Running)
@@ -501,18 +483,20 @@ JNIEXPORT jfloat JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetGameAsp
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_RefreshWiimotes(JNIEnv*, jclass)
 {
-  HostThreadWrapper<std::lock_guard<std::mutex>> guard(s_host_identity_lock);
+  HostThreadLock guard;
   WiimoteReal::Refresh();
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ReloadConfig(JNIEnv*, jclass)
 {
+  HostThreadLock guard;
   SConfig::GetInstance().LoadSettings();
 }
 
 JNIEXPORT void JNICALL
 Java_org_dolphinemu_dolphinemu_NativeLibrary_UpdateGCAdapterScanThread(JNIEnv*, jclass)
 {
+  HostThreadLock guard;
   if (GCAdapter::UseAdapter())
   {
     GCAdapter::StartScanThread();
@@ -525,6 +509,9 @@ Java_org_dolphinemu_dolphinemu_NativeLibrary_UpdateGCAdapterScanThread(JNIEnv*,
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Initialize(JNIEnv*, jclass)
 {
+  // InitControllers ends up calling config code, and some config callbacks use RunAsCPUThread
+  HostThreadLock guard;
+
   UICommon::CreateDirectories();
   Common::RegisterMsgAlertHandler(&MsgAlert);
   Common::AndroidSetReportHandler(&ReportSend);
@@ -559,7 +546,7 @@ static float GetRenderSurfaceScale(JNIEnv* env)
 
 static void Run(JNIEnv* env, std::unique_ptr<BootParameters>&& boot, bool riivolution)
 {
-  HostThreadWrapper<std::unique_lock<std::mutex>> host_identity_guard(s_host_identity_lock);
+  HostThreadLock host_identity_guard;
 
   if (riivolution && std::holds_alternative<BootParameters::Disc>(boot->parameters))
   {
@@ -590,15 +577,15 @@ static void Run(JNIEnv* env, std::unique_ptr<BootParameters>&& boot, bool riivol
 
   while (Core::IsRunning())
   {
-    host_identity_guard.lock.unlock();
+    host_identity_guard.Unlock();
     s_update_main_frame_event.Wait();
-    host_identity_guard.lock.lock();
+    host_identity_guard.Lock();
     Core::HostDispatchJobs();
   }
 
   s_game_metadata_is_valid = false;
   Core::Shutdown();
-  host_identity_guard.lock.unlock();
+  host_identity_guard.Unlock();
 
   env->CallStaticVoidMethod(IDCache::GetNativeLibraryClass(),
                             IDCache::GetFinishEmulationActivity());
@@ -639,6 +626,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_RunSystemMen
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ChangeDisc(JNIEnv* env, jclass,
                                                                                jstring jFile)
 {
+  HostThreadLock guard;
   const std::string path = GetJString(env, jFile);
   __android_log_print(ANDROID_LOG_INFO, DOLPHIN_TAG, "Change Disc: %s", path.c_str());
   Core::RunAsCPUThread([&path] { Core::System::GetInstance().GetDVDInterface().ChangeDisc(path); });

Source/Core/Core/Core.cpp

@@ -795,11 +795,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
-// TODO: Fix Android
-#ifndef ANDROID
+  // WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread
   ASSERT(IsHostThread());
-#endif
 
   if (!IsRunningAndStarted())
     return true;