Merge pull request #22106 from hrydgard/more-thread-fixes

More debugger threading fixes by Claude

AGENTS.md

@@ -383,11 +383,14 @@ Two more things found while doing this:
   `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 `Memory::Lock()`, always
+### Lock ordering: `g_frameMutex` before `Core_LockAgainstShutdown()`, always
 
-`Memory::Lock()` / `Memory::MemoryInitedLock` (a `recursive_mutex`, `g_shutdownLock` in
-`Core/MemMap.cpp`) guards the memory system against being torn down or reinitialized under you. When
-a function needs both it and `g_frameMutex`, **take `g_frameMutex` first**.
+`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
@@ -395,30 +398,36 @@ queued WebSocket handler, and `runImDebugger()` -> `ImMemView` -> `DisassembleRa
 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 `Memory::Lock()`** - teardown only happens
+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's held across a whole event handler, and the CPU
+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. Resolved by draining the queue while waiting for the lock rather than
-blocking on it outright (`WebSocketNotifyLifecycle` in `Core/Debugger/WebSocket.cpp`).
+flight hung both threads. That lock is gone now; the rule is what's left of it.
 
-`lifecycleLock` itself can't just be deleted, tempting as it looks: about half the subscribers
-(`GameSubscriber`, `GPU*Subscriber`, `InputSubscriber`, `MemoryInfoSubscriber`, `ReplaySubscriber`,
-`ClientConfigSubscriber`) and all four broadcasters still read core state directly on the WebSocket
-thread rather than going through `Core_RunOnCPUThread()`. It's what stops that racing with teardown.
-Routing those through the queue is the prerequisite for removing 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.
 
-The Win32 debugger's paint handlers *do* still need it, though, so don't "simplify" those away:
+`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 `Memory::Lock()` from the debugger entirely.
+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

Common/ExceptionHandlerSetup.cpp

@@ -20,9 +20,42 @@
 #include "Common/MachineContext.h"
 #include "Common/ExceptionHandlerSetup.h"
 
+#if defined(_MSC_VER)
+#include <crtdbg.h>
+#include "Common/CommonWindows.h"
+#endif
+
 static BadAccessHandler g_badAccessHandler;
 static void *altStack = nullptr;
 
+void SetupCRT(bool suppressDialogs) {
+#if defined(_MSC_VER)
+	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
+
+	if (suppressDialogs) {
+		// 1. Redirect CRT assertions/errors/warnings to stderr.
+		const _HFILE reportTarget = _CRTDBG_FILE_STDERR;
+
+		_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE);
+		_CrtSetReportFile(_CRT_ASSERT, reportTarget);
+
+		_CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE);
+		_CrtSetReportFile(_CRT_ERROR, reportTarget);
+
+		_CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE);
+		_CrtSetReportFile(_CRT_WARN, reportTarget);
+
+		// 2. Suppress the abort() message box & crash reporting dialogs.
+		_set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT);
+
+#if !PPSSPP_PLATFORM(UWP)
+		// 3. Suppress Windows OS-level "Program has stopped working" modal dialogs.
+		SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
+#endif
+	}
+#endif
+}
+
 #ifdef MACHINE_CONTEXT_SUPPORTED
 
 // We cannot handle exceptions in UWP builds. Bleh.

Common/ExceptionHandlerSetup.h

@@ -21,3 +21,8 @@ void InstallExceptionHandler(BadAccessHandler accessHandler, bool logStackTraceO
 
 // Implementation note: This must be a no-op if InstallExceptionHandler hasn't been called.
 void UninstallExceptionHandler();
+
+// MSVC-only, no-op elsewhere. Turns on the debug CRT's leak checking, and with suppressDialogs
+// set, routes CRT assertions and abort() to stderr instead of a modal dialog - required for
+// anything run non-interactively (headless, the unit tests, CI), where a dialog just hangs.
+void SetupCRT(bool suppressDialogs);

Core/Core.cpp

@@ -40,6 +40,7 @@
 #include "Core/System.h"
 #include "Core/MemFault.h"
 #include "Core/Debugger/Breakpoints.h"
+#include "Core/Debugger/WebSocket.h"
 #include "Core/MIPS/MIPS.h"
 #include "Core/MIPS/MIPSAnalyst.h"
 #include "Core/HLE/sceKernelModule.h"
@@ -113,6 +114,10 @@ void Core_ProcessCPUQueue() {
 		g_cpuThreadIdValid.store(true, std::memory_order_release);
 	});
 
