From 8666c9e3c1494245aa3d1721affadb74ba0456cb Mon Sep 17 00:00:00 2001 From: Xusheng Date: Mon, 6 Apr 2026 17:01:03 +0800 Subject: [PATCH 1/2] Fix mutex2 deadlock and ResumeEvent spam with conditional breakpoints (#1037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a conditional breakpoint evaluates to false, the auto-resume path skips callback dispatch. If a Pause/Quit/Detach operation is pending, its callback never fires, its semaphore never releases, and m_adapterMutex2 is permanently locked — breaking all subsequent Pause/Quit/Detach operations. Fix by checking m_userRequestedBreak before auto-resuming so that an explicit Pause is honored over the breakpoint condition. Also suppress the unnecessary ResumeEventType that adapters post synchronously inside Go() during auto-resume, eliminating the dispatcher-thread re-entrant warning spam. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/debuggercontroller.cpp | 17 ++++++++++++++--- core/debuggercontroller.h | 4 ++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/core/debuggercontroller.cpp b/core/debuggercontroller.cpp index 1713fe19..83fd5044 100644 --- a/core/debuggercontroller.cpp +++ b/core/debuggercontroller.cpp @@ -1986,6 +1986,12 @@ bool DebuggerController::RemoveEventCallbackInternal(size_t index) void DebuggerController::PostDebuggerEvent(const DebuggerEvent& event) { + // During conditional breakpoint auto-resume, suppress the ResumeEventType that adapters + // post inside Go(). The target is already considered running by the UI, and posting this + // event from the dispatcher thread would trigger a re-entrant warning. + if (m_suppressResumeEvent && event.type == ResumeEventType) + return; + auto pending = std::make_shared(); pending->event = event; std::future future = pending->done.get_future(); @@ -2059,13 +2065,18 @@ void DebuggerController::DebuggerMainThread() if (uint64_t ip = m_state->IP(); !isStepOperation && m_state->GetBreakpoints()->ContainsAbsolute(ip)) { - if (!EvaluateBreakpointCondition(ip)) + if (!EvaluateBreakpointCondition(ip) && !m_userRequestedBreak) { m_lastAdapterStopEventConsumed = true; current->done.set_value(); - // using m_adapter->Go() directly instead of Go() to avoid mutex deadlock - // since we're already inside ExecuteAdapterAndWait's event processing + // Using m_adapter->Go() directly instead of Go() to avoid mutex deadlock + // since we're already inside ExecuteAdapterAndWait's event processing. + // Suppress the ResumeEventType that some adapters post synchronously inside + // Go() — the UI already considers the target running, and posting from the + // dispatcher thread would be unexpected. + m_suppressResumeEvent = true; m_adapter->Go(); + m_suppressResumeEvent = false; continue; } } diff --git a/core/debuggercontroller.h b/core/debuggercontroller.h index a7df4703..f298d902 100644 --- a/core/debuggercontroller.h +++ b/core/debuggercontroller.h @@ -121,6 +121,10 @@ namespace BinaryNinjaDebugger { bool m_lastAdapterStopEventConsumed = true; + // When true, ResumeEventType events are suppressed in PostDebuggerEvent. + // Used during conditional breakpoint auto-resume to avoid posting events from the dispatcher thread. + bool m_suppressResumeEvent = false; + bool m_inputFileLoaded = false; bool m_initialBreakpointSeen = false; From cdf70d5e8b0ca54177164bbc45f621c2a5f6c588 Mon Sep 17 00:00:00 2001 From: Xusheng Date: Tue, 7 Apr 2026 12:48:24 +0800 Subject: [PATCH 2/2] Make sure to set the status as running after we evaluate the conditional breakpoint and decide to resume the adapter --- core/debuggercontroller.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/core/debuggercontroller.cpp b/core/debuggercontroller.cpp index 83fd5044..629ea127 100644 --- a/core/debuggercontroller.cpp +++ b/core/debuggercontroller.cpp @@ -2077,6 +2077,7 @@ void DebuggerController::DebuggerMainThread() m_suppressResumeEvent = true; m_adapter->Go(); m_suppressResumeEvent = false; + m_state->SetExecutionStatus(DebugAdapterRunningStatus); continue; } }