diff --git a/Core/Core.cpp b/Core/Core.cpp index 21a3dfa7fc..337185e003 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -17,9 +17,13 @@ #include "ppsspp_config.h" +#include #include #include +#include #include +#include +#include #include #include "Common/System/System.h" @@ -67,6 +71,64 @@ struct CPUStepCommand { static CPUStepCommand g_cpuStepCommand; +// Task queue for Core_RunOnCPUThread(), see Core.h for the rationale. Drained from Core_RunLoopUntil() +// below, so at least once per call to it (i.e. about once per host frame) even while the CPU is fully +// running, and continuously (in a tight spin) while it's stepping/paused. +struct CPUThreadTask { + std::function func; + bool done = false; +}; +static std::mutex g_cpuQueueMutex; +static std::condition_variable g_cpuQueueCond; +static std::vector> g_cpuQueue; +static std::once_flag g_cpuThreadIdOnce; +static std::thread::id g_cpuThreadId; +// Published via release/acquire around g_cpuThreadIdOnce, so it's safe to check from other threads +// without taking g_cpuQueueMutex - g_cpuThreadId itself never changes once this becomes true. +static std::atomic g_cpuThreadIdValid{ false }; + +void Core_RunOnCPUThread(std::function func) { + if (g_cpuThreadIdValid.load(std::memory_order_acquire) && std::this_thread::get_id() == g_cpuThreadId) { + // Already on the CPU thread (or called before it's ever run) - just do it now, avoids deadlock. + func(); + return; + } + + auto task = std::make_shared(); + task->func = std::move(func); + + std::unique_lock guard(g_cpuQueueMutex); + g_cpuQueue.push_back(task); + g_cpuQueueCond.wait(guard, [&] { return task->done; }); +} + +// Called from the CPU thread only. +static void Core_ProcessCPUQueue() { + std::call_once(g_cpuThreadIdOnce, [] { + g_cpuThreadId = std::this_thread::get_id(); + g_cpuThreadIdValid.store(true, std::memory_order_release); + }); + + std::vector> tasks; + { + std::lock_guard guard(g_cpuQueueMutex); + if (g_cpuQueue.empty()) + return; + tasks = std::move(g_cpuQueue); + g_cpuQueue.clear(); + } + + for (auto &task : tasks) + task->func(); + + { + std::lock_guard guard(g_cpuQueueMutex); + for (auto &task : tasks) + task->done = true; + } + g_cpuQueueCond.notify_all(); +} + // This is so that external threads can wait for the CPU to become inactive. static std::condition_variable m_InactiveCond; static std::mutex m_hInactiveMutex; @@ -206,6 +268,11 @@ bool Core_GetPowerSaving() { void Core_RunLoopUntil(u64 globalticks) { while (true) { + // Drain any functions queued up by Core_RunOnCPUThread() from other threads. Doing this at the + // top of this loop means it's reached at least once per call (i.e. about once per host frame) + // even while the CPU is fully running, and continuously (in a tight spin) while it's stepping/paused. + Core_ProcessCPUQueue(); + switch (coreState) { case CORE_POWERDOWN: case CORE_RUNTIME_ERROR: diff --git a/Core/Core.h b/Core/Core.h index 1ed94874f0..9b022a0da2 100644 --- a/Core/Core.h +++ b/Core/Core.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include @@ -149,6 +150,19 @@ bool Core_GetPowerSaving(); void Core_RunLoopUntil(u64 globalticks); +// Runs a function on the CPU thread - the thread that calls Core_RunLoopUntil (and thus, indirectly, +// NativeFrame). Useful for code running on unrelated threads (like the WebSocket debugger) that needs to +// safely touch state that's otherwise only ever touched from that thread (breakpoints, stepping, etc.), +// instead of poking at it directly from wherever the call happens to come from. +// +// Safe to call from any thread, including the CPU thread itself (in which case func just runs immediately). +// Blocks the calling thread until func has actually run, so don't call this from the CPU thread with +// something that would itself try to wait on the CPU thread - that'll deadlock. +// +// Currently only drained while the CPU is stepping/paused (see Core_ProcessStepping in Core.cpp) - fine +// for debugger use since queuing only makes sense once the CPU is alive, and by then this is reachable. +void Core_RunOnCPUThread(std::function func); + extern volatile CoreState coreState; extern volatile bool coreStatePending; diff --git a/Core/Debugger/WebSocket/SteppingSubscriber.cpp b/Core/Debugger/WebSocket/SteppingSubscriber.cpp index 9e6b3d58a5..64ac82800d 100644 --- a/Core/Debugger/WebSocket/SteppingSubscriber.cpp +++ b/Core/Debugger/WebSocket/SteppingSubscriber.cpp @@ -89,32 +89,40 @@ void WebSocketSteppingState::Into(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive()) return req.Fail("CPU not started"); if (!Core_IsStepping()) { + // Core_Break() is explicitly free-threaded (see Core.cpp), so no need to bounce this to the CPU + // thread - and we can't anyway, since Core_RunOnCPUThread() is only drained once the CPU actually + // *is* stepping, which this call is what triggers in the first place. Core_Break(BreakReason::DebugStepInto, 0); return; } - uint32_t threadID; - auto cpuDebug = CPUFromRequest(req, &threadID); - if (!cpuDebug) - return; + // The CPU is already stepping/paused, so the CPU thread is spinning in Core_ProcessStepping() and will + // promptly drain this. Route the actual breakpoint/stepping manipulation there instead of poking at it + // directly from this WebSocket handler thread. + Core_RunOnCPUThread([&] { + uint32_t threadID; + auto cpuDebug = CPUFromRequest(req, &threadID); + if (!cpuDebug) + return; - if (cpuDebug == currentDebugMIPS) { - // If the current PC is on a breakpoint, the user doesn't want to do nothing. - g_breakpoints.SetSkipFirst(currentMIPS->pc); + if (cpuDebug == currentDebugMIPS) { + // If the current PC is on a breakpoint, the user doesn't want to do nothing. + g_breakpoints.SetSkipFirst(currentMIPS->pc); - Core_RequestCPUStep(CPUStepType::Into, 1); - } else { - uint32_t breakpointAddress = cpuDebug->GetPC(); - PrepareResume(); - // Could have advanced to the breakpoint already in PrepareResume(). - // Note: we need to get cpuDebug again anyway (in case we ran some HLE above.) - cpuDebug = CPUFromRequest(req); - if (cpuDebug != currentDebugMIPS) { - g_breakpoints.AddBreakPoint(breakpointAddress, true); - AddThreadCondition(breakpointAddress, threadID); - Core_Resume(); + Core_RequestCPUStep(CPUStepType::Into, 1); + } else { + uint32_t breakpointAddress = cpuDebug->GetPC(); + PrepareResume(); + // Could have advanced to the breakpoint already in PrepareResume(). + // Note: we need to get cpuDebug again anyway (in case we ran some HLE above.) + cpuDebug = CPUFromRequest(req); + if (cpuDebug != currentDebugMIPS) { + g_breakpoints.AddBreakPoint(breakpointAddress, true); + AddThreadCondition(breakpointAddress, threadID); + Core_Resume(); + } } - } + }); } // Step over the next instruction (cpu.stepOver)