+	// Piggybacking on the one function that's reliably called on the CPU thread both in game
+	// (Core_RunLoopUntil) and at the menu (NativeFrame) - see WebSocketDebuggerTick().
+	WebSocketDebuggerTick();
+
 	std::vector<std::shared_ptr<CPUThreadTask>> tasks;
 	{
 		std::lock_guard<std::mutex> guard(g_cpuQueueMutex);
@@ -133,6 +138,21 @@ void Core_ProcessCPUQueue() {
 	g_cpuQueueCond.notify_all();
 }
 
+// See Core.h. Recursive because Memory::Shutdown() nests inside CPU_Shutdown()'s acquire.
+static std::recursive_mutex g_shutdownLock;
+
+CoreShutdownLock::CoreShutdownLock() {
+	g_shutdownLock.lock();
+}
+
+CoreShutdownLock::~CoreShutdownLock() {
+	g_shutdownLock.unlock();
+}
+
+CoreShutdownLock Core_LockAgainstShutdown() {
+	return CoreShutdownLock();
+}
+
 // See Core.h for the rationale. Held by NativeFrame() (in NativeApp.cpp) around the span where it
 // actually touches CPU-thread-owned debugger state.
 std::mutex g_frameMutex;

Core/Core.h

@@ -168,6 +168,20 @@ void Core_ReenterDispatcher();  // If you've done things that mess with caches,
 // even while it's fully running.
 void Core_RunOnCPUThread(std::function<void()> func);
 
+// Held while the core is being torn down (CPU_Shutdown) or its memory map reinitialized. 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 can be freed mid-read. Recursive, so nesting is fine.
+//
+// It is not a lock on memory *access*: it doesn't stop the CPU thread mutating anything, only stop
+// it going away. If you also need a stable snapshot, take g_frameMutex first - see the ordering
+// rule in AGENTS.md.
+class CoreShutdownLock {
+public:
+	CoreShutdownLock();
+	~CoreShutdownLock();
+};
+CoreShutdownLock Core_LockAgainstShutdown();
+
 // Drains the queue Core_RunOnCPUThread() feeds. Normally called from the top of every
 // Core_RunLoopUntil() iteration, but that function is only reached while a game is actually
 // loaded/running (via EmuScreen) - so NativeFrame() (UI/NativeApp.cpp) also calls this directly,

Core/Debugger/DisassemblyManager.cpp

@@ -28,6 +28,7 @@
 #include "Common/Log.h"
 #include "Common/StringUtils.h"
 #include "Common/Math/math_util.h"
+#include "Core/Core.h"
 #include "Core/MemMap.h"
 #include "Core/System.h"
 #include "Core/MIPS/MIPSDebugInterface.h"
@@ -992,7 +993,7 @@ bool GetDisasmAddressText(u32 address, char *dest, size_t bufSize, bool abbrevia
 
 // Utilify function from the old debugger.
 std::string DisassembleRange(u32 start, u32 size, bool displaySymbols, MIPSDebugInterface *debugger) {
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	std::string result;
 
 	// gather all branch targets without labels

Core/Debugger/WebSocket.cpp

@@ -15,8 +15,10 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
+#include <algorithm>
 #include <mutex>
 #include <condition_variable>
+#include <vector>
 
 #include "Common/Thread/ThreadUtil.h"
 #include "Common/TimeUtil.h"
@@ -92,9 +94,12 @@ static volatile bool stopRequested = false;
 static std::mutex stopLock;
 static std::condition_variable stopCond;
 
-// Prevent threading surprises and obscure crashes by locking startup/shutdown.
-static bool lifecycleLockSetup = false;
-static std::mutex lifecycleLock;
+// There is deliberately no lock guarding debugger handlers against the core being started or torn
+// down under them: every handler either does its emulator-state access inside Core_RunOnCPUThread()
+// (so it's serialized with startup/shutdown, which also run on the CPU thread), or only touches
+// state that carries its own lock - the log ring buffer, ctrlMutex, GPUStepping's rendezvous.
+// The lock that used to be here had to be held across a whole handler, including the blocking wait
+// inside Core_RunOnCPUThread(), which deadlocked against the CPU thread taking it on STOPPING.
 
 static void UpdateConnected(int delta) {
 	std::lock_guard<std::mutex> guard(stopLock);
@@ -102,40 +107,68 @@ static void UpdateConnected(int delta) {
 	stopCond.notify_all();
 }
 
-static void WebSocketNotifyLifecycle(CoreLifecycle stage) {
-	switch (stage) {
-	case CoreLifecycle::STARTING:
-	case CoreLifecycle::STOPPING:
-	case CoreLifecycle::MEMORY_REINITING:
-		if (debuggersConnected > 0) {
-			DEBUG_LOG(Log::System, "Waiting for debugger to complete on shutdown");
-		}
-		// Keep draining the CPU queue while we wait, instead of a plain blocking lock(). We're on
-		// the CPU thread here, and a debugger thread holding this lock may be parked inside
-		// Core_RunOnCPUThread() waiting for us to run its callback - so blocking outright means
-		// neither side can ever move. Core state is still fully alive at this point (STOPPING is
-		// notified before CPU_Shutdown), so running those callbacks now is safe.
-		while (!lifecycleLock.try_lock()) {
-			Core_ProcessCPUQueue();
-			sleep_ms(1, "debugger-lifecycle");
-		}
-		break;
+// Per-connection mailbox for events the CPU thread produces (cpu.stepping, game.start, ...).
+//
+// These used to be polled per connection from the WebSocket thread, which meant every connected
+// debugger was reading pc, the tick count, the UI state and the param SFO out from under the CPU
+// thread on every lap of its loop. Now the CPU thread notices the transition once, formats the
+// event, and drops it in here; the connection's own thread just drains and sends.
+struct DebuggerEventSink {
+	std::mutex lock;
+	std::vector<std::pair<const char *, std::string>> pending;
+	// A debugger that connects while the CPU is already stopped still wants to hear about it.
+	bool needsSteppingPrime = true;
 
-	case CoreLifecycle::START_COMPLETE:
-	case CoreLifecycle::STOPPED:
-	case CoreLifecycle::MEMORY_REINITED:
-		lifecycleLock.unlock();
-		if (debuggersConnected > 0) {
-			DEBUG_LOG(Log::System, "Debugger ready for shutdown");
-		}
-		break;
+	void Push(const char *category, std::string json) {
+		std::lock_guard<std::mutex> guard(lock);
+		pending.emplace_back(category, std::move(json));
 	}
+
+	void Take(std::vector<std::pair<const char *, std::string>> *out) {
+		std::lock_guard<std::mutex> guard(lock);
+		out->swap(pending);
+		pending.clear();
+	}
+};
+
+static std::mutex g_sinkLock;
+static std::vector<DebuggerEventSink *> g_sinks;
+
+static void RegisterSink(DebuggerEventSink *sink) {
+	std::lock_guard<std::mutex> guard(g_sinkLock);
+	g_sinks.push_back(sink);
 }
 
-static void SetupDebuggerLock() {
-	if (!lifecycleLockSetup) {
-		Core_ListenLifecycle(&WebSocketNotifyLifecycle);
-		lifecycleLockSetup = true;
+static void UnregisterSink(DebuggerEventSink *sink) {
+	std::lock_guard<std::mutex> guard(g_sinkLock);
+	g_sinks.erase(std::remove(g_sinks.begin(), g_sinks.end(), sink), g_sinks.end());
+}
+
+void WebSocketDebuggerTick() {
+	// Poll unconditionally, even with nothing connected: these track transitions, and skipping them
+	// would let the "previous" state go stale and fire a bogus event at whoever connects next.
+	const std::string gameEvent = GameBroadcaster::PollChange();
+	const std::string steppingEvent = SteppingBroadcaster::PollChange();
+
+	std::lock_guard<std::mutex> guard(g_sinkLock);
+	if (g_sinks.empty())
+		return;
+
+	std::string steppingPrime;
+	for (DebuggerEventSink *sink : g_sinks) {
+		if (sink->needsSteppingPrime) {
+			sink->needsSteppingPrime = false;
+			// Only format it if somebody actually needs it.
+			if (steppingPrime.empty())
+				steppingPrime = SteppingBroadcaster::CurrentState();
+			if (!steppingPrime.empty())
+				sink->Push("stepping", steppingPrime);
+			continue;
+		}
+		if (!gameEvent.empty())
+			sink->Push("game", gameEvent);
+		if (!steppingEvent.empty())
+			sink->Push("stepping", steppingEvent);
 	}
 }
 
@@ -148,20 +181,19 @@ void HandleDebuggerRequest(const http::ServerRequest &request) {
 	}
 
 	UpdateConnected(1);
-	SetupDebuggerLock();
 
 	WebSocketClientInfo client_info;
 	auto& disallowed_config = client_info.disallowed;
 
-	GameBroadcaster game;
 	LogBroadcaster logger;
 	InputBroadcaster input;
-	SteppingBroadcaster stepping;
+
+	DebuggerEventSink sink;
+	RegisterSink(&sink);
 
 	DebuggerEventHandlerMap eventHandlers;
 	std::vector<DebuggerSubscriber *> subscriberData;
 	for (auto init : subscribers) {
-		std::lock_guard<std::mutex> guard(lifecycleLock);
 		subscriberData.push_back(init(eventHandlers));
 	}
 
@@ -186,7 +218,6 @@ void HandleDebuggerRequest(const http::ServerRequest &request) {
 		DebuggerRequest req(event, ws, root, &client_info);
 		auto eventFunc = eventHandlers.find(event);
 		if (eventFunc != eventHandlers.end()) {
-			std::lock_guard<std::mutex> guard(lifecycleLock);
 			eventFunc->second(req);
 			if (!req.Finish()) {
 				// Poll more frequently for a second in case this triggers something.
@@ -206,20 +237,23 @@ void HandleDebuggerRequest(const http::ServerRequest &request) {
 	constexpr float lowActivityPollTimeStep = 1.0f / 60.0f;
 	constexpr float highActivityPollTimeStep = 1.0f / 1000.0f;
 	while (ws->Process(highActivity ? highActivityPollTimeStep : lowActivityPollTimeStep)) {
-		std::lock_guard<std::mutex> guard(lifecycleLock);
 		// These send events that aren't just responses to requests
 
 		// The client can explicitly ask not to be notified about some events
 		// so we check the client settings first
 		if (!disallowed_config["logger"])
 			logger.Broadcast(ws);
-		if (!disallowed_config["game"])
-			game.Broadcast(ws);
-		if (!disallowed_config["stepping"])
-			stepping.Broadcast(ws);
 		if (!disallowed_config["input"])
 			input.Broadcast(ws);
 
+		// Whatever the CPU thread queued up for us since last lap.
+		std::vector<std::pair<const char *, std::string>> events;
+		sink.Take(&events);
+		for (const auto &ev : events) {
+			if (!disallowed_config[ev.first])
+				ws->Send(ev.second);
+		}
+
 		for (size_t i = 0; i < subscribers.size(); ++i) {
 			if (subscriberData[i]) {
 				subscriberData[i]->Broadcast(ws);
@@ -235,7 +269,8 @@ void HandleDebuggerRequest(const http::ServerRequest &request) {
 		}
 	}
 
-	std::lock_guard<std::mutex> guard(lifecycleLock);
+	UnregisterSink(&sink);
+
 	for (size_t i = 0; i < subscribers.size(); ++i) {
 		delete subscriberData[i];
 	}

Core/Debugger/WebSocket.h

@@ -24,3 +24,8 @@ class ServerRequest;
 void HandleDebuggerRequest(const http::ServerRequest &request);
 // Note: blocks.
 void StopAllDebuggers();
+
+// Notices emulator state changes (cpu.stepping, game.start, ...) and pushes the resulting events to
+// connected debuggers, so their own threads never have to read that state themselves. CPU thread
+// only; cheap, and safe to call with no debugger connected (it still has to track transitions).
+void WebSocketDebuggerTick();

Core/Debugger/WebSocket/BreakpointSubscriber.cpp

@@ -108,10 +108,6 @@ struct WebSocketCPUBreakpointParams {
 		if (hasCondition) {
 			if (!req.ParamString("condition", &condition))
 				return false;
-			if (!initExpression(currentDebugMIPS, condition.c_str(), compiledCondition)) {
-				req.Fail(StringFromFormat("Could not parse expression syntax: %s", getExpressionError()));
-				return false;
-			}
 		}
 		hasLogFormat = req.HasParam("logFormat");
 		if (hasLogFormat) {
@@ -122,6 +118,17 @@ struct WebSocketCPUBreakpointParams {
 		return true;
 	}
 
+	// Compiled on the CPU thread rather than in Parse(): resolving symbols in an expression goes
+	// through g_symbolMap, which is CPU-thread-owned and destroyed on shutdown.
+	bool CompileCondition(std::string *error) {
+		if (!hasCondition || condition.empty())
+			return true;
+		if (initExpression(currentDebugMIPS, condition.c_str(), compiledCondition))
+			return true;
+		*error = StringFromFormat("Could not parse expression syntax: %s", getExpressionError());
+		return false;
+	}
+
 	void Apply() {
 		if (hasCondition && !condition.empty()) {
 			BreakPointCond cond;
@@ -174,10 +181,15 @@ void WebSocketCPUBreakpointAdd(DebuggerRequest &req) {
 
 	// Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly
 	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	std::string error;
 	Core_RunOnCPUThread([&] {
+		if (!params.CompileCondition(&error))
+			return;
 		g_breakpoints.AddBreakPoint(params.address);
 		params.Apply();
 	});
+	if (!error.empty())
+		return req.Fail(error);
 	req.Respond();
 }
 
@@ -199,13 +211,18 @@ void WebSocketCPUBreakpointUpdate(DebuggerRequest &req) {
 	// Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly
 	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
 	bool found = false;
+	std::string error;
 	Core_RunOnCPUThread([&] {
+		if (!params.CompileCondition(&error))
+			return;
 		bool enabled;
 		found = g_breakpoints.IsAddressBreakPoint(params.address, &enabled);
 		if (found)
 			params.Apply();
 	});
 
+	if (!error.empty())
+		return req.Fail(error);
 	if (!found)
 		return req.Fail("Breakpoint not found");
 	req.Respond();
@@ -349,10 +366,6 @@ struct WebSocketMemoryBreakpointParams {
 		if (hasCondition) {
 			if (!req.ParamString("condition", &condition))
 				return false;
-			if (!initExpression(currentDebugMIPS, condition.c_str(), compiledCondition)) {
-				req.Fail(StringFromFormat("Could not parse expression syntax: %s", getExpressionError()));
-				return false;
-			}
 		}
 		hasLogFormat = req.HasParam("logFormat");
 		if (hasLogFormat) {
@@ -381,6 +394,17 @@ struct WebSocketMemoryBreakpointParams {
 		return BreakAction(bits);
 	}
 
+	// Compiled on the CPU thread rather than in Parse(): resolving symbols in an expression goes
+	// through g_symbolMap, which is CPU-thread-owned and destroyed on shutdown.
+	bool CompileCondition(std::string *error) {
+		if (!hasCondition || condition.empty())
+			return true;
+		if (initExpression(currentDebugMIPS, condition.c_str(), compiledCondition))
+			return true;
+		*error = StringFromFormat("Could not parse expression syntax: %s", getExpressionError());
+		return false;
+	}
+
 	void Apply() {
 		if (hasCondition && !condition.empty()) {
 			BreakPointCond cond;
@@ -421,10 +445,15 @@ void WebSocketMemoryBreakpointAdd(DebuggerRequest &req) {
 
 	// Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly
 	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	std::string error;
 	Core_RunOnCPUThread([&] {
+		if (!params.CompileCondition(&error))
+			return;
 		g_breakpoints.AddMemCheck(params.address, params.end, params.cond, params.Action(true));
 		params.Apply();
 	});
+	if (!error.empty())
+		return req.Fail(error);
 	req.Respond();
 }
 
@@ -451,7 +480,10 @@ void WebSocketMemoryBreakpointUpdate(DebuggerRequest &req) {
 	// Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly
 	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
 	bool found = false;
+	std::string error;
 	Core_RunOnCPUThread([&] {
+		if (!params.CompileCondition(&error))
+			return;
 		MemCheck mc;
 		found = g_breakpoints.GetMemCheck(params.address, params.end, &mc);
 		if (found) {
@@ -460,6 +492,8 @@ void WebSocketMemoryBreakpointUpdate(DebuggerRequest &req) {
 		}
 	});
 
+	if (!error.empty())
+		return req.Fail(error);
 	if (!found)
 		return req.Fail("Breakpoint not found");
 	req.Respond();
@@ -590,10 +624,6 @@ struct WebSocketRegBreakpointParams {
 		if (hasCondition) {
 			if (!req.ParamString("condition", &condition))
 				return false;
-			if (!initExpression(currentDebugMIPS, condition.c_str(), compiledCondition)) {
-				req.Fail(StringFromFormat("Could not parse expression syntax: %s", getExpressionError()));
-				return false;
-			}
 		}
 		hasLogFormat = req.HasParam("logFormat");
 		if (hasLogFormat) {
@@ -604,6 +634,17 @@ struct WebSocketRegBreakpointParams {
 		return true;
 	}
 
+	// Compiled on the CPU thread rather than in Parse(): resolving symbols in an expression goes
+	// through g_symbolMap, which is CPU-thread-owned and destroyed on shutdown.
+	bool CompileCondition(std::string *error) {
+		if (!hasCondition || condition.empty())
+			return true;
+		if (initExpression(currentDebugMIPS, condition.c_str(), compiledCondition))
+			return true;
+		*error = StringFromFormat("Could not parse expression syntax: %s", getExpressionError());
+		return false;
+	}
+
 	void Apply() {
 		if (hasCondition && !condition.empty()) {
 			BreakPointCond cond;
@@ -661,10 +702,15 @@ void WebSocketRegBreakpointAdd(DebuggerRequest &req) {
 
 	// Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly
 	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	std::string error;
 	Core_RunOnCPUThread([&] {
+		if (!params.CompileCondition(&error))
+			return;
 		g_breakpoints.AddRegBreakpoint(params.reg);
 		params.Apply();
 	});
+	if (!error.empty())
+		return req.Fail(error);
 	req.Respond();
 }
 
@@ -681,13 +727,18 @@ void WebSocketRegBreakpointUpdate(DebuggerRequest &req) {
 	// Route the actual breakpoint manipulation to the CPU thread instead of poking at it directly
 	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
 	bool found = false;
+	std::string error;
 	Core_RunOnCPUThread([&] {
+		if (!params.CompileCondition(&error))
+			return;
 		RegBreakpoint bp;
 		found = g_breakpoints.GetRegBreakpoint(params.reg, &bp);
 		if (found)
 			params.Apply();
 	});
 
+	if (!error.empty())
+		return req.Fail(error);
 	if (!found)
 		return req.Fail("Breakpoint not found");
 	req.Respond();

Core/Debugger/WebSocket/GPUDisasmSubscriber.cpp

@@ -20,6 +20,7 @@
 #include "Common/StringUtils.h"
 #include "Core/Debugger/WebSocket/GPUDisasmSubscriber.h"
 #include "Core/Debugger/WebSocket/WebSocketUtils.h"
+#include "Core/Core.h"
 #include "Core/MemMap.h"
 #include "GPU/GPU.h"
 #include "GPU/GPUCommon.h"
@@ -58,13 +59,6 @@ DebuggerSubscriber *WebSocketGPUDisasmInit(DebuggerEventHandlerMap &map) {
 //    "AAAAAAAA  desc" - meant for skimming a display list by eye instead of parsing full JSON,
 //    same idea as memory.disasm's own compact mode.
 void WebSocketGPUDisplayListDisasm(DebuggerRequest &req) {
-	if (!gpu) {
-		return req.Fail("No GPU active (game not booted?)");
-	}
-	if (!Memory::IsActive()) {
-		return req.Fail("Memory not active");
-	}
-
 	// Mirrors memory.disasm's own limit - keeps a client typo (e.g. count=0xFFFFFFFF) from
 	// blocking the debugger connection for an unreasonable amount of time.
 	static const uint32_t MAX_RANGE = 10000;
@@ -89,7 +83,17 @@ void WebSocketGPUDisplayListDisasm(DebuggerRequest &req) {
 	if (!req.ParamBool("compact", &compact, DebuggerParamType::OPTIONAL))
 		return;
 
-	std::vector<GPUDebugOp> ops = gpu->DisassembleOpRange(start, end);
+	// gpu and the memory it disassembles from are CPU-thread-owned, so do the read over there
+	// rather than from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	bool active = false;
+	std::vector<GPUDebugOp> ops;
+	Core_RunOnCPUThread([&] {
+		active = gpu && Memory::IsActive();
+		if (active)
+			ops = gpu->DisassembleOpRange(start, end);
+	});
+	if (!active)
+		return req.Fail("No GPU active (game not booted?)");
 
 	JsonWriter &json = req.Respond();
 	json.pushArray("lines");

Core/Debugger/WebSocket/GPURecordSubscriber.cpp

@@ -17,6 +17,7 @@
 
 #include "Common/Data/Encoding/Base64.h"
 #include "Common/File/FileUtil.h"
+#include "Core/Core.h"
 #include "Core/Debugger/WebSocket/GPURecordSubscriber.h"
 #include "Core/Debugger/WebSocket/WebSocketUtils.h"
 #include "Core/System.h"
@@ -44,9 +45,14 @@ DebuggerSubscriber *WebSocketGPURecordInit(DebuggerEventHandlerMap &map) {
 }
 
 WebSocketGPURecordState::~WebSocketGPURecordState() {
-	// Clear the callback to hopefully avoid a crash.
-	if (pending_)
-		gpu->GetRecorder()->ClearCallback();
+	// Clear the callback to hopefully avoid a crash. On the CPU thread, since gpu itself is
+	// destroyed over there - see Core_RunOnCPUThread() in Core.h.
+	if (pending_) {
+		Core_RunOnCPUThread([&] {
+			if (gpu)
+				gpu->GetRecorder()->ClearCallback();
+		});
+	}
 }
 
 // Begin recording (gpu.record.dump)
@@ -58,16 +64,24 @@ WebSocketGPURecordState::~WebSocketGPURecordState() {
 //
 // Note: recording may take a moment.
 void WebSocketGPURecordState::Dump(DebuggerRequest &req) {
-	if (PSP_GetBootState() != BootState::Complete) {
-		return req.Fail("CPU not started");
-	}
-
-	bool result = gpu->GetRecorder()->RecordNextFrame([=](const Path &filename) {
-		lastFilename_ = filename;
-		pending_ = false;
+	// gpu is created and destroyed on the CPU thread, so ask it for a recording over there rather
+	// than dereferencing it from this WebSocket handler thread.
+	bool started = false;
+	bool haveGPU = false;
+	Core_RunOnCPUThread([&] {
+		haveGPU = PSP_GetBootState() == BootState::Complete && gpu != nullptr;
+		if (!haveGPU)
+			return;
+		started = gpu->GetRecorder()->RecordNextFrame([=](const Path &filename) {
+			lastFilename_ = filename;
+			pending_ = false;
+		});
 	});
 
-	if (!result) {
+	if (!haveGPU) {
+		return req.Fail("CPU not started");
+	}
+	if (!started) {
 		return req.Fail("Recording already in progress");
 	}
 

Core/Debugger/WebSocket/GPUStatsSubscriber.cpp

@@ -20,6 +20,7 @@
 
 #include "Common/Data/Text/StringWriter.h"
 #include "Core/Debugger/WebSocket/GPUStatsSubscriber.h"
+#include "Core/Core.h"
 #include "Core/HW/Display.h"
 #include "Core/System.h"
 
@@ -102,8 +103,10 @@ WebSocketGPUStatsState::WebSocketGPUStatsState() {
 }
 
 WebSocketGPUStatsState::~WebSocketGPUStatsState() {
+	// PSP_ForceDebugStats bumps a plain counter, so do it on the CPU thread that owns it - see
+	// Core_RunOnCPUThread() in Core.h.
 	if (forced_)
-		PSP_ForceDebugStats(false);
+		Core_RunOnCPUThread([] { PSP_ForceDebugStats(false); });
 	__DisplayForgetFlip(&WebSocketGPUStatsState::FlipForwarder, this);
 }
 
@@ -185,7 +188,7 @@ void WebSocketGPUStatsState::Feed(DebuggerRequest &req) {
 	std::lock_guard<std::mutex> guard(pendingLock_);
 	sendFeed_ = enable;
 	if (forced_ != enable) {
-		PSP_ForceDebugStats(enable);
+		Core_RunOnCPUThread([enable] { PSP_ForceDebugStats(enable); });
 		forced_ = enable;
 	}
 }

Core/Debugger/WebSocket/GameBroadcaster.cpp

@@ -73,22 +73,28 @@ struct GameStatusEvent {
 //     - id: string disc ID (such as ULUS12345.)
 //     - version: string disc version.
 //     - title: string game title.
-void GameBroadcaster::Broadcast(net::WebSocketServer *ws) {
-	// TODO: This is ugly.  Callbacks instead?
-	GlobalUIState state = GetUIState();
-	if (prevState_ != state) {
-		if (state == UISTATE_PAUSEMENU) {
-			ws->Send(GameStatusEvent{"game.pause"});
-			prevState_ = state;
-		} else if (state == UISTATE_INGAME && prevState_ == UISTATE_PAUSEMENU) {
-			ws->Send(GameStatusEvent{"game.resume"});
-			prevState_ = state;
-		} else if (state == UISTATE_INGAME && PSP_GetBootState() == BootState::Complete) {
-			ws->Send(GameStatusEvent{"game.start"});
-			prevState_ = state;
-		} else if (state == UISTATE_MENU && PSP_GetBootState() != BootState::Complete) {
-			ws->Send(GameStatusEvent{"game.quit"});
-			prevState_ = state;
-		}
+std::string GameBroadcaster::PollChange() {
+	// Tracked globally rather than per connection: this runs on the CPU thread, which owns the state
+	// being read, and the resulting event is then handed to every connected debugger.
+	static GlobalUIState prevState = GetUIState();
+
+	const GlobalUIState state = GetUIState();
+	if (prevState == state)
+		return std::string();
+
+	const char *ev = nullptr;
+	if (state == UISTATE_PAUSEMENU) {
+		ev = "game.pause";
+	} else if (state == UISTATE_INGAME && prevState == UISTATE_PAUSEMENU) {
+		ev = "game.resume";
+	} else if (state == UISTATE_INGAME && PSP_GetBootState() == BootState::Complete) {
+		ev = "game.start";
+	} else if (state == UISTATE_MENU && PSP_GetBootState() != BootState::Complete) {
+		ev = "game.quit";
 	}
+	if (!ev)
+		return std::string();
+
+	prevState = state;
+	return GameStatusEvent{ev};
 }

Core/Debugger/WebSocket/GameBroadcaster.h

@@ -17,20 +17,14 @@
 
 #pragma once
 
-#include "Core/System.h"
+#include <string>
 
-namespace net {
-class WebSocketServer;
-}
+namespace GameBroadcaster {
 
-struct GameBroadcaster {
-public:
-	GameBroadcaster() {
-		prevState_ = GetUIState();
-	}
+// Notices game start/quit/pause/resume transitions and returns the formatted event for one, or an
+// empty string if nothing changed. CPU thread only - it reads the UI state and the param SFO, and
+// is what lets connected debuggers hear about this without touching either from their own threads.
+// Must be called even when no debugger is connected, so the transition state doesn't go stale.
+std::string PollChange();
 
-	void Broadcast(net::WebSocketServer *ws);
-
-private:
-	GlobalUIState prevState_;
-};
+}  // namespace GameBroadcaster

Core/Debugger/WebSocket/GameSubscriber.cpp

@@ -31,6 +31,7 @@
 
 #include "Common/System/System.h"
 #include "Core/Config.h"
+#include "Core/Core.h"
 #include "Core/CoreParameter.h"
 #include "Core/Debugger/WebSocket/GameSubscriber.h"
 #include "Core/Debugger/WebSocket/WebSocketUtils.h"
@@ -64,15 +65,21 @@ DebuggerSubscriber *WebSocketGameInit(DebuggerEventHandlerMap &map) {
 //
 // Response (same event name) with no extra data or error.
 void WebSocketGameReset(DebuggerRequest &req) {
-	if (PSP_GetBootState() != BootState::Complete)
-		return req.Fail("Game not running");
-
 	bool needBreak = false;
 	if (!req.ParamBool("break", &needBreak, DebuggerParamType::OPTIONAL))
 		return;
 
-	if (needBreak)
-		PSP_CoreParameter().startBreak = true;
+	// Route the boot-state check and the startBreak write to the CPU thread instead of poking at
+	// them directly from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	bool running = false;
+	Core_RunOnCPUThread([&] {
+		running = PSP_GetBootState() == BootState::Complete;
+		if (running && needBreak)
+			PSP_CoreParameter().startBreak = true;
+	});
+
+	if (!running)
+		return req.Fail("Game not running");
 
 	// We can only support async resets here. A lot of the stuff in init must happen on the EmuThread,
 	// and we are not on it here.
@@ -92,17 +99,21 @@ void WebSocketGameReset(DebuggerRequest &req) {
 //     - title: string game title.
 //  - paused: boolean, true when gameplay is paused (not the same as stepping.)
 void WebSocketGameStatus(DebuggerRequest &req) {
-	JsonWriter &json = req.Respond();
-	if (PSP_GetBootState() == BootState::Complete) {
-		json.pushDict("game");
-		json.writeString("id", g_paramSFO.GetDiscID());
-		json.writeString("version", g_paramSFO.GetValueString("DISC_VERSION"));
-		json.writeString("title", g_paramSFO.GetValueString("TITLE"));
-		json.pop();
-	} else {
-		json.writeNull("game");
-	}
-	json.writeBool("paused", GetUIState() == UISTATE_PAUSEMENU);
+	// Route the boot state and param SFO reads to the CPU thread instead of poking at them directly
+	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	Core_RunOnCPUThread([&] {
+		JsonWriter &json = req.Respond();
+		if (PSP_GetBootState() == BootState::Complete) {
+			json.pushDict("game");
+			json.writeString("id", g_paramSFO.GetDiscID());
+			json.writeString("version", g_paramSFO.GetValueString("DISC_VERSION"));
+			json.writeString("title", g_paramSFO.GetValueString("TITLE"));
+			json.pop();
+		} else {
+			json.writeNull("game");
+		}
+		json.writeBool("paused", GetUIState() == UISTATE_PAUSEMENU);
+	});
 }
 
 // Notify debugger version info (version)
@@ -121,8 +132,6 @@ void WebSocketGameStatus(DebuggerRequest &req) {
 // meant to attach to. Ports aren't enough on their own: a leftover process may still be holding
 // the one you asked for, and you'd never know you were driving the wrong emulator.
 void WebSocketVersion(DebuggerRequest &req) {
-	JsonWriter &json = req.Respond();
-
 	std::string version = req.client->version;
 	if (!req.ParamString("version", &version, DebuggerParamType::OPTIONAL_LOOSE))
 		return;
@@ -133,14 +142,19 @@ void WebSocketVersion(DebuggerRequest &req) {
 	req.client->version = version;
 	req.client->name = name;
 
+	// fileToStart is CPU-thread-owned (PSP_Shutdown clears it), so read it over there - see
+	// Core_RunOnCPUThread() in Core.h. The rest is constant and safe to read here.
+	std::string path;
+	Core_RunOnCPUThread([&] {
+		path = PSP_CoreParameter().fileToStart.ToString();
+	});
+
+	JsonWriter &json = req.Respond();
 	json.writeString("name", "PPSSPP");
 	json.writeString("version", PPSSPP_GIT_VERSION);
-
 	json.writeUint("pid", GetOwnProcessID());
-
-	const Path &fileToStart = PSP_CoreParameter().fileToStart;
-	if (fileToStart.empty())
+	if (path.empty())
 		json.writeNull("path");
 	else
-		json.writeString("path", fileToStart.ToString());
+		json.writeString("path", path);
 }

Core/Debugger/WebSocket/MemoryInfoSubscriber.cpp

@@ -18,6 +18,7 @@
 #include <algorithm>
 #include "Core/MIPS/MIPS.h"
 #include "Core/MIPS/MIPSDebugInterface.h"
+#include "Core/Core.h"
 #include "Core/Debugger/MemBlockInfo.h"
 #include "Core/Debugger/WebSocket/MemoryInfoSubscriber.h"
 #include "Core/Debugger/WebSocket/WebSocketUtils.h"
@@ -141,11 +142,17 @@ void WebSocketMemoryInfoState::Config(DebuggerRequest &req) {
 	if (!req.ParamBool("detailed", &detailed, DebuggerParamType::OPTIONAL))
 		return;
 
-	JsonWriter &json = req.Respond();
-	json.writeBool("detailed", MemBlockInfoDetailed());
+	// MemBlockInfo's detail tracking is CPU-thread-owned, so flip it over there rather than from
+	// this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	bool nowDetailed = false;
+	Core_RunOnCPUThread([&] {
+		if (setDetailed)
+			UpdateOverride(detailed);
+		nowDetailed = MemBlockInfoDetailed();
+	});
 
-	if (setDetailed)
-		UpdateOverride(detailed);
+	JsonWriter &json = req.Respond();
+	json.writeBool("detailed", nowDetailed);
 }
 
 static MemBlockFlags FlagFromType(const std::string &type) {
@@ -196,9 +203,6 @@ static std::string TypeFromFlag(const MemBlockFlags &flag) {
 // Note: Only one tag per type is maintained for any given memory address.
 // Small extent info may be ignored unless detailed tracking enabled (see memory.info.config.)
 void WebSocketMemoryInfoState::Set(DebuggerRequest &req) {
-	if (!currentDebugMIPS->isAlive() || !Memory::IsActive())
-		return req.Fail("CPU not started");
-
 	std::string type;
 	if (!req.ParamString("type", &type))
 		return;
@@ -213,20 +217,37 @@ void WebSocketMemoryInfoState::Set(DebuggerRequest &req) {
 	uint32_t size;
 	if (!req.ParamU32("size", &size))
 		return;
-	uint32_t pc = currentMIPS->pc;
-	if (!req.ParamU32("pc", &pc, false, DebuggerParamType::OPTIONAL))
+	// Defaults to the current PC, but that has to be read on the CPU thread - see below.
+	const bool hasPC = req.HasParam("pc");
+	uint32_t pc = 0;
+	if (hasPC && !req.ParamU32("pc", &pc, false, DebuggerParamType::OPTIONAL))
 		return;
 
 	MemBlockFlags flags = MemBlockFlags::SKIP_MEMCHECK | FlagFromType(type);
 	if (flags == MemBlockFlags::SKIP_MEMCHECK)
 		return req.Fail("Invaid type - expecting write, texture, alloc, suballoc, free, or subfree");
 
-	if (!Memory::IsValidAddress(addr))
-		return req.Fail("Invalid address");
-	else if (!Memory::IsValidRange(addr, size))
-		return req.Fail("Invalid size");
+	// Everything below this point reads or writes CPU-thread-owned state, so do it over there -
+	// see Core_RunOnCPUThread() in Core.h. The validity checks come along for the ride, since
+	// answering them out here would just mean acting on a stale answer.
+	const char *failure = nullptr;
+	Core_RunOnCPUThread([&] {
+		if (!currentDebugMIPS->isAlive() || !Memory::IsActive())
+			failure = "CPU not started";
+		else if (!Memory::IsValidAddress(addr))
+			failure = "Invalid address";
+		else if (!Memory::IsValidRange(addr, size))
+			failure = "Invalid size";
+		if (failure)
+			return;
+
+		if (!hasPC)
+			pc = currentMIPS->pc;
+		NotifyMemInfoPC(flags, addr, size, pc, tag.c_str(), tag.size());
+	});
+	if (failure)
+		return req.Fail(failure);
 
-	NotifyMemInfoPC(flags, addr, size, pc, tag.c_str(), tag.size());
 	req.Respond();
 }
 
@@ -251,9 +272,6 @@ void WebSocketMemoryInfoState::Set(DebuggerRequest &req) {
 //     - tag: string tag for this memory extent.
 //     - allocated: boolean, if this extent is marked as allocated (for alloc/suballoc types.)
 void WebSocketMemoryInfoState::List(DebuggerRequest &req) {
-	if (!currentDebugMIPS->isAlive() || !Memory::IsActive())
-		return req.Fail("CPU not started");
-
 	std::string type;
 	if (!req.ParamString("type", &type, DebuggerParamType::OPTIONAL))
 		return;
@@ -269,16 +287,27 @@ void WebSocketMemoryInfoState::List(DebuggerRequest &req) {
 	if (flags == MemBlockFlags::SKIP_MEMCHECK && req.HasParam("type"))
 		return req.Fail("Invaid type - expecting write, texture, alloc, suballoc, free, or subfree");
 
-	if (!Memory::IsValidAddress(addr))
-		return req.Fail("Invalid address");
-	else if (!Memory::IsValidRange(addr, size))
-		return req.Fail("Invalid size");
-
+	// The slab maps FindMemInfo walks are CPU-thread-owned, so gather over there rather than from
+	// this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h.
+	const char *failure = nullptr;
 	std::vector<MemBlockInfo> results;
-	if (flags == MemBlockFlags::SKIP_MEMCHECK)
-		results = FindMemInfo(addr, size);
-	else
-		results = FindMemInfoByFlag(flags, addr, size);
+	Core_RunOnCPUThread([&] {
+		if (!currentDebugMIPS->isAlive() || !Memory::IsActive())
+			failure = "CPU not started";
+		else if (!Memory::IsValidAddress(addr))
+			failure = "Invalid address";
+		else if (!Memory::IsValidRange(addr, size))
+			failure = "Invalid size";
+		if (failure)
+			return;
+
+		if (flags == MemBlockFlags::SKIP_MEMCHECK)
+			results = FindMemInfo(addr, size);
+		else
+			results = FindMemInfoByFlag(flags, addr, size);
+	});
+	if (failure)
+		return req.Fail(failure);
 
 	JsonWriter &json = req.Respond();
 	json.pushArray("extents");
@@ -320,9 +349,6 @@ void WebSocketMemoryInfoState::List(DebuggerRequest &req) {
 //
 // Note: may not be fast.
 void WebSocketMemoryInfoState::Search(DebuggerRequest &req) {
-	if (!currentDebugMIPS->isAlive() || !Memory::IsActive())
-		return req.Fail("CPU not started");
-
 	uint32_t start = 0;
 	if (!req.ParamU32("address", &start, false, DebuggerParamType::OPTIONAL))
 		return;
@@ -346,34 +372,47 @@ void WebSocketMemoryInfoState::Search(DebuggerRequest &req) {
 	std::transform(match.begin(), match.end(), match.begin(), ::tolower);
 
 	bool found = false;
+	bool alive = false;
 	MemBlockInfo foundResult;
 
-	uint32_t addr = start;
-	constexpr uint32_t CHUNK_SIZE = 0x1000;
-	do {
-		uint32_t chunk_end = addr + CHUNK_SIZE;
-		if (addr < end && chunk_end >= end) {
-			chunk_end = end;
-		}
+	// The whole scan happens in one trip to the CPU thread - see Core_RunOnCPUThread() in Core.h.
+	// It can cover a fair bit of memory and so will stall emulation briefly, which is fine for
+	// something a human triggers by hand. (The chunking below is just to avoid gathering every
+	// extent in the range before looking at any of them.)
+	Core_RunOnCPUThread([&] {
+		alive = currentDebugMIPS->isAlive() && Memory::IsActive();
+		if (!alive)
+			return;
 
-		std::vector<MemBlockInfo> results;
-		if (flags == MemBlockFlags::SKIP_MEMCHECK)
-			results = FindMemInfo(addr, chunk_end - addr);
-		else
-			results = FindMemInfoByFlag(flags, addr, chunk_end - addr);
-
-		for (const auto &result : results) {
-			std::string lowercase = result.tag;
-			std::transform(lowercase.begin(), lowercase.end(), lowercase.begin(), ::tolower);
-
-			if (lowercase.find(match) != lowercase.npos) {
-				found = true;
-				foundResult = result;
-				break;
+		uint32_t addr = start;
+		constexpr uint32_t CHUNK_SIZE = 0x1000;
+		do {
+			uint32_t chunk_end = addr + CHUNK_SIZE;
+			if (addr < end && chunk_end >= end) {
+				chunk_end = end;
 			}
-		}
-		addr = RoundMemAddressUp(chunk_end);
-	} while (!found && addr != end);
+
+			std::vector<MemBlockInfo> results;
+			if (flags == MemBlockFlags::SKIP_MEMCHECK)
+				results = FindMemInfo(addr, chunk_end - addr);
+			else
+				results = FindMemInfoByFlag(flags, addr, chunk_end - addr);
+
+			for (const auto &result : results) {
+				std::string lowercase = result.tag;
+				std::transform(lowercase.begin(), lowercase.end(), lowercase.begin(), ::tolower);
+
+				if (lowercase.find(match) != lowercase.npos) {
+					found = true;
+					foundResult = result;
+					break;
+				}
+			}
+			addr = RoundMemAddressUp(chunk_end);
+		} while (!found && addr != end);
+	});
+	if (!alive)
+		return req.Fail("CPU not started");
 
 	JsonWriter &json = req.Respond();
 	if (found) {

Core/Debugger/WebSocket/MemorySubscriber.cpp

@@ -63,7 +63,7 @@ struct AutoDisabledReplacements {
 // by the time this is called, so nothing else can be concurrently executing MIPS code or touching the
 // JIT's emuhack ops on this thread while we hold onto them below.
 //
-// Deliberately does NOT take a Memory::MemoryInitedLock: memory teardown only ever happens on the
+// Deliberately does NOT take a CoreShutdownLock: memory teardown only ever happens on the
 // CPU thread too, so there's nothing to guard against, and taking it here deadlocked against the
 // Win32 debugger's paint handlers. See the lock ordering section in AGENTS.md.
 //

Core/Debugger/WebSocket/ReplaySubscriber.cpp

@@ -19,6 +19,7 @@
 #include "Common/Data/Encoding/Base64.h"
 #include "Common/Swap.h"
 #include "Core/HLE/sceRtc.h"
+#include "Core/Core.h"
 #include "Core/Replay.h"
 #include "Core/System.h"
 #include "Core/Debugger/WebSocket/ReplaySubscriber.h"
@@ -45,7 +46,12 @@ DebuggerSubscriber *WebSocketReplayInit(DebuggerEventHandlerMap &map) {
 //
 // Response (same event name) with no extra data.
 void WebSocketReplayBegin(DebuggerRequest &req) {
-	ReplayBeginSave();
+	// Replay state is consumed by the CPU thread as it runs, so mutate it over there rather than
+	// from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Same for the rest
+	// of the handlers below.
+	Core_RunOnCPUThread([&] {
+		ReplayBeginSave();
+	});
 	req.Respond();
 }
 
@@ -57,7 +63,9 @@ void WebSocketReplayBegin(DebuggerRequest &req) {
 //
 // Response (same event name) with no extra data.
 void WebSocketReplayAbort(DebuggerRequest &req) {
-	ReplayAbort();
+	Core_RunOnCPUThread([&] {
+		ReplayAbort();
+	});
 	req.Respond();
 }
 
@@ -71,11 +79,15 @@ void WebSocketReplayAbort(DebuggerRequest &req) {
 //  - version: unsigned integer, version number of data.
 //  - base64: base64 encode of binary data.
 void WebSocketReplayFlush(DebuggerRequest &req) {
-	if (PSP_GetBootState() != BootState::Complete)
-		return req.Fail("Game not running");
-
+	bool running = false;
 	std::vector<uint8_t> data;
-	ReplayFlushBlob(&data);
+	Core_RunOnCPUThread([&] {
+		running = PSP_GetBootState() == BootState::Complete;
+		if (running)
+			ReplayFlushBlob(&data);
+	});
+	if (!running)
+		return req.Fail("Game not running");
 
 	JsonWriter &json = req.Respond();
 	json.writeInt("version", ReplayVersion());
@@ -90,9 +102,6 @@ void WebSocketReplayFlush(DebuggerRequest &req) {
 //
 // Response (same event name) with no extra data.
 void WebSocketReplayExecute(DebuggerRequest &req) {
-	if (PSP_GetBootState() != BootState::Complete)
-		return req.Fail("Game not running");
-
 	uint32_t version = -1;
 	if (!req.ParamU32("version", &version))
 		return;
@@ -101,7 +110,16 @@ void WebSocketReplayExecute(DebuggerRequest &req) {
 		return;
 
 	std::vector<uint8_t> data = Base64Decode(encoded.data(), encoded.size());
-	if (!ReplayExecuteBlob(version, data))
+	bool running = false;
+	bool ok = false;
+	Core_RunOnCPUThread([&] {
+		running = PSP_GetBootState() == BootState::Complete;
+		if (running)
+			ok = ReplayExecuteBlob(version, data);
+	});
+	if (!running)
+		return req.Fail("Game not running");
+	if (!ok)
 		return req.Fail("Invalid replay data or version");
 
 	req.Respond();
@@ -115,9 +133,15 @@ void WebSocketReplayExecute(DebuggerRequest &req) {
 //  - executing: boolean if a replay is being executed.
 //  - saving: boolean if a replay is being recorded.
 void WebSocketReplayStatus(DebuggerRequest &req) {
+	bool executing = false, saving = false;
+	Core_RunOnCPUThread([&] {
+		executing = ReplayIsExecuting();
+		saving = ReplayIsSaving();
+	});
+
 	JsonWriter &json = req.Respond();
-	json.writeBool("executing", ReplayIsExecuting());
-	json.writeBool("saving", ReplayIsSaving());
+	json.writeBool("executing", executing);
+	json.writeBool("saving", saving);
 }
 
 // Get the base RTC (real time clock) time for replay data (replay.time.get)
@@ -130,11 +154,18 @@ void WebSocketReplayStatus(DebuggerRequest &req) {
 // Response (same event name):
 //  - value: unsigned integer, may have more than 32 integer bits.
 void WebSocketReplayTimeGet(DebuggerRequest &req) {
-	if (PSP_GetBootState() != BootState::Complete)
+	bool running = false;
+	uint32_t baseTime = 0;
+	Core_RunOnCPUThread([&] {
+		running = PSP_GetBootState() == BootState::Complete;
+		if (running)
+			baseTime = RtcBaseTime();
+	});
+	if (!running)
 		return req.Fail("Game not running");
 
 	JsonWriter &json = req.Respond();
-	json.writeUint("value", RtcBaseTime());
+	json.writeUint("value", baseTime);
 }
 
 // Overwrite the base RTC time (replay.time.set)
@@ -144,14 +175,19 @@ void WebSocketReplayTimeGet(DebuggerRequest &req) {
 //
 // Response (same event name) with no extra data.
 void WebSocketReplayTimeSet(DebuggerRequest &req) {
-	if (PSP_GetBootState() != BootState::Complete)
-		return req.Fail("Game not running");
-
 	uint32_t value;
 	if (!req.ParamU32("value", &value, false)) {
 		return;
 	}
 
-	RtcSetBaseTime((int32_t)value);
+	bool running = false;
+	Core_RunOnCPUThread([&] {
+		running = PSP_GetBootState() == BootState::Complete;
+		if (running)
+			RtcSetBaseTime((int32_t)value);
+	});
+	if (!running)
+		return req.Fail("Game not running");
+
 	req.Respond();
 }

Core/Debugger/WebSocket/SteppingBroadcaster.cpp

@@ -56,19 +56,33 @@ private:
 // CPU has resumed from stepping (cpu.resume)
 //
 // Sent unexpectedly with no other properties.
-void SteppingBroadcaster::Broadcast(net::WebSocketServer *ws) {
-	if (PSP_GetBootState() == BootState::Complete) {
-		int steppingCounter = Core_GetSteppingCounter();
-		// We ignore CORE_POWERDOWN as a stepping state.
-		if (coreState == CORE_STEPPING_CPU && steppingCounter != lastCounter_) {
-			ws->Send(CPUSteppingEvent(Core_GetSteppingReason()));
-		} else if (prevState_ == CORE_STEPPING_CPU && coreState != CORE_STEPPING_CPU && Core_IsActive()) {
-			ws->Send(R"({"event":"cpu.resume"})");
-		}
-		lastCounter_ = steppingCounter;
-		prevState_ = coreState;
-	} else {
-		lastCounter_ = -1;
-		prevState_ = CORE_POWERDOWN;
+// Tracked globally rather than per connection: this runs on the CPU thread, which owns the state
+// being read, and the resulting event is then handed to every connected debugger.
+static CoreState g_prevState = CORE_POWERDOWN;
+static int g_lastCounter = 0;
+
+std::string SteppingBroadcaster::PollChange() {
+	if (PSP_GetBootState() != BootState::Complete) {
+		g_lastCounter = -1;
+		g_prevState = CORE_POWERDOWN;
+		return std::string();
 	}
+
+	std::string result;
+	const int steppingCounter = Core_GetSteppingCounter();
+	// We ignore CORE_POWERDOWN as a stepping state.
+	if (coreState == CORE_STEPPING_CPU && steppingCounter != g_lastCounter) {
+		result = CPUSteppingEvent(Core_GetSteppingReason());
+	} else if (g_prevState == CORE_STEPPING_CPU && coreState != CORE_STEPPING_CPU && Core_IsActive()) {
+		result = R"({"event":"cpu.resume"})";
+	}
+	g_lastCounter = steppingCounter;
+	g_prevState = coreState;
+	return result;
+}
+
+std::string SteppingBroadcaster::CurrentState() {
+	if (PSP_GetBootState() != BootState::Complete || coreState != CORE_STEPPING_CPU)
+		return std::string();
+	return CPUSteppingEvent(Core_GetSteppingReason());
 }

Core/Debugger/WebSocket/SteppingBroadcaster.h

@@ -17,21 +17,18 @@
 
 #pragma once
 
-#include "Core/Core.h"
+#include <string>
 
-namespace net {
-class WebSocketServer;
-}
+namespace SteppingBroadcaster {
 
-struct SteppingBroadcaster {
-public:
-	SteppingBroadcaster() {
-		prevState_ = coreState;
-	}
+// Notices the CPU entering or leaving stepping and returns the formatted event for one, or an empty
+// string if nothing changed. CPU thread only - it reads pc and the tick count, which is exactly what
+// connected debuggers must not do from their own threads. Must be called even when no debugger is
+// connected, so the transition state doesn't go stale.
+std::string PollChange();
 
-	void Broadcast(net::WebSocketServer *ws);
+// The cpu.stepping event describing the state right now, for a debugger that connected while the
+// CPU was already stopped. Empty if it isn't stepping. CPU thread only.
+std::string CurrentState();
 
-private:
-	CoreState prevState_;
-	int lastCounter_ = 0;
-};
+}  // namespace SteppingBroadcaster

Core/HW/Display.cpp

@@ -15,6 +15,7 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
+#include <atomic>
 #include <algorithm>
 #include <cmath>
 #include <mutex>
@@ -41,7 +42,9 @@ typedef std::pair<FlipCallback, void *> FlipListener;
 static std::vector<FlipListener> flipListeners;
 
 static uint64_t frameStartTicks;
-static int numVBlanks;
+// Atomic because the WebSocket debugger reads it from its own thread (input.buttons.press uses
+// it to count down frames) while the CPU thread bumps it.
+static std::atomic<int> numVBlanks;
 // hCount is computed now.
 static int vCount;
 // The "AccumulatedHcount" can be adjusted, this is the base.

Core/MemMap.cpp

@@ -87,7 +87,6 @@ u32 g_PSPModel;
 
 static MemMapSetupFlags g_setupFlags;
 
-std::recursive_mutex g_shutdownLock;
 
 // We don't declare the IO region in here since its handled by other means.
 static MemoryView views[] = {
@@ -343,6 +342,9 @@ bool Init(MemMapSetupFlags flags) {
 void Reinit() {
 	_assert_msg_(PSP_GetBootState() == BootState::Complete, "Cannot reinit during startup/shutdown");
 	Core_NotifyLifecycle(CoreLifecycle::MEMORY_REINITING);
+	// Held across both halves: between Shutdown() and Init() there is no memory map at all, and a
+	// reader that only saw Shutdown()'s own acquire could slip into that gap.
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	MemMapSetupFlags flags = g_setupFlags;
 	Shutdown();
 	Init(flags);
@@ -421,7 +423,7 @@ void DoState(PointerWrap &p) {
 }
 
 void Shutdown() {
-	std::lock_guard<std::recursive_mutex> guard(g_shutdownLock);
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	u32 flags = 0;
 	MemoryMap_Shutdown();
 	base = nullptr;
@@ -432,21 +434,6 @@ bool IsActive() {
 	return base != nullptr;
 }
 
-// Wanting to avoid include pollution, MemMap.h is included a lot.
-MemoryInitedLock::MemoryInitedLock()
-{
-	g_shutdownLock.lock();
-}
-MemoryInitedLock::~MemoryInitedLock()
-{
-	g_shutdownLock.unlock();
-}
-
-MemoryInitedLock Lock()
-{
-	return MemoryInitedLock();
-}
-
 static Opcode Read_Instruction(u32 address, bool resolveReplacements, Opcode inst) {
 	if (!MIPS_IS_EMUHACK(inst.encoding)) {
 		return inst;

Core/MemMap.h

@@ -118,16 +118,6 @@ void DoState(PointerWrap &p);
 // False when shutdown has already been called.
 bool IsActive();
 
-class MemoryInitedLock {
-public:
-	MemoryInitedLock();
-	~MemoryInitedLock();
-};
-
-// This doesn't lock memory access or anything, it just makes sure memory isn't freed.
-// Use it when accessing PSP memory from external threads.
-MemoryInitedLock Lock();
-
 // used by JIT to read instructions. Does not resolve replacements.
 Opcode Read_Opcode_JIT(const u32 _Address);
 // used by JIT. Reads in the "Locked cache" mode

Core/System.cpp

@@ -15,6 +15,7 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
+#include <atomic>
 #include "ppsspp_config.h"
 
 #ifdef _WIN32
@@ -102,7 +103,9 @@ static GPUBackend gpuBackend;
 static std::string gpuBackendDevice;
 static bool g_fileLoggingWasEnabled;
 
-static BootState g_bootState = BootState::Off;
+// Atomic because it's read as a fast-fail from the WebSocket debugger's own thread while the
+// CPU and loader threads move it along.
+static std::atomic<BootState> g_bootState = BootState::Off;
 
 BootState PSP_GetBootState() {
 	return g_bootState;
@@ -565,6 +568,12 @@ static bool CPU_Init(FileLoader *fileLoader, IdentifiedFileType type, std::strin
 }
 
 void CPU_Shutdown(bool success) {
+	// Held across the whole teardown, not just Memory::Shutdown() further down. Everything below
+	// frees state the debugger UIs read from other threads - kernel objects, the symbol map, the
+	// memory map - and this is the lock they take to be sure none of it goes away mid-read. See
+	// Core_LockAgainstShutdown(); it's recursive, so the nested acquire in Memory::Shutdown() is fine.
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
+
 	UninstallExceptionHandler();
 
 	GPURecord::Replay_Unload();

Windows/Debugger/CtrlDisAsmView.cpp

@@ -61,9 +61,10 @@ void CtrlDisAsmView::deinit()
 void CtrlDisAsmView::scanVisibleFunctions()
 {
 	// Reads live memory/symbol state to detect function boundaries - hold g_frameMutex for the
-	// duration, which NativeFrame() also holds while it's actually touching that state. See
-	// g_frameMutex in Core.h.
+	// duration, which NativeFrame() also holds while it's actually touching that state, and
+	// Core_LockAgainstShutdown() so the core can't be torn down mid-read. See g_frameMutex in Core.h.
 	std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	g_disassemblyManager.analyze(windowStart, g_disassemblyManager.getNthNextAddress(windowStart,visibleRows)-windowStart);
 }
 
@@ -243,7 +244,7 @@ std::string trimString(std::string input)
 
 void CtrlDisAsmView::assembleOpcode(u32 address, const std::string &defaultText)
 {
-	Memory::MemoryInitedLock memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!Core_IsStepping()) {
 		MessageBox(wnd,L"Cannot change code while the core is running!",L"Error",MB_OK);
 		return;
@@ -461,9 +462,9 @@ void CtrlDisAsmView::onPaint(WPARAM wParam, LPARAM lParam)
 	// with the CPU thread - hold g_frameMutex for the duration of the read, which NativeFrame()
 	// also holds while it's actually touching that state. See g_frameMutex in Core.h.
 	//
-	// g_frameMutex first, then Memory::Lock() - never the other way around. See CtrlMemView::onPaint.
+	// g_frameMutex first, then Core_LockAgainstShutdown() - never the other way around. See CtrlMemView::onPaint.
 	std::lock_guard<std::mutex> frameGuard(g_frameMutex);
-	Memory::MemoryInitedLock memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!debugger->isAlive() || Achievements::HardcoreModeActive()) return;
 
 	PAINTSTRUCT ps;
@@ -1195,7 +1196,7 @@ void CtrlDisAsmView::onMouseMove(WPARAM wParam, LPARAM lParam, int button)
 
 void CtrlDisAsmView::updateStatusBarText()
 {
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited())
 		return;
 
@@ -1229,7 +1230,7 @@ void CtrlDisAsmView::calculatePixelPositions()
 
 void CtrlDisAsmView::search(bool continueSearch)
 {
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	u32 searchAddress;
 
 	if (continueSearch == false || searchQuery[0] == 0)

Windows/Debugger/CtrlMemView.cpp

@@ -185,10 +185,10 @@ void CtrlMemView::onPaint(WPARAM wParam, LPARAM lParam) {
 	// thread - hold g_frameMutex for the duration of the read, which NativeFrame() also holds
 	// while it's actually touching that state. See g_frameMutex in Core.h.
 	//
-	// g_frameMutex first, then Memory::Lock() - never the other way around. The CPU thread has no
+	// g_frameMutex first, then Core_LockAgainstShutdown() - never the other way around. The CPU thread has no
 	// choice but that order (NativeFrame wraps everything below it), so this side has to match.
 	std::lock_guard<std::mutex> frameGuard(g_frameMutex);
-	Memory::MemoryInitedLock memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 
 	// draw to a bitmap for double buffering
 	PAINTSTRUCT ps;	
@@ -424,7 +424,7 @@ void CtrlMemView::onKeyDown(WPARAM wParam, LPARAM lParam) {
 }
 
 void CtrlMemView::onChar(WPARAM wParam, LPARAM lParam) {
-	Memory::MemoryInitedLock memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited())
 		return;
 
@@ -526,7 +526,7 @@ void CtrlMemView::onMouseUp(WPARAM wParam, LPARAM lParam, int button) {
 			
 		case ID_MEMVIEW_COPYVALUE_8:
 			{
-				auto memLock = Memory::Lock();
+				CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 				size_t tempSize = 3 * selectedSize + 1;
 				char *temp = new char[tempSize];
 				memset(temp, 0, tempSize);
@@ -556,7 +556,7 @@ void CtrlMemView::onMouseUp(WPARAM wParam, LPARAM lParam, int button) {
 			
 		case ID_MEMVIEW_COPYVALUE_16:
 			{
-				auto memLock = Memory::Lock();
+				CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 				size_t tempSize = 5 * ((selectedSize + 1) / 2) + 1;
 				char *temp = new char[tempSize];
 				memset(temp, 0, tempSize);
@@ -577,7 +577,7 @@ void CtrlMemView::onMouseUp(WPARAM wParam, LPARAM lParam, int button) {
 			
 		case ID_MEMVIEW_COPYVALUE_32:
 			{
-				auto memLock = Memory::Lock();
+				CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 				size_t tempSize = 9 * ((selectedSize + 3) / 4) + 1;
 				char *temp = new char[tempSize];
 				memset(temp, 0, tempSize);
@@ -598,7 +598,7 @@ void CtrlMemView::onMouseUp(WPARAM wParam, LPARAM lParam, int button) {
 
 		case ID_MEMVIEW_COPYFLOAT_32:
 		{
-			auto memLock = Memory::Lock();
+			CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 			std::ostringstream stream;
 			stream << (Memory::IsValid4AlignedAddress(curAddress_) ? Memory::ReadUnchecked_Float(curAddress_) : NAN);
 			auto temp_string = stream.str();
@@ -856,7 +856,7 @@ bool CtrlMemView::ParseSearchString(std::string_view query, bool asHex, std::vec
 std::vector<u32> CtrlMemView::searchString(std::string_view searchQuery) {
 	std::vector<u32> searchResAddrs;
 
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited())
 		return searchResAddrs;
 
@@ -893,7 +893,7 @@ std::vector<u32> CtrlMemView::searchString(std::string_view searchQuery) {
 };
 
 void CtrlMemView::search(bool continueSearch) {
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited())
 		return;
 

Windows/Debugger/CtrlRegisterList.cpp

@@ -203,6 +203,7 @@ void CtrlRegisterList::onPaint(WPARAM wParam, LPARAM lParam)
 	// with the CPU thread - hold g_frameMutex for the duration of the read, which NativeFrame()
 	// also holds while it's actually touching that state. See g_frameMutex in Core.h.
 	std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	// The values are a moving target while the core is running - gray them out rather than trying
 	// to highlight "changes" that are really just noise at that point.
 	bool running = !Core_IsStepping();

Windows/Debugger/Debugger_Disasm.cpp

@@ -730,6 +730,7 @@ void CDisasm::Show(bool bShow, bool includeToTop) {
 			// thread - hold g_frameMutex for the duration of the read, which NativeFrame() also
 			// holds while it's actually touching that state. See g_frameMutex in Core.h.
 			std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+			CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 			g_symbolMap->FillSymbolListBox(GetDlgItem(m_hDlg, IDC_FUNCTIONLIST), ST_FUNCTION);
 			deferredSymbolFill_ = false;
 		}
@@ -740,6 +741,7 @@ void CDisasm::Show(bool bShow, bool includeToTop) {
 void CDisasm::NotifyMapLoaded() {
 	if (m_bShowState != SW_HIDE && g_symbolMap) {
 		std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+		CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 		g_symbolMap->FillSymbolListBox(GetDlgItem(m_hDlg, IDC_FUNCTIONLIST), ST_FUNCTION);
 	} else {
 		deferredSymbolFill_ = true;

Windows/Debugger/Debugger_Lists.cpp

@@ -12,6 +12,7 @@
 #include "Windows/main.h"
 #include "Common/Data/Encoding/Utf8.h"
 #include "Core/Core.h"
+#include "Core/MemMap.h"
 #include "Core/HLE/sceKernelThread.h"
 
 enum { TL_NAME, TL_PROGRAMCOUNTER, TL_ENTRYPOINT, TL_PRIORITY, TL_STATE, TL_WAITTYPE, TL_COLUMNCOUNT };
@@ -260,6 +261,7 @@ void CtrlThreadList::reloadThreads()
 	// while it's actually touching that state. See g_frameMutex in Core.h.
 	{
 		std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+		CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 		threads = GetThreadsInfo();
 	}
 	Update();
@@ -335,6 +337,7 @@ void CtrlBreakpointList::reloadBreakpoints()
 	// with the CPU thread - see g_frameMutex in Core.h.
 	{
 		std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+		CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 		displayedBreakPoints_ = g_breakpoints.GetBreakpoints();
 		displayedMemChecks_= g_breakpoints.GetMemChecks();
 	}
@@ -747,9 +750,9 @@ void CtrlStackTraceView::loadStackTrace() {
 	// the CPU thread - hold g_frameMutex for the duration of the read, which NativeFrame() also
 	// holds while it's actually touching that state. See g_frameMutex in Core.h.
 	//
-	// g_frameMutex first, then Memory::Lock() - never the other way around. See CtrlMemView::onPaint.
+	// g_frameMutex first, then Core_LockAgainstShutdown() - never the other way around. See CtrlMemView::onPaint.
 	std::lock_guard<std::mutex> frameGuard(g_frameMutex);
-	Memory::MemoryInitedLock memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited())
 		return;
 
@@ -845,6 +848,7 @@ void CtrlModuleList::loadModules()
 	// actually touching that state. See g_frameMutex in Core.h.
 	{
 		std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+		CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 		if (g_symbolMap) {
 			modules = g_symbolMap->getAllModules();
 		} else {
@@ -870,6 +874,7 @@ void CtrlWatchList::RefreshValues() {
 	// otherwise race with the CPU thread - hold g_frameMutex for the duration, which NativeFrame()
 	// also holds while it's actually touching that state. See g_frameMutex in Core.h.
 	std::lock_guard<std::mutex> frameGuard(g_frameMutex);
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 
 	int steppingCounter = Core_GetSteppingCounter();
 	int changes = false;

Windows/Debugger/DumpMemoryWindow.cpp

@@ -88,7 +88,7 @@ INT_PTR CALLBACK DumpMemoryWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam,
 				// queued callback.
 				enum class Outcome { NotInited, OpenFailed, Success } outcome = Outcome::NotInited;
 				Core_RunOnCPUThread([&] {
-					Memory::MemoryInitedLock memLock = Memory::Lock();
+					CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 					if (!PSP_IsInited())
 						return;
 

Windows/GEDebugger/GEDebugger.cpp

@@ -552,7 +552,7 @@ void CGEDebugger::PreviewToClipboard(const GPUDebugBuffer *dbgBuffer, bool saveA
 }
 
 void CGEDebugger::UpdatePreviews() {
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited()) {
 		return;
 	}

Windows/GEDebugger/TabVertices.cpp

@@ -186,7 +186,7 @@ void CtrlVertexList::GetColumnText(wchar_t *dest, size_t destSize, int row, int
 }
 
 int CtrlVertexList::GetRowCount() {
-	auto memLock = Memory::Lock();
+	CoreShutdownLock coreLock = Core_LockAgainstShutdown();
 	if (!PSP_IsInited()) {
 		return 0;
 	}

headless/Headless.cpp

@@ -38,6 +38,7 @@
 #include <csignal>
 #endif
 #include "Common/CPUDetect.h"
+#include "Common/ExceptionHandlerSetup.h"
 #include "Common/File/VFS/VFS.h"
 #include "Common/File/VFS/ZipFileReader.h"
 #include "Common/File/VFS/DirectoryReader.h"
@@ -520,35 +521,6 @@ public:
 	}
 };
 
-// Has a parameter because we might start using this from the main build.
-void SetupCRT(bool headless) {
-#if defined(_MSC_VER)
-	_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
-
-	if (headless) {
-		// Suppress abort dialogs and similar.
-		// 1. Redirect CRT Assertions/Errors/Warnings to stdout/stderr
-
-		const _HFILE reportTarget = _CRTDBG_FILE_STDERR;
-
-		_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE);
-		_CrtSetReportFile(_CRT_ASSERT, reportTarget);
-
-		_CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE);
-		_CrtSetReportFile(_CRT_ERROR, reportTarget);
-
-		_CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE);
-		_CrtSetReportFile(_CRT_WARN, reportTarget);
-
-		// 2. Suppress the abort() message box & crash reporting dialogs
-		_set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT);
-
-		// 3. Suppress Windows OS-level "Program has stopped working" modal dialogs
-		SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
-	}
-#endif
-}
-
 int main(int argc, const char* argv[]) {
 	PROFILE_INIT();
 	TimeInit();

libretro/libretro.cpp

@@ -1984,7 +1984,7 @@ void System_PostUIMessage(UIMessage message, std::string_view param) {}
 void System_RunOnMainThread(std::function<void()>) {}
 void NativeFrame(GraphicsContext *graphicsContext) {}
 void NativeResized() {}
-
+void WebSocketDebuggerTick() {}  // stub to let things link
 void System_Toast(std::string_view str) {}
 
 inline int16_t Clamp16(int32_t sample) {

unittest/UnitTest.cpp

@@ -77,6 +77,7 @@
 #include "Common/ArmEmitter.h"
 #include "Common/BitScan.h"
 #include "Common/CPUDetect.h"
+#include "Common/ExceptionHandlerSetup.h"
 #include "Common/Log.h"
 #include "Common/StringUtils.h"
 #include "Core/Config.h"
@@ -1762,6 +1763,8 @@ TestItem availableTests[] = {
 };
 
 int main(int argc, const char *argv[]) {
+	// Never block on a modal dialog - these get run from CI and from tooling.
+	SetupCRT(true);
 	SetCurrentThreadName("UnitTest");
 	TimeInit();