Fix mutex2 deadlock and ResumeEvent spam with conditional breakpoints

[xusheng6]

Summary

Fixes #1037

• Mutex2 deadlock: When a conditional breakpoint evaluates to false, the auto-resume path (continue at line 2069) 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. Fix: check m_userRequestedBreak before auto-resuming so an explicit Pause is honored over the breakpoint condition.
• ResumeEventType warning spam: Adapters that post ResumeEventType synchronously inside Go() (WindowsNativeAdapter, GdbAdapter, EsrevenAdapter, CorelliumAdapter) trigger re-entrant warnings when called from the dispatcher thread. Fix: suppress the unnecessary ResumeEventType during auto-resume via a flag checked in PostDebuggerEvent. Adapters that post asynchronously (LLDB, DbgEng) are unaffected.

Test plan

• Open helloworld_thread binary for Windows x86_64
• Launch with native Windows adapter
• Continue past initial breakpoint
• Pause, set a conditional breakpoint like rax == 0x1234
• Continue — verify no "type 1 posted from dispatcher" warnings
• Pause — verify it succeeds without "Cannot obtain mutex2" errors
• Restart/quit — verify these also work correctly

🤖 Generated with Claude Code

[plafosse]

Actually had Codex review and it may have found something:

Suppressing ResumeEventType during the conditional-breakpoint auto-resume leaves the controller in the paused state while the target is actually running. In the auto-resume path, DebuggerMainThread explicitly sets m_state->SetExecutionStatus(DebugAdapterPausedStatus) before evaluating the condition core/debuggercontroller.cpp, around line 2044), and the only normal path back to DebugAdapterRunningStatus is EventHandler on ResumeEventType (core/debuggercontroller.cpp, around lines 1850-1856). This PR drops that event in PostDebuggerEvent, so DebuggerState::IsRunning() stays false (core/debuggerstate.h, line 311). That breaks logic that depends on the running bit, for example CanResumeTarget() (core/debuggercontroller.cpp, lines 525-527) and QuitAndWait()'s decision to pause first. The warning goes away, but the controller/UI state can now say "paused" while the process is running.

In other words the specific problem is that auto-resume starts from a forced paused state during conditional-breakpoint evaluation, and ResumeEventType is what normally flips the controller back to running. If this PR suppresses that event without also controller state another way, the debugger can think the target is paused while it is actually running. That is a behavioral regression, not just a style concern.

A safer fix would need one of these:

1. Keep suppressing only the UI-facing warning, but still preserve the internal running-state transition.
2. Set the controller state to running explicitly before or after the direct m_adapter->Go() auto-resume path.
3. Separate “internal state update” from “dispatch callbacks/UI event” so the PR can suppress the noisy re-entrant
   callback without losing the state change.

[xusheng6]

Actually had Codex review and it may have found something:

Suppressing ResumeEventType during the conditional-breakpoint auto-resume leaves the controller in the paused state while the target is actually running. In the auto-resume path, DebuggerMainThread explicitly sets m_state->SetExecutionStatus(DebugAdapterPausedStatus) before evaluating the condition core/debuggercontroller.cpp, around line 2044), and the only normal path back to DebugAdapterRunningStatus is EventHandler on ResumeEventType (core/debuggercontroller.cpp, around lines 1850-1856). This PR drops that event in PostDebuggerEvent, so DebuggerState::IsRunning() stays false (core/debuggerstate.h, line 311). That breaks logic that depends on the running bit, for example CanResumeTarget() (core/debuggercontroller.cpp, lines 525-527) and QuitAndWait()'s decision to pause first. The warning goes away, but the controller/UI state can now say "paused" while the process is running.

In other words the specific problem is that auto-resume starts from a forced paused state during conditional-breakpoint evaluation, and ResumeEventType is what normally flips the controller back to running. If this PR suppresses that event without also controller state another way, the debugger can think the target is paused while it is actually running. That is a behavioral regression, not just a style concern.

A safer fix would need one of these:

1. Keep suppressing only the UI-facing warning, but still preserve the internal running-state transition.

2. Set the controller state to running explicitly before or after the direct m_adapter->Go() auto-resume path.

3. Separate “internal state update” from “dispatch callbacks/UI event” so the PR can suppress the noisy re-entrant
   callback without losing the state change.

Nice catch! This indeed breaks the running/stopped status. In my testing, I did not see it because the various widgets are not updated if the controller does not report a true stop. However, if I close and open the debugger sidebar, I can see it reports the target status as stopped, which is unintended.

I have added code to set the target status as running when we decide to resume the target at the conditional breakpoint site

[plafosse]

I think Codex is right here:

Suppressing ResumeEventType during conditional-breakpoint auto-resume still looks incorrect because it drops the
controller’s only transition back to “running”. In the breakpoint-stop path, the controller explicitly sets paused state
first (public/debugger/core/debuggercontroller.cpp:2044). The normal path back to running is EventHandler handling
ResumeEventType (public/debugger/core/debuggercontroller.cpp:1850). IsRunning() is just m_targetStatus == DebugAdapterRunningStatus (public/debugger/core/debuggerstate.h:311), and resume gating depends on that (public/debugger/core/debuggercontroller.cpp:525). If this PR suppresses the resume event without also updating controller state another way, the target can be running while the controller still believes it is paused. That is a real behavioral regression, not just warning suppression.

Suggestions on how to fix:
• A safer fix is to separate “internal state transition to running” from “queue a ResumeEventType callback”.

Right now the PR suppresses the entire ResumeEventType, but EventHandler is what flips the controller back to running. The
minimal fix is:

1. Keep the m_userRequestedBreak guard in the conditional-breakpoint auto-resume path.
2. Do not blindly drop ResumeEventType in PostDebuggerEvent.
3. Before or immediately after the direct m_adapter->Go() auto-resume, explicitly restore controller state:

m_state->SetConnectionStatus(DebugAdapterConnectedStatus);
m_state->SetExecutionStatus(DebugAdapterRunningStatus);
m_state->MarkDirty();

4. Then suppress only the noisy re-entrant callback/UI dispatch for that specific synchronous adapter-generated
   ResumeEventType.

In practice, the cleanest version is usually:

• Add a helper like SetRunningStateForResume() for the internal state update.
• Add a narrowly-scoped flag for “ignore the next synchronous ResumeEventType callback from auto-resume”.
• In PostDebuggerEvent, if that flag is set and event.type == ResumeEventType, skip enqueuing the event, but only because the
  controller state was already updated locally.

That preserves:

• the deadlock fix
• the warning suppression
• correct IsRunning() behavior

What I would not do is keep using ResumeEventType as both:

• the internal state transition mechanism
• the UI notification mechanism

That coupling is what made this patch fragile in the first place.