From c1922783f8125dee4f12fdb20bdcbcf764e7ade4 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 12 May 2016 09:17:17 +0000 Subject: Core: Threadsafety Synchronization Fixes (Frame Advance / FifoPlayer) Fix Frame Advance and FifoPlayer pause/unpause/stop. CPU::EnableStepping is not atomic but is called from multiple threads which races and leaves the system in a random state; also instruction stepping was unstable, m_StepEvent had an almost random value because of the dual purpose it served which could cause races where CPU::Run would SingleStep when it was supposed to be sleeping. FifoPlayer never FinishStateMove()d which was causing it to deadlock. Rather than partially reimplementing CPU::Run, just use CPUCoreBase and then call CPU::Run(). More DRY and less likely to have weird bugs specific to the player (i.e the previous freezing on pause/stop). Refactor PowerPC::state into CPU since it manages the state of the CPU Thread which is controlled by CPU, not PowerPC. This simplifies the architecture somewhat and eliminates races that can be caused by calling PowerPC state functions directly instead of using CPU's (because they bypassed the EnableStepping lock). --- Source/Android/jni/MainAndroid.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'Source/Android/jni/MainAndroid.cpp') diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index d9b8a61fba..cd96fe46a2 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -29,7 +30,6 @@ #include "Core/HW/Wiimote.h" #include "Core/HW/WiimoteReal/WiimoteReal.h" #include "Core/PowerPC/JitInterface.h" -#include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/Profiler.h" #include "DiscIO/Volume.h" @@ -66,8 +66,13 @@ void Host_NotifyMapLoaded() {} void Host_RefreshDSPDebuggerWindow() {} Common::Event updateMainFrameEvent; +static bool s_have_wm_user_stop = false; void Host_Message(int Id) { + if (Id == WM_USER_STOP) + { + s_have_wm_user_stop = true; + } } void* Host_GetRenderHandle() @@ -657,11 +662,22 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run(JNIEnv * WiimoteReal::InitAdapterClass(); // No use running the loop when booting fails + s_have_wm_user_stop = false; if ( BootManager::BootCore( g_filename.c_str() ) ) { - PowerPC::Start(); - while (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) + static constexpr int TIMEOUT = 10000; + static constexpr int WAIT_STEP = 25; + int time_waited = 0; + // A Core::CORE_ERROR state would be helpful here. + while (!Core::IsRunning() && time_waited < TIMEOUT && !s_have_wm_user_stop) + { + std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_STEP)); + time_waited += WAIT_STEP; + } + while (Core::IsRunning()) + { updateMainFrameEvent.Wait(); + } } Core::Shutdown(); -- cgit v1.2.3 From c1944f623b5918ab45152795dea9e49f57878138 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 12 May 2016 01:18:30 +0000 Subject: Core/Movie: Add ability to run code in Host context EndPlayInput runs on the CPU thread so it can't directly call UpdateWantDeterminism. PlayController also tries to ChangeDisc from the CPU Thread which is also invalid. It now just pauses execution and posts a request to the Host to fix it instead. The Core itself also did dodgy things like PauseAndLock-ing from the CPU Thread and SetState from EmuThread which have been removed. --- Source/Android/jni/MainAndroid.cpp | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) (limited to 'Source/Android/jni/MainAndroid.cpp') diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index cd96fe46a2..6b30bc5409 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -65,13 +66,23 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved) void Host_NotifyMapLoaded() {} void Host_RefreshDSPDebuggerWindow() {} +// 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. +static std::mutex s_host_identity_lock; Common::Event updateMainFrameEvent; static bool s_have_wm_user_stop = false; void Host_Message(int Id) { - if (Id == WM_USER_STOP) + if (Id == WM_USER_JOB_DISPATCH) + { + updateMainFrameEvent.Set(); + } + else if (Id == WM_USER_STOP) { s_have_wm_user_stop = true; + if (Core::IsRunning()) + Core::QueueHostJob(&Core::Stop); } } @@ -393,15 +404,18 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestr JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_UnPauseEmulation(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SetState(Core::CORE_RUN); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulation(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SetState(Core::CORE_PAUSE); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SaveScreenShot("thumb"); Renderer::s_screenshotCompleted.WaitFor(std::chrono::seconds(2)); Core::Stop(); @@ -490,6 +504,7 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Supports JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveScreenShot(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SaveScreenShot(); } @@ -534,11 +549,13 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetFilename( JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveState(JNIEnv *env, jobject obj, jint slot) { + std::lock_guard guard(s_host_identity_lock); State::Save(slot); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadState(JNIEnv *env, jobject obj, jint slot) { + std::lock_guard guard(s_host_identity_lock); State::Load(slot); } @@ -565,6 +582,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_CreateUserFo JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetUserDirectory(JNIEnv *env, jobject obj, jstring jDirectory) { + std::lock_guard guard(s_host_identity_lock); std::string directory = GetJString(env, jDirectory); g_set_userpath = directory; UICommon::SetUserDirectory(directory); @@ -577,6 +595,7 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetUserDi JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling(JNIEnv *env, jobject obj, jboolean enable) { + std::lock_guard guard(s_host_identity_lock); Core::SetState(Core::CORE_PAUSE); JitInterface::ClearCache(); Profiler::g_ProfileBlocks = enable; @@ -585,6 +604,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_WriteProfileResults(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); std::string filename = File::GetUserPath(D_DUMP_IDX) + "Debug/profiler.txt"; File::CreateFullPath(filename); JitInterface::WriteProfileResults(filename); @@ -643,6 +663,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestr } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_RefreshWiimotes(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); WiimoteReal::Refresh(); } @@ -656,6 +677,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run(JNIEnv * RegisterMsgAlertHandler(&MsgAlert); + std::unique_lock guard(s_host_identity_lock); UICommon::SetUserDirectory(g_set_userpath); UICommon::Init(); @@ -676,12 +698,16 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run(JNIEnv * } while (Core::IsRunning()) { + guard.unlock(); updateMainFrameEvent.Wait(); + guard.lock(); + Core::HostDispatchJobs(); } } Core::Shutdown(); UICommon::Shutdown(); + guard.unlock(); if (surf) { -- cgit v1.2.3