From b3c9f49cbedd03673a490cbeb7f886a3663e3bb8 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Fri, 2 Jun 2023 18:00:51 +0200 Subject: [PATCH] Core: Assert that only the Host thread may call PauseAndLock(). --- Source/Android/jni/MainAndroid.cpp | 55 ++++++++++++++++---------- Source/Core/Core/Core.cpp | 18 +++++++++ Source/Core/Core/Core.h | 3 ++ Source/Core/DolphinNoGUI/MainNoGUI.cpp | 2 + Source/Core/DolphinQt/Host.cpp | 10 ----- Source/Core/DolphinQt/Main.cpp | 2 +- Source/Core/DolphinQt/Settings.cpp | 2 +- Source/Core/DolphinTool/ToolMain.cpp | 3 ++ Source/UnitTests/UnitTestsMain.cpp | 2 + 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 +struct HostThreadWrapper +{ + T lock; + + explicit HostThreadWrapper(auto&&... args) : lock(std::forward(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 guard(s_host_identity_lock); + HostThreadWrapper> guard(s_host_identity_lock); Core::SetState(Core::State::Running); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulation(JNIEnv*, jclass) { - std::lock_guard guard(s_host_identity_lock); + HostThreadWrapper> guard(s_host_identity_lock); Core::SetState(Core::State::Paused); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv*, jclass) { - std::lock_guard guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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 guard(s_host_identity_lock); + HostThreadWrapper> 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> 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 guard(s_host_identity_lock); + HostThreadWrapper> guard(s_host_identity_lock); WiimoteReal::Refresh(); } @@ -535,7 +550,7 @@ static float GetRenderSurfaceScale(JNIEnv* env) static void Run(JNIEnv* env, std::unique_ptr&& boot, bool riivolution) { - std::unique_lock host_identity_guard(s_host_identity_lock); + HostThreadWrapper> host_identity_guard(s_host_identity_lock); if (riivolution && std::holds_alternative(boot->parameters)) { @@ -566,15 +581,15 @@ static void Run(JNIEnv* env, std::unique_ptr&& 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 1e940d1fed..74a87ed779 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 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 9ba5de1e98..1371507e1f 100644 --- a/Source/Core/DolphinNoGUI/MainNoGUI.cpp +++ b/Source/Core/DolphinNoGUI/MainNoGUI.cpp @@ -181,6 +181,8 @@ static std::unique_ptr 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 #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 #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();