[Lldb-commits] [lldb] lldb] Fix two issues causing TestEvents.py flakiness (PR #194438)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/194438

>From 338c6c7dee616256830d503952b31aa92f77cf83 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 27 Apr 2026 11:17:12 -0700
Subject: [PATCH 1/2] lldb] Fix two issues causing TestEvents.py flakiness

This PR fixes two issues that contributed to TestEvents.py being flaky
in CI:

1. `ProcessEventData::DoOnRemoval` runs the full stop-handling logic
   (like `ShouldStop` and `RunStopHooks`) every time an event is
   consumed from any listener. When the primary listener consumes an
   event and then the shadow listener consumes the same event, the logic
   runs twice. The second execution can race with subsequent event
   processing. Fix this by incrementing `m_update_state` after the first
   successful run so secondary listeners skip the full logic.

2. `Target::RunStopHooks` updates `m_latest_stop_hook_id` (marking a
   stop as "handled") before checking whether any threads have stop
   reasons. If the check fails and hooks don't run, the stop ID is
   already consumed, preventing hooks from ever running for that stop.
   Fix this by deferring the update until we're certain we'll actually
   run hooks.
---
 lldb/source/Target/Process.cpp | 14 +++++++++-----
 lldb/source/Target/Target.cpp  |  4 ++--
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 53b3b480ad4d2..6e5722a2ad437 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4481,7 +4481,9 @@ bool Process::ProcessEventData::ForwardEventToPendingListeners(
 
   // For state changed events, if the update state is zero, we are handling
   // this on the private state thread.  We should wait for the public event.
-  return m_update_state == 1;
+  // After the primary listener processes it, m_update_state becomes 2, which
+  // is when we forward to pending (secondary) listeners.
+  return m_update_state >= 1;
 }
 
 void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {
@@ -4498,12 +4500,14 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {
   // pulled off of the private process event queue, and then any number of
   // times, first when it gets pulled off of the public event queue, then other
   // times when we're pretending that this is where we stopped at the end of
-  // expression evaluation.  m_update_state is used to distinguish these three
-  // cases; it is 0 when we're just pulling it off for private handling, and >
-  // 1 for expression evaluation, and we don't want to do the breakpoint
-  // command handling then.
+  // expression evaluation.  m_update_state is used to distinguish these
+  // cases; it is 0 when we're just pulling it off for private handling, 1
+  // when the primary public listener consumes it, and > 1 after that (e.g.
+  // secondary listeners or expression evaluation) where we don't want to
+  // redo the breakpoint command handling or stop hooks.
   if (m_update_state != 1)
     return;
+  m_update_state++;
 
   process_sp->SetPublicState(
       m_state, Process::ProcessEventData::GetRestartedFromEvent(event_ptr));
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 27ddc4e7e5092..8a518685e716b 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3162,8 +3162,6 @@ bool Target::RunStopHooks(bool at_initial_stop) {
   if (last_natural_stop != 0 && m_latest_stop_hook_id == last_natural_stop)
     return false;
 
-  m_latest_stop_hook_id = last_natural_stop;
-
   std::vector<ExecutionContext> exc_ctx_with_reasons;
 
   ThreadList &cur_threadlist = m_process_sp->GetThreadList();
@@ -3195,6 +3193,8 @@ bool Target::RunStopHooks(bool at_initial_stop) {
     }
   }
 
+  m_latest_stop_hook_id = last_natural_stop;
+
   StreamSP output_sp = m_debugger.GetAsyncOutputStream();
   llvm::scope_exit on_exit([output_sp] { output_sp->Flush(); });
 

>From 6cb712917533983ebdebd7bb28d36af68e8b0bfa Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 27 Apr 2026 13:57:44 -0700
Subject: [PATCH 2/2] Address Jim's point

---
 lldb/source/Target/Process.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6e5722a2ad437..5fb1998dfbf49 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4481,9 +4481,10 @@ bool Process::ProcessEventData::ForwardEventToPendingListeners(
 
   // For state changed events, if the update state is zero, we are handling
   // this on the private state thread.  We should wait for the public event.
-  // After the primary listener processes it, m_update_state becomes 2, which
-  // is when we forward to pending (secondary) listeners.
-  return m_update_state >= 1;
+  // After the primary listener processes it in DoOnRemoval, m_update_state
+  // is incremented from 1 to 2, which is when we forward to pending
+  // (secondary) listeners.
+  return m_update_state > 1;
 }
 
 void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {