mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-19 09:19:05 +02:00
Fix deadlock between the Win32 debugger's paint handlers and the CPU thread
Reported hang: the CPU thread held g_frameMutex (NativeFrame) and blocked on g_shutdownLock inside a queued memory.read, while the GUI thread held g_shutdownLock (CtrlMemView::onPaint) and blocked on g_frameMutex. Textbook ABBA. The CPU thread's order is structural - NativeFrame wraps everything below it in g_frameMutex, and both Core_ProcessCPUQueue() and runImDebugger() -> DisassembleRange() lock memory from under there - so the GUI side is the one that has to match. Swaps the three handlers that had it backwards (CtrlMemView::onPaint, CtrlDisAsmView::onPaint, CtrlStackTraceView:: loadStackTrace) to take g_frameMutex first. They already took both locks, so this is ordering only, and g_shutdownLock is recursive so nesting is fine. Also drops the Memory::MemoryInitedLock from the WebSocket LockMemory(), which is what made the CPU thread want that lock in the first place. It was guarding against another thread tearing down the memory system, but that doesn't happen: Memory::Shutdown() is only reached via CPU_Shutdown() <- PSP_Shutdown(), whose callers all run on the CPU thread, and Memory::Reinit() runs from Memory::DoState() on savestate load, likewise. WebSocket.cpp additionally holds lifecycleLock across the whole handler and takes it on STOPPING. Note this second part isn't sufficient on its own - ImMemView's copy-disassembly path also locks memory from inside the frame span - which is why the ordering fix is the real one. Not removing Memory::Lock() from the Win32 paint handlers: teardown isn't fully inside the g_frameMutex span yet. EmuScreen::render()'s PSP_Shutdown() is, but the ones in EmuScreen::sendMessage() (game reset, loading a new game) run from g_screenManager->sendMessage(), above where NativeFrame takes the guard. Closing that is the prerequisite, and is left for later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
This commit is contained in:
1 parent
8784b709e8
commit
81511e7a08
4 files changed
+17
-15
No files matched your search
@@ -53,7 +53,6 @@ struct AutoDisabledReplacements {
|
||||
AutoDisabledReplacements &operator =(const AutoDisabledReplacements &) = delete;
|
||||
~AutoDisabledReplacements();
|
||||
|
||||
Memory::MemoryInitedLock *lock = nullptr;
|
||||
std::map<u32, u32> replacements;
|
||||
std::vector<u32> emuhacks;
|
||||
bool saved = false;
|
||||
@@ -64,12 +63,13 @@ 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
|
||||
// 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.
|
||||
//
|
||||
// Important: Only use keepReplacements=false when reading, not writing.
|
||||
static AutoDisabledReplacements LockMemory(bool keepReplacements) {
|
||||
AutoDisabledReplacements result;
|
||||
// This still guards against a *different* thread (not the CPU thread) tearing down or
|
||||
// reinitializing the memory system underneath us, e.g. during shutdown.
|
||||
result.lock = new Memory::MemoryInitedLock();
|
||||
if (!keepReplacements) {
|
||||
result.saved = true;
|
||||
// Okay, save so we can restore later.
|
||||
@@ -82,8 +82,6 @@ static AutoDisabledReplacements LockMemory(bool keepReplacements) {
|
||||
}
|
||||
|
||||
AutoDisabledReplacements::AutoDisabledReplacements(AutoDisabledReplacements &&other) {
|
||||
lock = other.lock;
|
||||
other.lock = nullptr;
|
||||
replacements = std::move(other.replacements);
|
||||
emuhacks = std::move(other.emuhacks);
|
||||
saved = other.saved;
|
||||
@@ -96,7 +94,6 @@ AutoDisabledReplacements::~AutoDisabledReplacements() {
|
||||
MIPSComp::jit->RestoreSavedEmuHackOps(emuhacks);
|
||||
RestoreSavedReplacements(replacements);
|
||||
}
|
||||
delete lock;
|
||||
}
|
||||
|
||||
// Read a byte from memory (memory.read_u8)
|
||||
|
||||
Reference in new issue
Block a user