Skip to content

Commit

Permalink
Merge pull request #11873 from AdmiralCurtiss/pause-and-lock-host
Browse files Browse the repository at this point in the history
Core: Assert that only the Host thread may call PauseAndLock().
  • Loading branch information
JosJuice committed Jun 6, 2023
2 parents 83b4af8 + b3c9f49 commit 2d56daf
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 32 deletions.
55 changes: 35 additions & 20 deletions Source/Android/jni/MainAndroid.cpp
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}

Expand All @@ -317,29 +332,29 @@ 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);
}

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveStateAs(JNIEnv* env, jclass,
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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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());
Expand Down
18 changes: 18 additions & 0 deletions Source/Core/Core/Core.cpp
Expand Up @@ -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);

Expand Down Expand Up @@ -224,6 +225,11 @@ bool IsGPUThread()
return tls_is_gpu_thread;
}

bool IsHostThread()
{
return tls_is_host_thread;
}

bool WantsDeterminism()
{
return s_wants_determinism;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand Down
3 changes: 3 additions & 0 deletions Source/Core/Core/Core.h
Expand Up @@ -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);

Expand All @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions Source/Core/DolphinNoGUI/MainNoGUI.cpp
Expand Up @@ -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")
Expand Down
10 changes: 0 additions & 10 deletions Source/Core/DolphinQt/Host.cpp
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/DolphinQt/Main.cpp
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/DolphinQt/Settings.cpp
Expand Up @@ -71,7 +71,7 @@ Settings::Settings()
});

m_hotplug_callback_handle = g_controller_interface.RegisterDevicesChangedCallback([this] {
if (Host::GetInstance()->IsHostThread())
if (Core::IsHostThread())
{
emit DevicesChanged();
}
Expand Down
3 changes: 3 additions & 0 deletions Source/Core/DolphinTool/ToolMain.cpp
Expand Up @@ -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"
Expand All @@ -27,6 +28,8 @@ static int PrintUsage(int code)

int main(int argc, char* argv[])
{
Core::DeclareAsHostThread();

if (argc < 2)
return PrintUsage(1);

Expand Down
2 changes: 2 additions & 0 deletions Source/UnitTests/UnitTestsMain.cpp
Expand Up @@ -6,6 +6,7 @@
#include <fmt/format.h>

#include "Common/MsgHandler.h"
#include "Core/Core.h"

#include "gtest/gtest.h"

Expand All @@ -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();
Expand Down

0 comments on commit 2d56daf

Please sign in to comment.