Legacy Win32 debugger: route mutations through the CPU thread

The Win32 debugger dialogs mutate CPU-thread-owned state (breakpoints,
symbol map, registers, memory, kernel threads) directly from the GUI
thread with no synchronization, same problem the WebSocket debugger had.
Route all of these through Core_RunOnCPUThread instead, following the
same pattern used there.

Also drops two forced-pause (Core_Break/Core_WaitInactive/Core_Resume)
dances in CtrlMemView::onChar and DumpMemoryWindow's dump handler, now
unnecessary since Core_RunOnCPUThread works whether the CPU is running
or already stepping.

Modal dialogs (MessageBox, InputBox_GetString, BreakpointWindow::exec,
etc.) are kept outside any queued callback so they never block the CPU
thread on user input. Pure reads/painting (disassembly formatting,
search, list reloads, onPaint) are intentionally left as-is for now.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH

Windows/Debugger/CtrlDisAsmView.cpp

@@ -6,10 +6,12 @@
 #include "Windows/MainWindow.h"
 #include "Windows/InputBox.h"
 
+#include "Core/Core.h"
 #include "Core/MIPS/MIPSAsm.h"
 #include "Core/MIPS/MIPSAnalyst.h"
 #include "Core/MIPS/MIPSTables.h"
 #include "Core/Config.h"
+#include "Core/Debugger/Breakpoints.h"
 #include "Core/Debugger/SymbolMap.h"
 #include "Core/Reporting.h"
 #include "Common/StringUtils.h"
