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
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) - 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 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 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.