Files
ppsspp/docs/DebuggerThreading.md
T
Henrik RydgårdandClaude Opus 5 bd94e8141f Android: Converge the GL and Vulkan lifecycles on a shared RunGraphicsLoop
Stage 5 of the JavaGL removal. Now that the GL path owns its own EGL context and
render thread, Android's two lifecycle branches and the Win32 one in
MainThreadFunc were the same code three times over, so this moves the branch into
one place - RunGraphicsLoop() in Core/EmuThread.cpp. It picks the thread
arrangement the context asks for: either this thread becomes the render thread
and spawns the emu thread (OpenGL), or it just runs the main loop because the
backend spawns its own render thread (Vulkan, D3D11). Android, Windows and
headless all go through it now, which is what the "TODO: Merge with the Win32
EmuThread and so on" in app-android.cpp was asking for.

Falls out of having one implementation instead of three:

- MainThreadFunc called graphicsContext->ThreadStart() right after
  EmuThread_Start(), which already calls it, and ThreadEnd() right after
  EmuThread_Join(), which already calls that too. So Windows and headless GL ran
  CreateDeviceObjects()/DestroyDeviceObjects() twice per context, leaking a VAO
  on the way in and double-deleting it on the way out. Dates back to 16206d18ab,
  which moved those calls into EmuThread_Start/Join without removing the
  originals.

- RECREATE_VIEWS is gone. Only the GL path ever posted it; Vulkan goes through
  the same destroy/recreate without it and always has. Verified on device: views
  survive pause/resume, and both a real rotation and a density change - each one
  an activity destroy and recreate - lay them out again by themselves.

- The non-separate path is now named EmuThread rather than MainThread on
  Windows. It's the thread that runs NativeFrame(), which is what
  DebuggerThreading.md calls the emu thread, and the real main thread there is
  the GUI message pump.

The loop keeps an explicit exit flag, as a shouldExit callback, rather than
relying on ThreadFrame() returning false. In VR we skip ThreadFrame() entirely
while the XR session is idle, and the session going idle is exactly what
backgrounding the app does - so the exit would never be observed, the render
thread would sit in that sleep forever, and the UI thread joining it would hang.
That is not hypothetical: it deadlocked a Quest 2 on the first try.

Also drops a dead EmuThreadState enum and an unused JNIEnv local in
app-android.cpp, both orphaned when the JavaGL JNI entry points went away.

Tested on a nubia Z2464N (Mali-G57, Android 15) on both backends: cold start,
booting a homebrew, pause/resume in the menu, in a settings screen and in-game,
rotation back and forth, a density-change activity recreate with the game live,
the pause menu, and exit to menu. ThreadStart/ThreadEnd log exactly once per
context, and every one of those cycles tears the EGL context down and builds a
new one.

Also tested on a Quest 2 with the vr flavor, which is what shook out the deadlock
above: cold start into a focused XR session at 72fps, backgrounding (render
thread joins in 27ms where it used to never return), resuming back into VR with
a fresh EGL context, and exiting from in-app. xrCreateSession runs on the render
loop thread, which is where the EGL context is current - VRBase.cpp builds its
graphics binding from eglGetCurrentDisplay/eglGetCurrentContext, so that ordering
matters.

Windows x64 debug and the Android gradle builds are clean.

Not converged, and not a small change: NeedsSeparateEmuThread() itself. GL needs
its API calls on the thread its context is current on, so somebody has to pump
ThreadFrame() there, whereas VulkanRenderManager owns its thread outright.
Getting rid of the virtual means teaching GLRenderManager to spawn and own its
own render thread, for every GL platform at once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01152igCdjTNhT29qvphPyPC
2026-10-08 15:29:12 +02:00

11 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) - queues func to 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 every Core_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 call SendMessage() 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-thread SendMessage() back to it deadlocks; keep such calls outside the lambda instead.
  • g_frameMutex (Core.h/Core.cpp) - a plain std::mutex, held by NativeFrame() (UI/NativeApp.cpp) only across the span where it actually touches that state (g_breakpoints.Frame() through g_screenManager->render() - where Core_RunLoopUntil()/actual CPU stepping happens - through runImDebugger/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 through Core_RunOnCPUThread would be too slow/heavy. The legacy Win32 debugger windows do this now - see CtrlRegisterList::onPaint, CtrlDisAsmView::onPaint, CtrlMemView::onPaint, CtrlBreakpointList::reloadBreakpoints, CtrlThreadList::reloadThreads, etc. in Windows/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 RunGraphicsLoop() in Core/EmuThread.cpp, which every platform goes through (Windows and headless via MainThreadFunc(), Android from its render loop thread). 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 where Core_RunLoopUntil() runs, which requires a game to be loaded (it's called from EmuScreen::render()). Calling Core_RunOnCPUThread() while at the main menu with nothing loaded used to hang forever. Fixed by also calling the (now public) Core_ProcessCPUQueue() directly from NativeFrame(), right before g_screenManager->render(), inside the same g_frameMutex-locked span - so it always runs, not just while a game is active.
  • Lock-ordering rule: because Core_ProcessCPUQueue() is called from inside NativeFrame()'s g_frameMutex-locked span, any Core_RunOnCPUThread() lambda that itself tries to lock g_frameMutex (directly, or indirectly - e.g. by calling something like CDisasm::NotifyMapLoaded(), which locks it internally) will deadlock. Keep such calls outside the queued lambda, same as the modal-dialog and SendMessage() 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.