Core: Store current state in less places

Core::GetState reads from four different pieces of state: s_is_stopping,
s_hardware_initialized, s_is_booting, and CPUManager::IsStepping.
I'm keeping that last one as is for now because there's code in Dolphin
that sets it directly, but we can unify the other three to make things
easier to reason about.

This commit also gets rid of s_is_started. This was previously used in
Core::IsRunningAndStarted to ensure true wouldn't be returned until the
CPU thread was started, but it wasn't used in Core::GetState, so
Core::GetState would happily return State::Running after we had
initialized the hardware but before we had initialized the CPU thread.
As far as I know, there are no callers that have any real need to know
whether the boot process is currently initializing the hardware or the
CPU thread. Perhaps once upon a time there was a desire to make the
apploader debuggable, but a long time has passed without anyone stepping
up to implement it, and the way CBoot::RunApploader is implemented makes
it rather difficult. So this commit makes all the functions in Core.cpp
consider the core to still be starting until the CPU thread is started.
This commit is contained in:
JosJuice
2024-06-02 15:10:25 +02:00
parent 04c246d11f
commit 962230f91e
13 changed files with 61 additions and 76 deletions

View File

@ -101,10 +101,6 @@ namespace Core
static bool s_wants_determinism;
// Declarations and definitions
static bool s_is_stopping = false;
static bool s_hardware_initialized = false;
static bool s_is_started = false;
static Common::Flag s_is_booting;
static std::thread s_emu_thread;
static std::vector<StateChangedCallbackFunc> s_on_state_changed_callbacks;
@ -114,6 +110,10 @@ static std::atomic<double> s_last_actual_emulation_speed{1.0};
static bool s_frame_step = false;
static std::atomic<bool> s_stop_frame_step;
// The value Paused is never stored in this variable. The core is considered to be in
// the Paused state if this variable is Running and the CPU reports that it's stepping.
static std::atomic<State> s_state = State::Uninitialized;
#ifdef USE_MEMORYWATCHER
static std::unique_ptr<MemoryWatcher> s_memory_watcher;
#endif
@ -190,7 +190,7 @@ std::string StopMessage(bool main_thread, std::string_view message)
void DisplayMessage(std::string message, int time_in_ms)
{
if (!IsRunning(Core::System::GetInstance()))
if (!IsRunningOrStarting(Core::System::GetInstance()))
return;
// Actually displaying non-ASCII could cause things to go pear-shaped
@ -202,12 +202,13 @@ void DisplayMessage(std::string message, int time_in_ms)
bool IsRunning(Core::System& system)
{
return (GetState(system) != State::Uninitialized || s_hardware_initialized) && !s_is_stopping;
return s_state.load() == State::Running;
}
bool IsRunningAndStarted()
bool IsRunningOrStarting(Core::System& system)
{
return s_is_started && !s_is_stopping;
const State state = s_state.load();
return state == State::Running || state == State::Starting;
}
bool IsCPUThread()
@ -262,7 +263,7 @@ bool Init(Core::System& system, std::unique_ptr<BootParameters> boot, const Wind
g_video_backend->PrepareWindow(prepared_wsi);
// Start the emu thread
s_is_booting.Set();
s_state.store(State::Starting);
s_emu_thread = std::thread(EmuThread, std::ref(system), std::move(boot), prepared_wsi);
return true;
}
@ -281,15 +282,13 @@ static void ResetRumble()
// Called from GUI thread
void Stop(Core::System& system) // - Hammertime!
{
if (const State state = GetState(system);
state == State::Stopping || state == State::Uninitialized)
{
const State state = s_state.load();
if (state == State::Stopping || state == State::Uninitialized)
return;
}
AchievementManager::GetInstance().CloseGame();
s_is_stopping = true;
s_state.store(State::Stopping);
CallOnStateChangedCallbacks(State::Stopping);
@ -394,7 +393,11 @@ static void CpuThread(Core::System& system, const std::optional<std::string>& sa
File::Delete(*savestate_path);
}
s_is_started = true;
// If s_state is Starting, change it to Running. But if it's already been set to Stopping
// by the host thread, don't change it.
State expected = State::Starting;
s_state.compare_exchange_strong(expected, State::Running);
{
#ifndef _WIN32
std::string gdb_socket = Config::Get(Config::MAIN_GDB_SOCKET);
@ -426,8 +429,6 @@ static void CpuThread(Core::System& system, const std::optional<std::string>& sa
s_memory_watcher.reset();
#endif
s_is_started = false;
if (exception_handler)
EMM::UninstallExceptionHandler();
@ -453,12 +454,15 @@ static void FifoPlayerThread(Core::System& system, const std::optional<std::stri
if (auto cpu_core = system.GetFifoPlayer().GetCPUCore())
{
system.GetPowerPC().InjectExternalCPUCore(cpu_core.get());
s_is_started = true;
// If s_state is Starting, change it to Running. But if it's already been set to Stopping
// by the host thread, don't change it.
State expected = State::Starting;
s_state.compare_exchange_strong(expected, State::Running);
CPUSetInitialExecutionState();
system.GetCPU().Run();
s_is_started = false;
system.GetPowerPC().InjectExternalCPUCore(nullptr);
system.GetFifoPlayer().Close();
}
@ -479,10 +483,7 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
{
CallOnStateChangedCallbacks(State::Starting);
Common::ScopeGuard flag_guard{[] {
s_is_booting.Clear();
s_is_started = false;
s_is_stopping = false;
s_wants_determinism = false;
s_state.store(State::Uninitialized);
CallOnStateChangedCallbacks(State::Uninitialized);
@ -557,8 +558,6 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
NetPlay::IsNetPlayRunning() ? &(boot_session_data.GetNetplaySettings()->sram) : nullptr);
Common::ScopeGuard hw_guard{[&system] {
// We must set up this flag before executing HW::Shutdown()
s_hardware_initialized = false;
INFO_LOG_FMT(CONSOLE, "{}", StopMessage(false, "Shutting down HW"));
HW::Shutdown(system);
INFO_LOG_FMT(CONSOLE, "{}", StopMessage(false, "HW shutdown"));
@ -602,10 +601,6 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
AudioCommon::PostInitSoundStream(system);
// The hardware is initialized.
s_hardware_initialized = true;
s_is_booting.Clear();
// Set execution state to known values (CPU/FIFO/Audio Paused)
system.GetCPU().Break();
@ -701,7 +696,7 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
void SetState(Core::System& system, State state, bool report_state_change)
{
// State cannot be controlled until the CPU Thread is operational
if (!IsRunningAndStarted())
if (s_state.load() != State::Running)
return;
switch (state)
@ -732,21 +727,11 @@ void SetState(Core::System& system, State state, bool report_state_change)
State GetState(Core::System& system)
{
if (s_is_stopping)
return State::Stopping;
if (s_hardware_initialized)
{
if (system.GetCPU().IsStepping())
return State::Paused;
return State::Running;
}
if (s_is_booting.IsSet())
return State::Starting;
return State::Uninitialized;
const State state = s_state.load();
if (state == State::Running && system.GetCPU().IsStepping())
return State::Paused;
else
return state;
}
static std::string GenerateScreenshotFolderPath()
@ -800,7 +785,7 @@ static bool PauseAndLock(Core::System& system, bool do_lock, bool unpause_on_unl
{
// WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread
if (!IsRunningAndStarted())
if (!IsRunning(system))
return true;
bool was_unpaused = true;
@ -1027,13 +1012,12 @@ void HostDispatchJobs(Core::System& system)
HostJob job = std::move(s_host_jobs_queue.front());
s_host_jobs_queue.pop();
// NOTE: Memory ordering is important. The booting flag needs to be
// checked first because the state transition is:
// Core::State::Uninitialized: s_is_booting -> s_hardware_initialized
// We need to check variables in the same order as the state
// transition, otherwise we race and get transient failures.
if (!job.run_after_stop && !s_is_booting.IsSet() && !IsRunning(system))
continue;
if (!job.run_after_stop)
{
const State state = s_state.load();
if (state == State::Stopping || state == State::Uninitialized)
continue;
}
guard.unlock();
job.job(system);