Merge pull request #13689 from JosJuice/lock-core-any-thread

Core: Let any thread call previously host-thread-only functions
This commit is contained in:
JosJuice 2025-11-16 18:35:17 +01:00 committed by GitHub
commit b6e062f2e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 103 additions and 182 deletions

View File

@ -5,37 +5,24 @@
#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.
// TODO: The above isn't true anymore, so we should get rid of this class.
struct HostThreadLock
{
explicit HostThreadLock() : m_lock(s_host_identity_mutex) { Core::DeclareAsHostThread(); }
explicit HostThreadLock() : m_lock(s_host_identity_mutex) {}
~HostThreadLock()
{
if (m_lock.owns_lock())
Core::UndeclareAsHostThread();
}
~HostThreadLock() = default;
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 Lock() { m_lock.lock(); }
void Unlock()
{
m_lock.unlock();
Core::UndeclareAsHostThread();
}
void Unlock() { m_lock.unlock(); }
private:
static std::mutex s_host_identity_mutex;

View File

@ -65,7 +65,7 @@ void AchievementManager::Init(void* hwnd)
{
{
std::lock_guard lg{m_lock};
m_client = rc_client_create(MemoryVerifier, Request);
m_client = rc_client_create(MemoryPeeker, Request);
}
std::string host_url = Config::Get(Config::RA_HOST_URL);
if (!host_url.empty())
@ -170,7 +170,6 @@ void AchievementManager::LoadGame(const DiscIO::Volume* volume)
}
else
{
rc_client_set_read_memory_function(m_client, MemoryVerifier);
rc_client_begin_load_game(m_client, "", LoadGameCallback, NULL);
}
return;
@ -210,7 +209,6 @@ void AchievementManager::LoadGame(const DiscIO::Volume* volume)
else
{
u32 console_id = FindConsoleID(volume->GetVolumeType());
rc_client_set_read_memory_function(m_client, MemoryVerifier);
rc_client_begin_identify_and_load_game(m_client, console_id, "", NULL, 0, LoadGameCallback,
NULL);
}
@ -320,22 +318,6 @@ void AchievementManager::DoFrame()
if (!(IsGameLoaded() || m_dll_found) || !Core::IsCPUThread())
return;
{
#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION
if (m_dll_found)
{
std::lock_guard lg{m_memory_lock};
Core::System* system = m_system.load(std::memory_order_acquire);
if (!system)
return;
Core::CPUThreadGuard thread_guard(*system);
u32 mem2_size = (system->IsWii()) ? system->GetMemory().GetExRamSizeReal() : 0;
if (m_cloned_memory.size() != MEM1_SIZE + mem2_size)
m_cloned_memory.resize(MEM1_SIZE + mem2_size);
system->GetMemory().CopyFromEmu(m_cloned_memory.data(), 0, MEM1_SIZE);
if (mem2_size > 0)
system->GetMemory().CopyFromEmu(m_cloned_memory.data() + MEM1_SIZE, MEM2_START, mem2_size);
}
#endif // RC_CLIENT_SUPPORTS_RAINTEGRATION
std::lock_guard lg{m_lock};
rc_client_do_frame(m_client);
}
@ -1061,7 +1043,6 @@ void AchievementManager::LoadGameCallback(int result, const char* error_message,
if (game == nullptr)
return;
rc_client_set_read_memory_function(instance.m_client, MemoryPeeker);
instance.FetchGameBadges();
instance.m_system.store(&Core::System::GetInstance(), std::memory_order_release);
instance.update_event.Trigger({.all = true});
@ -1337,53 +1318,11 @@ void AchievementManager::Request(const rc_api_request_t* request,
});
}
// Currently, when rc_client calls the memory peek method provided in its constructor (or in
// rc_client_set_read_memory_function) it will do so on the thread that calls DoFrame, which is
// currently the host thread, with one exception: an asynchronous callback in the load game process.
// This is done to validate/invalidate each memory reference in the downloaded assets, mark assets
// as unsupported, and notify the player upon startup that there are unsupported assets and how
// many. As such, all that call needs to do is return the number of bytes that can be read with this
// call. As only the CPU and host threads are allowed to read from memory, I provide a separate
// method for this verification. In lieu of a more convenient set of steps, I provide MemoryVerifier
// to rc_client at construction, and in the Load Game callback, after the verification has been
// complete, I call rc_client_set_read_memory_function to switch to the usual MemoryPeeker for all
// future synchronous calls.
u32 AchievementManager::MemoryVerifier(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client)
{
auto& system = Core::System::GetInstance();
u32 mem2_size = system.GetMemory().GetExRamSizeReal();
if (address < MEM1_SIZE + mem2_size)
return std::min(MEM1_SIZE + mem2_size - address, num_bytes);
return 0;
}
u32 AchievementManager::MemoryPeeker(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client)
{
if (buffer == nullptr)
return 0u;
#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION
auto& instance = AchievementManager::GetInstance();
if (instance.m_dll_found)
{
std::lock_guard lg{instance.m_memory_lock};
if (u64(address) + num_bytes > instance.m_cloned_memory.size())
{
ERROR_LOG_FMT(ACHIEVEMENTS,
"Attempt to read past memory size: size {} address {} write length {}",
instance.m_cloned_memory.size(), address, num_bytes);
return 0;
}
std::copy(instance.m_cloned_memory.begin() + address,
instance.m_cloned_memory.begin() + address + num_bytes, buffer);
return num_bytes;
}
#endif // RC_CLIENT_SUPPORTS_RAINTEGRATION
auto& system = Core::System::GetInstance();
if (!(Core::IsHostThread() || Core::IsCPUThread()))
{
ASSERT_MSG(ACHIEVEMENTS, false, "MemoryPeeker called from wrong thread");
return 0;
}
Core::CPUThreadGuard thread_guard(system);
if (address > MEM1_SIZE)
address += (MEM2_START - MEM1_SIZE);
@ -1607,32 +1546,17 @@ void AchievementManager::MemoryPoker(u32 address, u8* buffer, u32 num_bytes, rc_
{
if (buffer == nullptr)
return;
if (!(Core::IsHostThread() || Core::IsCPUThread()))
{
Core::QueueHostJob([address, buffer, num_bytes, client](Core::System& system) {
MemoryPoker(address, buffer, num_bytes, client);
});
return;
}
auto& instance = AchievementManager::GetInstance();
if (u64(address) + num_bytes >= instance.m_cloned_memory.size())
{
ERROR_LOG_FMT(ACHIEVEMENTS,
"Attempt to write past memory size: size {} address {} write length {}",
instance.m_cloned_memory.size(), address, num_bytes);
return;
}
Core::System* system = instance.m_system.load(std::memory_order_acquire);
if (!system)
return;
Core::CPUThreadGuard thread_guard(*system);
std::lock_guard lg{instance.m_memory_lock};
if (address < MEM1_SIZE)
system->GetMemory().CopyToEmu(address, buffer, num_bytes);
else
system->GetMemory().CopyToEmu(address - MEM1_SIZE + MEM2_START, buffer, num_bytes);
std::copy(buffer, buffer + num_bytes, instance.m_cloned_memory.begin() + address);
}
void AchievementManager::GameTitleEstimateHandler(char* buffer, u32 buffer_size,
rc_client_t* client)
{

View File

@ -251,7 +251,6 @@ private:
static void Request(const rc_api_request_t* request, rc_client_server_callback_t callback,
void* callback_data, rc_client_t* client);
static u32 MemoryVerifier(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client);
static u32 MemoryPeeker(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client);
void FetchBadge(Badge* badge, u32 badge_type, const BadgeNameFunction function,
const UpdatedItems callback_data);
@ -293,8 +292,6 @@ private:
bool m_dll_found = false;
#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION
std::vector<u8> m_cloned_memory;
std::recursive_mutex m_memory_lock;
std::string m_title_estimate;
#endif // RC_CLIENT_SUPPORTS_RAINTEGRATION

View File

@ -105,6 +105,11 @@ static bool s_is_throttler_temp_disabled = false;
static bool s_frame_step = false;
static std::atomic<bool> s_stop_frame_step;
// Threads other than the CPU thread must hold this when taking on the role of the CPU thread.
// The CPU thread is not required to hold this when doing normal work, but must hold it if writing
// to s_state.
static std::recursive_mutex s_core_mutex;
// 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;
@ -126,7 +131,6 @@ 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(Core::System& system, std::unique_ptr<BootParameters> boot,
WindowSystemInfo wsi);
@ -207,11 +211,6 @@ bool IsGPUThread()
return tls_is_gpu_thread;
}
bool IsHostThread()
{
return tls_is_host_thread;
}
bool WantsDeterminism()
{
return s_wants_determinism;
@ -221,6 +220,8 @@ bool WantsDeterminism()
// BootManager.cpp
bool Init(Core::System& system, std::unique_ptr<BootParameters> boot, const WindowSystemInfo& wsi)
{
std::lock_guard lock(s_core_mutex);
if (s_emu_thread.joinable())
{
if (!IsUninitialized(system))
@ -266,16 +267,20 @@ static void ResetRumble()
// Called from GUI thread
void Stop(Core::System& system) // - Hammertime!
{
const State state = s_state.load();
if (state == State::Stopping || state == State::Uninitialized)
return;
{
std::lock_guard lock(s_core_mutex);
AchievementManager::GetInstance().CloseGame();
const State state = s_state.load();
if (state == State::Stopping || state == State::Uninitialized)
return;
s_state.store(State::Stopping);
s_state.store(State::Stopping);
}
NotifyStateChanged(State::Stopping);
AchievementManager::GetInstance().CloseGame();
// Dump left over jobs
HostDispatchJobs(system);
@ -308,21 +313,11 @@ 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)
{
// The CPU starts in stepping state, and will wait until a new state is set before executing.
// SetState must be called on the host thread, so we defer it for later.
// SetState isn't safe to call from the CPU thread, so we ask the host thread to call it.
QueueHostJob([force_paused](Core::System& system) {
bool paused = SConfig::GetInstance().bBootToPause || force_paused;
SetState(system, paused ? State::Paused : State::Running, true, true);
@ -362,10 +357,14 @@ static void CpuThread(Core::System& system, const std::optional<std::string>& sa
File::Delete(*savestate_path);
}
// 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);
{
std::unique_lock core_lock(s_core_mutex);
// If s_state is Starting, change it to Running. But if it's already been set to Stopping
// because another thread called Stop, don't change it.
State expected = State::Starting;
s_state.compare_exchange_strong(expected, State::Running);
}
{
#ifndef _WIN32
@ -423,12 +422,17 @@ static void FifoPlayerThread(Core::System& system, const std::optional<std::stri
{
system.GetPowerPC().InjectExternalCPUCore(cpu_core.get());
// 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);
{
std::lock_guard core_lock(s_core_mutex);
// If s_state is Starting, change it to Running. But if it's already been set to Stopping
// because another thread called Stop, don't change it.
State expected = State::Starting;
s_state.compare_exchange_strong(expected, State::Running);
}
CPUSetInitialExecutionState();
system.GetCPU().Run();
system.GetPowerPC().InjectExternalCPUCore(nullptr);
@ -517,7 +521,10 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
{
NotifyStateChanged(State::Starting);
Common::ScopeGuard flag_guard{[] {
s_state.store(State::Uninitialized);
{
std::lock_guard lock(s_core_mutex);
s_state.store(State::Uninitialized);
}
NotifyStateChanged(State::Uninitialized);
@ -682,35 +689,39 @@ static void EmuThread(Core::System& system, std::unique_ptr<BootParameters> boot
void SetState(Core::System& system, State state, bool report_state_change,
bool override_achievement_restrictions)
{
// State cannot be controlled until the CPU Thread is operational
if (s_state.load() != State::Running)
return;
{
std::lock_guard lock(s_core_mutex);
switch (state)
{
case State::Paused:
#ifdef USE_RETRO_ACHIEVEMENTS
if (!override_achievement_restrictions && !AchievementManager::GetInstance().CanPause())
// State cannot be controlled until the CPU Thread is operational
if (s_state.load() != State::Running)
return;
#endif // USE_RETRO_ACHIEVEMENTS
// NOTE: GetState() will return State::Paused immediately, even before anything has
// stopped (including the CPU).
system.GetCPU().SetStepping(true); // Break
Wiimote::Pause();
ResetRumble();
switch (state)
{
case State::Paused:
#ifdef USE_RETRO_ACHIEVEMENTS
AchievementManager::GetInstance().DoIdle();
if (!override_achievement_restrictions && !AchievementManager::GetInstance().CanPause())
return;
#endif // USE_RETRO_ACHIEVEMENTS
break;
case State::Running:
{
system.GetCPU().SetStepping(false);
Wiimote::Resume();
break;
}
default:
PanicAlertFmt("Invalid state");
break;
// NOTE: GetState() will return State::Paused immediately, even before anything has
// stopped (including the CPU).
system.GetCPU().SetStepping(true); // Break
Wiimote::Pause();
ResetRumble();
#ifdef USE_RETRO_ACHIEVEMENTS
AchievementManager::GetInstance().DoIdle();
#endif // USE_RETRO_ACHIEVEMENTS
break;
case State::Running:
{
system.GetCPU().SetStepping(false);
Wiimote::Resume();
break;
}
default:
PanicAlertFmt("Invalid state");
break;
}
}
// Certain callers only change the state momentarily. Sending a callback for them causes
@ -781,7 +792,7 @@ void SaveScreenShot(std::string_view name)
static bool PauseAndLock(Core::System& system)
{
// WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread
s_core_mutex.lock();
if (!IsRunning(system))
return true;
@ -804,7 +815,7 @@ static bool PauseAndLock(Core::System& system)
static void RestoreStateAndUnlock(Core::System& system, const bool unpause_on_unlock)
{
// WARNING: RestoreStateAndUnlock is not fully threadsafe so is only valid on the Host Thread
Common::ScopeGuard scope_guard([] { s_core_mutex.unlock(); });
if (!IsRunning(system))
return;
@ -824,8 +835,7 @@ static void RestoreStateAndUnlock(Core::System& system, const bool unpause_on_un
void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> function,
bool wait_for_completion)
{
// If the CPU thread is not running, assume there is no active CPU thread we can race against.
if (!IsRunning(system) || IsCPUThread())
if (IsCPUThread())
{
function();
return;
@ -834,10 +844,15 @@ void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> funct
// Pause the CPU (set it to stepping mode).
const bool was_running = PauseAndLock(system);
// Queue the job function.
if (wait_for_completion)
if (!IsRunning(system))
{
// Trigger the event after executing the function.
// If the core hasn't been started, there is no active CPU thread we can race against.
function();
wait_for_completion = false;
}
else if (wait_for_completion)
{
// Queue the job function followed by triggering the event.
s_cpu_thread_job_finished.Reset();
system.GetCPU().AddCPUThreadJob([&function] {
function();
@ -846,6 +861,7 @@ void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> funct
}
else
{
// Queue the job function.
system.GetCPU().AddCPUThreadJob(std::move(function));
}
@ -943,6 +959,8 @@ void NotifyStateChanged(Core::State state)
void UpdateWantDeterminism(Core::System& system, bool initial)
{
const Core::CPUThreadGuard guard(system);
// For now, this value is not itself configurable. Instead, individual
// settings that depend on it, such as GPU determinism mode. should have
// override options for testing,
@ -951,7 +969,6 @@ void UpdateWantDeterminism(Core::System& system, bool initial)
{
NOTICE_LOG_FMT(COMMON, "Want determinism <- {}", new_want_determinism ? "true" : "false");
const Core::CPUThreadGuard guard(system);
s_wants_determinism = new_want_determinism;
const auto ios = system.GetIOS();
if (ios)
@ -1013,6 +1030,9 @@ void DoFrameStep(Core::System& system)
OSD::AddMessage("Frame stepping is disabled in RetroAchievements hardcore mode");
return;
}
std::lock_guard lock(s_core_mutex);
if (GetState(system) == State::Paused)
{
// if already paused, frame advance for 1 frame

View File

@ -92,10 +92,9 @@ enum class ConsoleType : u32
ReservedTDEVSystem = 0x20000007,
};
// This is an RAII alternative to using PauseAndLock. If constructed from the host thread, the CPU
// thread is paused, and the current thread temporarily becomes the CPU thread. If constructed from
// the CPU thread, nothing special happens. This should only be constructed on the CPU thread or the
// host thread.
// This is an RAII alternative to using PauseAndLock. If constructed from any thread other than the
// CPU thread, the CPU thread is paused, and the current thread temporarily becomes the CPU thread.
// If constructed from the CPU thread, nothing special happens.
//
// Some functions use a parameter of this type to indicate that the function should only be called
// from the CPU thread. If the parameter is a pointer, the function has a fallback for being called
@ -119,6 +118,8 @@ private:
bool m_was_unpaused = false;
};
// These three are normally called from the Host thread. However, they can be called from any thread
// that isn't launched by the emulator core.
bool Init(Core::System& system, std::unique_ptr<BootParameters> boot, const WindowSystemInfo& wsi);
void Stop(Core::System& system);
void Shutdown(Core::System& system);
@ -127,8 +128,6 @@ void DeclareAsCPUThread();
void UndeclareAsCPUThread();
void DeclareAsGPUThread();
void UndeclareAsGPUThread();
void DeclareAsHostThread();
void UndeclareAsHostThread();
std::string StopMessage(bool main_thread, std::string_view message);
@ -141,11 +140,11 @@ bool IsUninitialized(Core::System& system);
bool IsCPUThread(); // this tells us whether we are the CPU thread.
bool IsGPUThread();
bool IsHostThread();
bool WantsDeterminism();
// [NOT THREADSAFE] For use by Host only
// SetState can't be called by the CPU thread, but can be called by any thread that isn't launched
// by the emulator core.
void SetState(Core::System& system, State state, bool report_state_change = true,
bool override_achievement_restrictions = false);
State GetState(Core::System& system);
@ -160,7 +159,6 @@ void FrameUpdateOnCPUThread();
void OnFrameEnd(Core::System& system);
// Run a function on the CPU thread, asynchronously.
// This is only valid to call from the host thread, since it uses PauseAndLock() internally.
void RunOnCPUThread(Core::System& system, Common::MoveOnlyFunction<void()> function,
bool wait_for_completion);
@ -170,7 +168,6 @@ using StateChangedCallbackFunc = std::function<void(Core::State)>;
[[nodiscard]] Common::EventHook AddOnStateChangedCallback(StateChangedCallbackFunc callback);
void NotifyStateChanged(Core::State state);
// Run on the Host thread when the factors change. [NOT THREADSAFE]
void UpdateWantDeterminism(Core::System& system, bool initial = false);
// Queue an arbitrary function to asynchronously run once on the Host thread later.

View File

@ -189,8 +189,6 @@ static std::unique_ptr<Platform> GetPlatform(const optparse::Values& options)
int main(const int argc, char* argv[])
{
Core::DeclareAsHostThread();
const auto parser =
CommandLineParse::CreateParser(CommandLineParse::ParserOptions::OmitGUIOptions);
parser->add_option("-p", "--platform")

View File

@ -8,6 +8,7 @@
#include <QAbstractEventDispatcher>
#include <QApplication>
#include <QLocale>
#include <QThread>
#include <imgui.h>
@ -245,7 +246,8 @@ bool Host_TASInputHasFocus()
void Host_YieldToUI()
{
qApp->processEvents(QEventLoop::ExcludeUserInputEvents);
if (qApp->thread() == QThread::currentThread())
qApp->processEvents(QEventLoop::ExcludeUserInputEvents);
}
void Host_UpdateDisasmDialog()

View File

@ -127,8 +127,6 @@ int main(int argc, char* argv[])
}
#endif
Core::DeclareAsHostThread();
#ifdef __APPLE__
// On macOS, a command line option matching the format "-psn_X_XXXXXX" is passed when
// the application is launched for the first time. This is to set the "ProcessSerialNumber",

View File

@ -17,6 +17,7 @@
#include <QSize>
#include <QStyle>
#include <QStyleHints>
#include <QThread>
#include <QWidget>
#include "AudioCommon/AudioCommon.h"
@ -76,7 +77,7 @@ Settings::Settings()
});
m_hotplug_event_hook = g_controller_interface.RegisterDevicesChangedCallback([this] {
if (Core::IsHostThread())
if (qApp->thread() == QThread::currentThread())
{
emit DevicesChanged();
}

View File

@ -32,8 +32,6 @@ static void PrintUsage()
int main(int argc, char* argv[])
{
Core::DeclareAsHostThread();
if (argc < 2)
{
PrintUsage();

View File

@ -25,7 +25,6 @@ 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();