10 KiB
Debugger threading model (Core_RunOnCPUThread / g_frameMutex)
Which thread may touch CPU-thread-owned debugger state (breakpoints, symbol map, registers, memory,
kernel threads), and in which order the locks have to be taken. Read this before touching
Windows/Debugger/*, Core/Debugger/WebSocket* or anything else that reaches into core state from
outside the CPU thread.
CPU-thread-owned debugger state (g_breakpoints, g_symbolMap, g_disassemblyManager, registers,
memory, kernel threads) used to be touched directly from other threads (the WebSocket handler
thread, and the legacy Win32 debugger's message-pump thread) with no synchronization. Two
mechanisms now exist for doing this safely - pick based on whether you're mutating or just reading:
Core_RunOnCPUThread(func)(Core.h/Core.cpp) - queuesfuncto run on the CPU thread, blocking the caller until it's done. Use for mutations (breakpoint add/remove, register/memory writes, symbol map edits, stepping requests, thread wake/kill). Drained at the top of everyCore_RunLoopUntil()iteration, so it's reached whether the CPU is running or stepping/paused; runs immediately if already called from the CPU thread. Two hard rules learned the hard way: never put a modal Win32 dialog call (MessageBox,DialogBoxParam,InputBox_GetString,SomeDialog::exec()) inside the queued lambda - it would block the CPU thread on user input, so split the function into "read/decide", "show modal", "mutate" pieces instead. And never callSendMessage()targeting one of your own GUI-thread windows from inside the lambda - the calling GUI thread is blocked waiting on the CPU thread rather than pumping messages, so a cross-threadSendMessage()back to it deadlocks; keep such calls outside the lambda instead.g_frameMutex(Core.h/Core.cpp) - a plainstd::mutex, held byNativeFrame()(UI/NativeApp.cpp) only across the span where it actually touches that state (g_breakpoints.Frame()throughg_screenManager->render()- whereCore_RunLoopUntil()/actual CPU stepping happens - throughrunImDebugger/renderImDebugger), not across input handling or the present/frame-pacing waits. Use for reads invoked very frequently (WM_PAINT, a list reload triggered on every debugger-state-changed notification) where routing throughCore_RunOnCPUThreadwould be too slow/heavy. The legacy Win32 debugger windows do this now - seeCtrlRegisterList::onPaint,CtrlDisAsmView::onPaint,CtrlMemView::onPaint,CtrlBreakpointList::reloadBreakpoints,CtrlThreadList::reloadThreads, etc. inWindows/Debugger/*.cpp.
Architecture fact that makes g_frameMutex correct: regardless of graphics backend, NativeFrame()
(and thus CPU emulation via Core_RunLoopUntil(), and the Dear ImGui debugger) always runs on the
same thread - see Core/EmuThread.cpp. When a backend needs its own thread for actual graphics
API calls (GraphicsContext::NeedsSeparateEmuThread() - true for OpenGL, SDL, headless, libretro,
Qt; false for D3D11/Vulkan on Windows), the original thread stays behind purely to pump
graphicsContext->ThreadFrame() (i.e. just executes queued graphics API calls), and a newly
spawned thread takes over NativeFrame()/game logic/CPU duty. So UI/ImDebugger/*.cpp is always
safe to read/write this state directly, without either mechanism - it's always on the same thread
as Core_RunLoopUntil(). The legacy Win32 debugger is different: its dialogs are pumped by the
original WinMain message-loop thread, which is a genuinely separate OS thread from whichever
thread ends up running NativeFrame()/CPU, regardless of backend (this split happens one level
above the NeedsSeparateEmuThread() branch) - that's the whole reason it needed this mechanism.
Update (2026-08-08): both BreakpointManager's and SymbolMap's internal mutexes have been
removed after auditing every touchpoint across the codebase (WebSocket subscribers,
Windows/Debugger/*.cpp, Windows/MainWindowMenu.cpp/Windows/MainWindow.cpp main-window menu
items, Core/Core.cpp's free-threaded Core_Break/Core_Resume, UI/ImDebugger/*.cpp,
JIT/interpreter backends, Core/Debugger/MemBlockInfo.cpp) and confirming each is covered by one of
the two mechanisms above or is already on the CPU/NativeFrame thread. Notably, Windows/main.cpp's
SortSymbols() calls (fired from System_Notify(BOOT_DONE)/System_Notify(SYMBOL_MAP_UPDATED))
turned out to already be safe without any change - both notifications are only ever fired from the
CPU/NativeFrame thread (UI/EmuScreen.cpp, Core/HLE/sceKernelModule.cpp), despite an old comment
there claiming reliance on the (now-removed) internal lock. Qt/mainwindow.cpp/Qt/QtMain.cpp
still poke at g_symbolMap directly and unguarded on the Qt UI thread - a pre-existing issue,
deliberately left alone since Qt isn't a maintained backend and is slated for removal; removing the
lock doesn't change SymbolMap's public API, so Qt still builds, just without that safety net.
GPU/Common/GPUDebugInterface.cpp (GE debugger expression evaluation) and Core/MemFault.cpp
(crash-time diagnostics) also touch g_symbolMap and were deliberately not audited this round -
different subsystem / best-effort-by-nature respectively, follow up if they ever come up.
Two more things found while doing this:
Core_RunOnCPUThread()'s queue is only drained whereCore_RunLoopUntil()runs, which requires a game to be loaded (it's called fromEmuScreen::render()). CallingCore_RunOnCPUThread()while at the main menu with nothing loaded used to hang forever. Fixed by also calling the (now public)Core_ProcessCPUQueue()directly fromNativeFrame(), right beforeg_screenManager->render(), inside the sameg_frameMutex-locked span - so it always runs, not just while a game is active.- Lock-ordering rule: because
Core_ProcessCPUQueue()is called from insideNativeFrame()'sg_frameMutex-locked span, anyCore_RunOnCPUThread()lambda that itself tries to lockg_frameMutex(directly, or indirectly - e.g. by calling something likeCDisasm::NotifyMapLoaded(), which locks it internally) will deadlock. Keep such calls outside the queued lambda, same as the modal-dialog andSendMessage()rules above.
Lock ordering: g_frameMutex before Core_LockAgainstShutdown(), always
Core_LockAgainstShutdown() / CoreShutdownLock (a recursive_mutex, g_shutdownLock in
Core/Core.cpp) is held across CPU_Shutdown() and Memory::Reinit(), i.e. while the core is
going away. Take it on any thread other than the CPU thread before reading core state - emulated
memory, the symbol map, kernel objects - so none of it is freed mid-read. It was called
Memory::Lock() and only covered the memory map; the name misled people into thinking it was about
memory access. When a function needs both it and g_frameMutex, take g_frameMutex first.
The CPU thread's order is structural and can't be changed: NativeFrame() wraps everything below it
in g_frameMutex, and several things under there lock memory - Core_ProcessCPUQueue() running a
queued WebSocket handler, and runImDebugger() -> ImMemView -> DisassembleRange(). So the
GUI-thread side is the one that has to match. (Getting it backwards deadlocked for real: a paint
handler held the memory lock and waited for g_frameMutex while the CPU thread did the reverse.)
Also: a Core_RunOnCPUThread() callback does not need Core_LockAgainstShutdown() - teardown only happens
on the CPU thread itself (Memory::Shutdown() via CPU_Shutdown() <- PSP_Shutdown(), all callers
on that thread; Memory::Reinit() from Memory::DoState() on savestate load). Don't add one.
The general rule behind both of these: never make the CPU thread wait for a thread that is (or may
be) waiting on the CPU thread. Core_RunOnCPUThread() blocks until the CPU thread drains the
queue, so anything the CPU thread might block on must not be held across such a call. The WebSocket
debugger's lifecycleLock hit exactly this - it was held across a whole event handler, and the CPU
thread took it in Core_NotifyLifecycle(STOPPING), so stopping a game with a debugger request in
flight hung both threads. That lock is gone now; the rule is what's left of it.
The WebSocket debugger no longer has any lock guarding it against startup/shutdown, and must not
grow one back. The invariant instead is: a handler either does its emulator-state access inside
Core_RunOnCPUThread() - which serializes it against startup and teardown, since those run on the
CPU thread too - or touches only state that carries its own lock (the log ring buffer, ctrlMutex,
GPUStepping's pause-action rendezvous). When adding a subscriber, put the core access in the
queued callback, including the isAlive()/IsValidAddress() checks: answering those outside it
just means acting on an answer that may already be stale.
game.* and cpu.stepping/cpu.resume are pushed rather than polled - WebSocketDebuggerTick()
(called from Core_ProcessCPUQueue()) notices the transition on the CPU thread, formats the event
there, and drops it in a per-connection mailbox. Don't add a broadcaster that reads emulator state
from the connection's own thread; produce the event on the CPU thread and push it instead.
The Win32 debugger's GUI-thread readers do still need it, so don't "simplify" those away:
teardown is not yet fully inside the g_frameMutex span. EmuScreen::render()'s PSP_Shutdown() is
inside it, but the ones in EmuScreen::sendMessage() (REQUEST_GAME_RESET, loading a new game) run
from g_screenManager->sendMessage() in NativeFrame(), which sits above where the guard is
taken. Closing that hole - moving those shutdowns inside the span, or deferring them to render time -
is the prerequisite for dropping the shutdown lock from the debugger entirely.
Painting-problem design history, in case a similar tradeoff comes up elsewhere: routing every paint
through Core_RunOnCPUThread was rejected as too slow for something invoked continuously. A
per-window snapshot/cache with a per-row-rechecked Core_IsStepping() guard was tried first and
worked, but still had a narrow TOCTOU race (the CPU could resume between the check and that row's
reads) and the per-row-recheck pattern itself wasn't liked. Settled on g_frameMutex instead -
simpler, and actually race-free rather than just lower-risk. CtrlRegisterList shows live values
always now, grayed out by color alone (not cached) while the core is running, since a
constantly-moving value isn't meaningful to read closely anyway.