@@ -237,7 +239,7 @@ std::string trimString(std::string input)
 
 void CtrlDisAsmView::assembleOpcode(u32 address, const std::string &defaultText)
 {
-	auto memLock = Memory::Lock();
+	Memory::MemoryInitedLock memLock = Memory::Lock();
 	if (!Core_IsStepping()) {
 		MessageBox(wnd,L"Cannot change code while the core is running!",L"Error",MB_OK);
 		return;
@@ -249,7 +251,7 @@ void CtrlDisAsmView::assembleOpcode(u32 address, const std::string &defaultText)
 	}
 
 	// check if it changes registers first
-	auto separator = op.find('=');
+	size_t separator = op.find('=');
 	if (separator != std::string::npos)
 	{
 		std::string registerName = trimString(op.substr(0,separator));
@@ -264,8 +266,12 @@ void CtrlDisAsmView::assembleOpcode(u32 address, const std::string &defaultText)
 				{
 					if (strcasecmp(debugger->GetRegName(cat,reg).c_str(), registerName.c_str()) == 0)
 					{
-						debugger->SetRegValue(cat,reg,value);
-						Reporting::NotifyDebugger();
+						// Route the actual register write to the CPU thread instead of poking at it
+						// directly from this GUI thread - see Core_RunOnCPUThread() in Core.h.
+						Core_RunOnCPUThread([&] {
+							debugger->SetRegValue(cat,reg,value);
+							Reporting::NotifyDebugger();
+						});
 						SendMessage(GetParent(wnd),WM_DEB_UPDATE,0,0);
 						return;
 					}
@@ -276,9 +282,13 @@ void CtrlDisAsmView::assembleOpcode(u32 address, const std::string &defaultText)
 		// try to assemble the input if it failed
 	}
 
+	// Route the actual code assembly to the CPU thread instead of poking at it directly
+	// from this GUI thread - see Core_RunOnCPUThread() in Core.h.
 	std::string error;
-	result = MipsAssembleOpcode(op, debugger, address, &error);
-	Reporting::NotifyDebugger();
+	Core_RunOnCPUThread([&] {
+		result = MipsAssembleOpcode(op, debugger, address, &error);
+		Reporting::NotifyDebugger();
+	});
 	if (result) {
 		scanVisibleFunctions();
 
@@ -640,29 +650,37 @@ void CtrlDisAsmView::editBreakpoint()
 {
 	BreakpointWindow win(wnd,debugger);
 
+	// Route the actual breakpoint read to the CPU thread instead of poking at it directly
+	// from this GUI thread - see Core_RunOnCPUThread() in Core.h.
 	bool exists = false;
-	if (g_breakpoints.IsAddressBreakPoint(curAddress))
-	{
-		auto breakpoints = g_breakpoints.GetBreakpoints();
-		for (size_t i = 0; i < breakpoints.size(); i++)
+	Core_RunOnCPUThread([&] {
+		if (g_breakpoints.IsAddressBreakPoint(curAddress))
 		{
-			if (breakpoints[i].addr == curAddress)
+			std::vector<BreakPoint> breakpoints = g_breakpoints.GetBreakpoints();
+			for (size_t i = 0; i < breakpoints.size(); i++)
 			{
-				win.loadFromBreakpoint(breakpoints[i]);
-				exists = true;
-				break;
+				if (breakpoints[i].addr == curAddress)
+				{
+					win.loadFromBreakpoint(breakpoints[i]);
+					exists = true;
+					break;
+				}
 			}
 		}
-	}
+	});
 
 	if (!exists)
 		win.initBreakpoint(curAddress);
 
+	// win.exec() is a modal dialog - must stay outside any queued callback, or we'd block the
+	// CPU thread on user input.
 	if (win.exec())
 	{
-		if (exists)
-			g_breakpoints.RemoveBreakPoint(curAddress);
-		win.addBreakpoint();
+		Core_RunOnCPUThread([&] {
+			if (exists)
+				g_breakpoints.RemoveBreakPoint(curAddress);
+			win.addBreakpoint();
+		});
 	}
 }
 
@@ -767,7 +785,10 @@ void CtrlDisAsmView::onKeyDown(WPARAM wParam, LPARAM lParam)
 			displaySymbols = !displaySymbols;
 			break;
 		case VK_SPACE:
-			debugger->toggleBreakpoint(curAddress);
+			{
+				u32 toggleAddress = curAddress;
+				Core_RunOnCPUThread([&] { debugger->toggleBreakpoint(toggleAddress); });
+			}
 			break;
 		case VK_F3:
 			search(true);
@@ -818,25 +839,34 @@ void CtrlDisAsmView::redraw()
 
 void CtrlDisAsmView::toggleBreakpoint(bool toggleEnabled)
 {
-	bool enabled;
-	if (g_breakpoints.IsAddressBreakPoint(curAddress, &enabled)) {
+	// Route the breakpoint read to the CPU thread instead of poking at it directly from this GUI
+	// thread - see Core_RunOnCPUThread() in Core.h. The possible MessageBox() below is modal, so we
+	// can't just wrap the whole function - figure out what to do first, ask if needed, then mutate.
+	bool exists = false, enabled = false, hasCondition = false;
+	Core_RunOnCPUThread([&] {
+		exists = g_breakpoints.IsAddressBreakPoint(curAddress, &enabled);
+		if (exists)
+			hasCondition = g_breakpoints.GetBreakPointCondition(curAddress) != nullptr;
+	});
+
+	if (exists) {
 		if (!enabled) {
 			// enable disabled breakpoints
-			g_breakpoints.ChangeBreakPoint(curAddress, true);
-		} else if (!toggleEnabled && g_breakpoints.GetBreakPointCondition(curAddress) != nullptr) {
+			Core_RunOnCPUThread([&] { g_breakpoints.ChangeBreakPoint(curAddress, true); });
+		} else if (!toggleEnabled && hasCondition) {
 			// don't just delete a breakpoint with a custom condition
 			int ret = MessageBox(wnd,L"This breakpoint has a custom condition.\nDo you want to remove it?",L"Confirmation",MB_YESNO);
 			if (ret == IDYES)
-				g_breakpoints.RemoveBreakPoint(curAddress);
+				Core_RunOnCPUThread([&] { g_breakpoints.RemoveBreakPoint(curAddress); });
 		} else if (toggleEnabled) {
 			// disable breakpoint
-			g_breakpoints.ChangeBreakPoint(curAddress, false);
+			Core_RunOnCPUThread([&] { g_breakpoints.ChangeBreakPoint(curAddress, false); });
 		} else {
 			// otherwise just remove breakpoint
-			g_breakpoints.RemoveBreakPoint(curAddress);
+			Core_RunOnCPUThread([&] { g_breakpoints.RemoveBreakPoint(curAddress); });
 		}
 	} else {
-		g_breakpoints.AddBreakPoint(curAddress);
+		Core_RunOnCPUThread([&] { g_breakpoints.AddBreakPoint(curAddress); });
 	}
 }
 
@@ -912,13 +942,17 @@ void CtrlDisAsmView::CopyFunctionHash(u32 addr) {
 
 
 void CtrlDisAsmView::NopInstructions(u32 selectRangeStart, u32 selectRangeEnd) {
-	for (u32 addr = selectRangeStart; addr < selectRangeEnd; addr += 4) {
-		Memory::Write_U32(0, addr);
-	}
+	// Route the memory writes to the CPU thread instead of poking at it directly from this GUI
+	// thread - see Core_RunOnCPUThread() in Core.h.
+	Core_RunOnCPUThread([&] {
+		for (u32 addr = selectRangeStart; addr < selectRangeEnd; addr += 4) {
+			Memory::Write_U32(0, addr);
+		}
 
-	if (currentMIPS) {
-		currentMIPS->InvalidateICache(selectRangeStart, selectRangeEnd - selectRangeStart);
-	}
+		if (currentMIPS) {
+			currentMIPS->InvalidateICache(selectRangeStart, selectRangeEnd - selectRangeStart);
+		}
+	});
 }
 
 void CtrlDisAsmView::onMouseUp(WPARAM wParam, LPARAM lParam, int button)
@@ -970,16 +1004,22 @@ void CtrlDisAsmView::onMouseUp(WPARAM wParam, LPARAM lParam, int button)
 			break;
 		case ID_DISASM_EDITSYMBOLS:
 			{
+				// esw.exec() is a modal dialog - must stay outside any queued callback, or we'd
+				// block the CPU thread on user input. Only the resulting mutation is routed to the
+				// CPU thread - see Core_RunOnCPUThread() in Core.h.
 				EditSymbolsWindow esw(wnd, debugger);
 				if (esw.exec()) {
-					esw.eval();
+					Core_RunOnCPUThread([&] { esw.eval(); });
 					SendMessage(GetParent(wnd), WM_DEB_MAPLOADED, 0, 0);
 					redraw();
 				}
 			}
 			break;
 		case ID_DISASM_SETPCTOHERE:
-			debugger->SetPC(curAddress);
+			{
+				u32 newPC = curAddress;
+				Core_RunOnCPUThread([&] { debugger->SetPC(newPC); });
+			}
 			redraw();
 			break;
 		case ID_DISASM_FOLLOWBRANCH:
@@ -993,18 +1033,29 @@ void CtrlDisAsmView::onMouseUp(WPARAM wParam, LPARAM lParam, int button)
 			break;
 		case ID_DISASM_RENAMEFUNCTION:
 			{
-				u32 funcBegin = g_symbolMap->GetFunctionStart(curAddress);
+				// Route the symbol map reads/mutations to the CPU thread instead of poking at it
+				// directly from this GUI thread - see Core_RunOnCPUThread() in Core.h.
+				// InputBox_GetString() is modal, so it must stay outside any queued callback, or
+				// we'd block the CPU thread on user input.
+				u32 funcBegin = -1;
+				char name[256] = {0};
+				Core_RunOnCPUThread([&] {
+					funcBegin = g_symbolMap->GetFunctionStart(curAddress);
+					if (funcBegin != -1)
+						truncate_cpy(name, g_symbolMap->GetLabelString(funcBegin));
+				});
+
 				if (funcBegin != -1)
 				{
-					char name[256];
 					std::string newname;
-					truncate_cpy(name, g_symbolMap->GetLabelString(funcBegin));
 					if (InputBox_GetString(MainWindow::GetHInstance(), MainWindow::GetHWND(), L"New function name", name, newname)) {
-						g_symbolMap->SetLabelName(newname.c_str(), funcBegin);
-						u32 funcSize = g_symbolMap->GetFunctionSize(funcBegin);
-						MIPSAnalyst::RegisterFunction(funcBegin, funcSize, newname.c_str());
-						MIPSAnalyst::UpdateHashMap();
-						MIPSAnalyst::ApplyHashMap();
+						Core_RunOnCPUThread([&] {
+							g_symbolMap->SetLabelName(newname.c_str(), funcBegin);
+							u32 funcSize = g_symbolMap->GetFunctionSize(funcBegin);
+							MIPSAnalyst::RegisterFunction(funcBegin, funcSize, newname.c_str());
+							MIPSAnalyst::UpdateHashMap();
+							MIPSAnalyst::ApplyHashMap();
+						});
 						SendMessage(GetParent(wnd),WM_DEB_MAPLOADED,0,0);
 						redraw();
 					}
@@ -1017,25 +1068,37 @@ void CtrlDisAsmView::onMouseUp(WPARAM wParam, LPARAM lParam, int button)
 			break;
 		case ID_DISASM_REMOVEFUNCTION:
 			{
-				char statusBarTextBuff[256];
-				u32 funcBegin = g_symbolMap->GetFunctionStart(curAddress);
-				if (funcBegin != -1)
-				{
-					u32 prevBegin = g_symbolMap->GetFunctionStart(funcBegin-1);
-					if (prevBegin != -1)
+				// Route the symbol map reads/mutations to the CPU thread instead of poking at it
+				// directly from this GUI thread - see Core_RunOnCPUThread() in Core.h. SendMessage()
+				// must stay outside the lambda: this GUI thread is blocked waiting for the CPU
+				// thread while the lambda runs, so a SendMessage() to one of our own windows from
+				// inside it would deadlock.
+				bool found = false;
+				Core_RunOnCPUThread([&] {
+					u32 funcBegin = g_symbolMap->GetFunctionStart(curAddress);
+					if (funcBegin != -1)
 					{
-						u32 expandedSize = g_symbolMap->GetFunctionSize(prevBegin) + g_symbolMap->GetFunctionSize(funcBegin);
-						g_symbolMap->SetFunctionSize(prevBegin,expandedSize);
-					}
-					
-					g_symbolMap->RemoveFunction(funcBegin,true);
-					g_symbolMap->SortSymbols();
-					g_disassemblyManager.clear();
+						found = true;
+						u32 prevBegin = g_symbolMap->GetFunctionStart(funcBegin-1);
+						if (prevBegin != -1)
+						{
+							u32 expandedSize = g_symbolMap->GetFunctionSize(prevBegin) + g_symbolMap->GetFunctionSize(funcBegin);
+							g_symbolMap->SetFunctionSize(prevBegin,expandedSize);
+						}
 
+						g_symbolMap->RemoveFunction(funcBegin,true);
+						g_symbolMap->SortSymbols();
+						g_disassemblyManager.clear();
+					}
+				});
+
+				if (found)
+				{
 					SendMessage(GetParent(wnd), WM_DEB_MAPLOADED, 0, 0);
 				}
 				else
 				{
+					char statusBarTextBuff[256];
 					snprintf(statusBarTextBuff,256, "WARNING: unable to find function symbol here");
 					SendMessage(GetParent(wnd), WM_DEB_SETSTATUSBARTEXT, 0, (LPARAM) statusBarTextBuff);
 				}
@@ -1044,39 +1107,49 @@ void CtrlDisAsmView::onMouseUp(WPARAM wParam, LPARAM lParam, int button)
 			break;
 		case ID_DISASM_ADDFUNCTION:
 			{
-				char statusBarTextBuff[256];
-				u32 prevBegin = g_symbolMap->GetFunctionStart(curAddress);
-				if (prevBegin != -1)
-				{
-					if (prevBegin == curAddress)
+				// Same SendMessage-must-stay-outside-the-lambda reasoning as ID_DISASM_REMOVEFUNCTION
+				// above.
+				bool alreadyExists = false;
+				Core_RunOnCPUThread([&] {
+					u32 prevBegin = g_symbolMap->GetFunctionStart(curAddress);
+					if (prevBegin != -1)
 					{
-						snprintf(statusBarTextBuff,256, "WARNING: There's already a function entry point at this adress");
-						SendMessage(GetParent(wnd), WM_DEB_SETSTATUSBARTEXT, 0, (LPARAM) statusBarTextBuff);
+						if (prevBegin == curAddress)
+						{
+							alreadyExists = true;
+						}
+						else
+						{
+							char symname[128];
+							u32 prevSize = g_symbolMap->GetFunctionSize(prevBegin);
+							u32 newSize = curAddress-prevBegin;
+							g_symbolMap->SetFunctionSize(prevBegin,newSize);
+
+							newSize = prevSize-newSize;
+							snprintf(symname,128,"u_un_%08X",curAddress);
+							g_symbolMap->AddFunction(symname,curAddress,newSize);
+							g_symbolMap->SortSymbols();
+							g_disassemblyManager.clear();
+						}
 					}
 					else
 					{
 						char symname[128];
-						u32 prevSize = g_symbolMap->GetFunctionSize(prevBegin);
-						u32 newSize = curAddress-prevBegin;
-						g_symbolMap->SetFunctionSize(prevBegin,newSize);
-
-						newSize = prevSize-newSize;
-						snprintf(symname,128,"u_un_%08X",curAddress);
-						g_symbolMap->AddFunction(symname,curAddress,newSize);
+						int newSize = selectRangeEnd - selectRangeStart;
+						snprintf(symname, 128, "u_un_%08X", selectRangeStart);
+						g_symbolMap->AddFunction(symname, selectRangeStart, newSize);
 						g_symbolMap->SortSymbols();
-						g_disassemblyManager.clear();
-
-						SendMessage(GetParent(wnd), WM_DEB_MAPLOADED, 0, 0);
 					}
+				});
+
+				if (alreadyExists)
+				{
+					char statusBarTextBuff[256];
+					snprintf(statusBarTextBuff,256, "WARNING: There's already a function entry point at this adress");
+					SendMessage(GetParent(wnd), WM_DEB_SETSTATUSBARTEXT, 0, (LPARAM) statusBarTextBuff);
 				}
 				else
 				{
-					char symname[128];
-					int newSize = selectRangeEnd - selectRangeStart;
-					snprintf(symname, 128, "u_un_%08X", selectRangeStart);
-					g_symbolMap->AddFunction(symname, selectRangeStart, newSize);
-					g_symbolMap->SortSymbols();
-
 					SendMessage(GetParent(wnd), WM_DEB_MAPLOADED, 0, 0);
 				}
 				redraw();

Windows/Debugger/CtrlMemView.cpp

@@ -5,6 +5,7 @@
 #include "ext/xxhash.h"
 #include "Common/StringUtils.h"
 #include "Core/Config.h"
+#include "Core/Core.h"
 #include "Core/System.h"
 #include "Core/MemMap.h"
 #include "Core/Reporting.h"
@@ -416,7 +417,7 @@ void CtrlMemView::onKeyDown(WPARAM wParam, LPARAM lParam) {
 }
 
 void CtrlMemView::onChar(WPARAM wParam, LPARAM lParam) {
-	auto memLock = Memory::Lock();
+	Memory::MemoryInitedLock memLock = Memory::Lock();
 	if (!PSP_IsInited())
 		return;
 
@@ -428,12 +429,10 @@ void CtrlMemView::onChar(WPARAM wParam, LPARAM lParam) {
 		return;
 	}
 
-	bool active = Core_IsActive();
-	if (active)
-		Core_Break(BreakReason::MemoryAccess, curAddress_);
-
+	// Route the actual memory write to the CPU thread instead of poking at it directly from this
+	// GUI thread - see Core_RunOnCPUThread() in Core.h. No need to force the core to pause first.
 	if (asciiSelected_) {
-		Memory::WriteUnchecked_U8((u8)wParam, curAddress_);
+		Core_RunOnCPUThread([&] { Memory::WriteUnchecked_U8((u8)wParam, curAddress_); });
 		ScrollCursor(1, GotoMode::RESET);
 	} else {
 		wParam = tolower(wParam);
@@ -445,17 +444,17 @@ void CtrlMemView::onChar(WPARAM wParam, LPARAM lParam) {
 		if (inputValue >= 0) {
 			int shiftAmount = (1 - selectedNibble_) * 4;
 
-			u8 oldValue = Memory::ReadUnchecked_U8(curAddress_);
-			oldValue &= ~(0xF << shiftAmount);
-			u8 newValue = oldValue | (inputValue << shiftAmount);
-			Memory::WriteUnchecked_U8(newValue, curAddress_);
+			Core_RunOnCPUThread([&] {
+				u8 oldValue = Memory::ReadUnchecked_U8(curAddress_);
+				oldValue &= ~(0xF << shiftAmount);
+				u8 newValue = oldValue | (inputValue << shiftAmount);
+				Memory::WriteUnchecked_U8(newValue, curAddress_);
+			});
 			ScrollCursor(1, GotoMode::RESET);
 		}
 	}
 
 	Reporting::NotifyDebugger();
-	if (active)
-		Core_Resume();
 }
 
 void CtrlMemView::redraw() {

Windows/Debugger/CtrlRegisterList.cpp

@@ -3,6 +3,7 @@
 #include "Common/System/Display.h"
 #include "Common/Data/Encoding/Utf8.h"
 #include "Core/Config.h"
+#include "Core/Core.h"
 #include "Core/MemMap.h"
 #include "Core/Reporting.h"
 #include "Windows/W32Util/ContextMenu.h"
@@ -416,7 +417,11 @@ void CtrlRegisterList::editRegisterValue()
 	}
 
 	char temp[24];
-	u32 val = getSelectedRegValue(temp, 24);
+	// Route the register read/mutation to the CPU thread instead of poking at it directly from
+	// this GUI thread - see Core_RunOnCPUThread() in Core.h. InputBox_GetString() is modal, so it
+	// must stay outside any queued callback, or we'd block the CPU thread on user input.
+	u32 val = 0;
+	Core_RunOnCPUThread([&] { val = getSelectedRegValue(temp, 24); });
 	int reg = selection;
 
 	std::string value = temp;
@@ -424,22 +429,24 @@ void CtrlRegisterList::editRegisterValue()
 		if (parseExpression(value.c_str(),cpu,val) == false) {
 			displayExpressionError(wnd);
 		} else {
-			switch (reg)
-			{
-			case REGISTER_PC:
-				cpu->SetPC(val);
-				break;
-			case REGISTER_HI:
-				cpu->SetHi(val);
-				break;
-			case REGISTER_LO:
-				cpu->SetLo(val);
-				break;
-			default:
-				cpu->SetRegValue(category, reg, val);
-				break;
-			}
-			Reporting::NotifyDebugger();
+			Core_RunOnCPUThread([&] {
+				switch (reg)
+				{
+				case REGISTER_PC:
+					cpu->SetPC(val);
+					break;
+				case REGISTER_HI:
+					cpu->SetHi(val);
+					break;
+				case REGISTER_LO:
+					cpu->SetLo(val);
+					break;
+				default:
+					cpu->SetRegValue(category, reg, val);
+					break;
+				}
+				Reporting::NotifyDebugger();
+			});
 			redraw();
 			SendMessage(GetParent(wnd),WM_DEB_UPDATE,0,0);	// registers changed -> disassembly needs to be updated
 		}

Windows/Debugger/Debugger_Disasm.cpp

@@ -206,7 +206,9 @@ void CDisasm::step(CPUStepType stepType) {
 	ptr->setDontRedraw(true);
 	lastTicks_ = CoreTiming::GetTicks();
 
-	Core_RequestCPUStep(stepType, 1);
+	// Route the actual step request to the CPU thread instead of poking at it directly from this
+	// GUI thread - see Core_RunOnCPUThread() in Core.h.
+	Core_RunOnCPUThread([&] { Core_RequestCPUStep(stepType, 1); });
 }
 
 void CDisasm::runToLine() {
@@ -219,7 +221,9 @@ void CDisasm::runToLine() {
 
 	lastTicks_ = CoreTiming::GetTicks();
 	ptr->setDontRedraw(true);
-	breakpoints_->AddBreakPoint(pos,true);
+	// Route the breakpoint mutation to the CPU thread instead of poking at it directly from this
+	// GUI thread - see Core_RunOnCPUThread() in Core.h. Core_Resume() itself is free-threaded.
+	Core_RunOnCPUThread([&] { breakpoints_->AddBreakPoint(pos,true); });
 	Core_Resume();
 }
 
@@ -294,17 +298,16 @@ BOOL CDisasm::DlgProc(UINT message, WPARAM wParam, LPARAM lParam) {
 					CtrlDisAsmView *view = DisAsmView();
 					keepStatusBarText = true;
 					view->LockPosition();
-					bool isRunning = Core_IsActive();
-					if (isRunning) {
-						Core_Break(BreakReason::AddBreakpoint, 0);
-						Core_WaitInactive();
-					}
 
 					BreakpointWindow bpw(m_hDlg,cpu);
-					if (bpw.exec()) bpw.addBreakpoint();
+					if (bpw.exec()) {
+						// Route the actual breakpoint mutation to the CPU thread instead of poking
+						// at it directly from this GUI thread - see Core_RunOnCPUThread() in
+						// Core.h. No need to force the core to pause first: Core_RunOnCPUThread()
+						// runs the callback whether the CPU is running or already stepping.
+						Core_RunOnCPUThread([&] { bpw.addBreakpoint(); });
+					}
 
-					if (isRunning)
-						Core_Resume();
 					view->UnlockPosition();
 					keepStatusBarText = false;
 				}
@@ -422,7 +425,10 @@ BOOL CDisasm::DlgProc(UINT message, WPARAM wParam, LPARAM lParam) {
 						break;
 					lastTicks_ = CoreTiming::GetTicks();
 
-					hleDebugBreak();
+					// Route the actual HLE-break mutation to the CPU thread instead of poking at
+					// it directly from this GUI thread - see Core_RunOnCPUThread() in Core.h.
+					// Core_Resume() itself is free-threaded.
+					Core_RunOnCPUThread([&] { hleDebugBreak(); });
 					Core_Resume();
 				}
 				break;

Windows/Debugger/Debugger_Lists.cpp

@@ -11,6 +11,7 @@
 #include "Windows/resource.h"
 #include "Windows/main.h"
 #include "Common/Data/Encoding/Utf8.h"
+#include "Core/Core.h"
 #include "Core/HLE/sceKernelThread.h"
 
 enum { TL_NAME, TL_PROGRAMCOUNTER, TL_ENTRYPOINT, TL_PRIORITY, TL_STATE, TL_WAITTYPE, TL_COLUMNCOUNT };
@@ -148,11 +149,19 @@ void CtrlThreadList::showMenu(int itemIndex, const POINT &pt)
 	switch (TriggerContextMenu(ContextMenuID::THREADLIST, GetHandle(), ContextPoint::FromClient(pt)))
 	{
 	case ID_DISASM_THREAD_FORCERUN:
-		__KernelResumeThreadFromWait(threadInfo.id, 0);
+		{
+			// Route the actual thread-state mutation to the CPU thread instead of poking at it
+			// directly from this GUI thread - see Core_RunOnCPUThread() in Core.h.
+			SceUID threadID = threadInfo.id;
+			Core_RunOnCPUThread([&] { __KernelResumeThreadFromWait(threadID, 0); });
+		}
 		reloadThreads();
 		break;
 	case ID_DISASM_THREAD_KILL:
-		sceKernelTerminateThread(threadInfo.id);
+		{
+			SceUID threadID = threadInfo.id;
+			Core_RunOnCPUThread([&] { sceKernelTerminateThread(threadID); });
+		}
 		reloadThreads();
 		break;
 	}
@@ -342,6 +351,9 @@ void CtrlBreakpointList::editBreakpoint(int itemIndex)
 	int index = getBreakpointIndex(itemIndex, isMemory);
 	if (index == -1) return;
 
+	// Route the breakpoint mutation to the CPU thread instead of poking at it directly from this
+	// GUI thread - see Core_RunOnCPUThread() in Core.h. win.exec() is a modal dialog, so it must
+	// stay outside any queued callback, or we'd block the CPU thread on user input.
 	BreakpointWindow win(GetHandle(),cpu);
 	if (isMemory)
 	{
@@ -349,16 +361,20 @@ void CtrlBreakpointList::editBreakpoint(int itemIndex)
 		win.loadFromMemcheck(mem);
 		if (win.exec())
 		{
-			g_breakpoints.RemoveMemCheck(mem.start,mem.end);
-			win.addBreakpoint();
+			Core_RunOnCPUThread([&] {
+				g_breakpoints.RemoveMemCheck(mem.start,mem.end);
+				win.addBreakpoint();
+			});
 		}
 	} else {
 		auto bp = displayedBreakPoints_[index];
 		win.loadFromBreakpoint(bp);
 		if (win.exec())
 		{
-			g_breakpoints.RemoveBreakPoint(bp.addr);
-			win.addBreakpoint();
+			Core_RunOnCPUThread([&] {
+				g_breakpoints.RemoveBreakPoint(bp.addr);
+				win.addBreakpoint();
+			});
 		}
 	}
 }
@@ -369,12 +385,14 @@ void CtrlBreakpointList::toggleEnabled(int itemIndex)
 	int index = getBreakpointIndex(itemIndex, isMemory);
 	if (index == -1) return;
 
+	// Route the breakpoint mutation to the CPU thread instead of poking at it directly from this
+	// GUI thread - see Core_RunOnCPUThread() in Core.h.
 	if (isMemory) {
 		MemCheck mcPrev = displayedMemChecks_[index];
-		g_breakpoints.ChangeMemCheck(mcPrev.start, mcPrev.end, mcPrev.cond, BreakAction(mcPrev.result ^ BREAK_ACTION_PAUSE));
+		Core_RunOnCPUThread([&] { g_breakpoints.ChangeMemCheck(mcPrev.start, mcPrev.end, mcPrev.cond, BreakAction(mcPrev.result ^ BREAK_ACTION_PAUSE)); });
 	} else {
 		BreakPoint bpPrev = displayedBreakPoints_[index];
-		g_breakpoints.ChangeBreakPoint(bpPrev.addr, BreakAction(bpPrev.result ^ BREAK_ACTION_PAUSE));
+		Core_RunOnCPUThread([&] { g_breakpoints.ChangeBreakPoint(bpPrev.addr, BreakAction(bpPrev.result ^ BREAK_ACTION_PAUSE)); });
 	}
 }
 
@@ -404,12 +422,14 @@ void CtrlBreakpointList::removeBreakpoint(int itemIndex)
 	int index = getBreakpointIndex(itemIndex,isMemory);
 	if (index == -1) return;
 
+	// Route the breakpoint mutation to the CPU thread instead of poking at it directly from this
+	// GUI thread - see Core_RunOnCPUThread() in Core.h.
 	if (isMemory) {
 		auto mc = displayedMemChecks_[index];
-		g_breakpoints.RemoveMemCheck(mc.start, mc.end);
+		Core_RunOnCPUThread([&] { g_breakpoints.RemoveMemCheck(mc.start, mc.end); });
 	} else {
 		u32 address = displayedBreakPoints_[index].addr;
-		g_breakpoints.RemoveBreakPoint(address);
+		Core_RunOnCPUThread([&] { g_breakpoints.RemoveBreakPoint(address); });
 	}
 }
 
@@ -583,8 +603,12 @@ void CtrlBreakpointList::showBreakpointMenu(int itemIndex, const POINT &pt)
 		{
 		case ID_DISASM_ADDNEWBREAKPOINT:
 			{		
+				// bpw.exec() is a modal dialog - must stay outside any queued callback, or we'd
+				// block the CPU thread on user input - see Core_RunOnCPUThread() in Core.h.
 				BreakpointWindow bpw(GetHandle(),cpu);
-				if (bpw.exec()) bpw.addBreakpoint();
+				if (bpw.exec()) {
+					Core_RunOnCPUThread([&] { bpw.addBreakpoint(); });
+				}
 			}
 			break;
 		}
@@ -607,10 +631,12 @@ void CtrlBreakpointList::showBreakpointMenu(int itemIndex, const POINT &pt)
 		switch (TriggerContextMenu(ContextMenuID::BREAKPOINTLIST, GetHandle(), ContextPoint::FromClient(pt)))
 		{
 		case ID_DISASM_DISABLEBREAKPOINT:
+			// Route the breakpoint mutation to the CPU thread instead of poking at it directly
+			// from this GUI thread - see Core_RunOnCPUThread() in Core.h.
 			if (isMemory) {
-				g_breakpoints.ChangeMemCheck(mcPrev.start, mcPrev.end, mcPrev.cond, BreakAction(mcPrev.result ^ BREAK_ACTION_PAUSE));
+				Core_RunOnCPUThread([&] { g_breakpoints.ChangeMemCheck(mcPrev.start, mcPrev.end, mcPrev.cond, BreakAction(mcPrev.result ^ BREAK_ACTION_PAUSE)); });
 			} else {
-				g_breakpoints.ChangeBreakPoint(bpPrev.addr, BreakAction(bpPrev.result ^ BREAK_ACTION_PAUSE));
+				Core_RunOnCPUThread([&] { g_breakpoints.ChangeBreakPoint(bpPrev.addr, BreakAction(bpPrev.result ^ BREAK_ACTION_PAUSE)); });
 			}
 			break;
 		case ID_DISASM_EDITBREAKPOINT:
@@ -618,8 +644,12 @@ void CtrlBreakpointList::showBreakpointMenu(int itemIndex, const POINT &pt)
 			break;
 		case ID_DISASM_ADDNEWBREAKPOINT:
 			{		
+				// bpw.exec() is a modal dialog - must stay outside any queued callback, or we'd
+				// block the CPU thread on user input - see Core_RunOnCPUThread() in Core.h.
 				BreakpointWindow bpw(GetHandle(),cpu);
-				if (bpw.exec()) bpw.addBreakpoint();
+				if (bpw.exec()) {
+					Core_RunOnCPUThread([&] { bpw.addBreakpoint(); });
+				}
 			}
 			break;
 		case ID_DISASM_DELETEBREAKPOINT:

Windows/Debugger/DumpMemoryWindow.cpp

@@ -80,49 +80,51 @@ INT_PTR CALLBACK DumpMemoryWindow::dlgFunc(HWND hwnd, UINT iMsg, WPARAM wParam,
 			break;
 		case IDOK:
 			if (bp->fetchDialogData(hwnd)) {
-				bool priorDumpWasStepping = Core_IsStepping();
-				if (!priorDumpWasStepping && PSP_IsInited()) {
-					// If emulator isn't paused force paused state, but wait before locking.
-					Core_Break(BreakReason::MemoryAccess, bp->start);
-					Core_WaitInactive();
-				}
+				bool includeReplacements = SendMessage(GetDlgItem(hwnd, IDC_DUMP_INCLUDEHACKS), BM_GETCHECK, 0, 0) != 0;
 
-				auto memLock = Memory::Lock();
-				if (!PSP_IsInited())
-					break;
+				// Route the actual memory dump to the CPU thread instead of forcing the emulator
+				// to pause and poking at it directly from this GUI thread - see
+				// Core_RunOnCPUThread() in Core.h. MessageBoxA() is modal, so it stays outside the
+				// queued callback.
+				enum class Outcome { NotInited, OpenFailed, Success } outcome = Outcome::NotInited;
+				Core_RunOnCPUThread([&] {
+					Memory::MemoryInitedLock memLock = Memory::Lock();
+					if (!PSP_IsInited())
+						return;
 
-				FILE *output = _wfopen(bp->fileName_.c_str(), L"wb");
-				if (output == nullptr) {
+					FILE *output = _wfopen(bp->fileName_.c_str(), L"wb");
+					if (output == nullptr) {
+						outcome = Outcome::OpenFailed;
+						return;
+					}
+
+					if (includeReplacements) {
+						fwrite(Memory::GetPointer(bp->start), 1, bp->size, output);
+					} else {
+						auto savedReplacements = SaveAndClearReplacements();
+						std::lock_guard<std::recursive_mutex> guard(MIPSComp::jitLock);
+						if (MIPSComp::jit) {
+							auto savedBlocks = MIPSComp::jit->SaveAndClearEmuHackOps();
+							fwrite(Memory::GetPointer(bp->start), 1, bp->size, output);
+							MIPSComp::jit->RestoreSavedEmuHackOps(savedBlocks);
+						} else {
+							fwrite(Memory::GetPointer(bp->start), 1, bp->size, output);
+						}
+						RestoreSavedReplacements(savedReplacements);
+					}
+
+					fclose(output);
+					outcome = Outcome::Success;
+				});
+
+				if (outcome == Outcome::OpenFailed) {
 					char errorMessage[2048];
 					snprintf(errorMessage, sizeof(errorMessage), "Could not open file \"%S\".", bp->fileName_.c_str());
 					MessageBoxA(hwnd, errorMessage, "Error", MB_OK);
-					break;
+				} else if (outcome == Outcome::Success) {
+					MessageBoxA(hwnd, "Done.", "Information", MB_OK);
+					EndDialog(hwnd, true);
 				}
-
-				bool includeReplacements = SendMessage(GetDlgItem(hwnd, IDC_DUMP_INCLUDEHACKS), BM_GETCHECK, 0, 0) != 0;
-				if (includeReplacements) {
-					fwrite(Memory::GetPointer(bp->start), 1, bp->size, output);
-				} else {
-					auto savedReplacements = SaveAndClearReplacements();
-					std::lock_guard<std::recursive_mutex> guard(MIPSComp::jitLock);
-					if (MIPSComp::jit) {
-						auto savedBlocks = MIPSComp::jit->SaveAndClearEmuHackOps();
-						fwrite(Memory::GetPointer(bp->start), 1, bp->size, output);
-						MIPSComp::jit->RestoreSavedEmuHackOps(savedBlocks);
-					} else {
-						fwrite(Memory::GetPointer(bp->start), 1, bp->size, output);
-					}
-					RestoreSavedReplacements(savedReplacements);
-				}
-
-				fclose(output);
-				if (!priorDumpWasStepping) {
-					// If emulator wasn't paused before memory dump resume emulation automatically.
-					Core_Resume();
-				}
-
-				MessageBoxA(hwnd, "Done.", "Information", MB_OK);
-				EndDialog(hwnd, true);
 			}
 			break;
 		case IDCANCEL: