Private process events were being delivered to the secondary listener

lldb/include/lldb/Target/Process.h

@@ -465,6 +465,8 @@ class Process : public std::enable_shared_from_this<Process>,
     static bool SetUpdateStateOnRemoval(Event *event_ptr);
 
   private:
+    bool ForwardEventToPendingListeners(Event *event_ptr) override;
+
     void SetUpdateStateOnRemoval() { m_update_state++; }
 
     void SetRestarted(bool new_value) { m_restarted = new_value; }

lldb/include/lldb/Utility/Event.h

@@ -48,6 +48,17 @@ class EventData {
   virtual void Dump(Stream *s) const;
 
 private:
+  /// This will be queried for a Broadcaster with a primary and some secondary
+  /// listeners after the primary listener pulled the event from the event queue
+  /// and ran its DoOnRemoval, right before the event is delivered.
+  /// If it returns true, the event will also be forwarded to the secondary
+  /// listeners, and if false, event propagation stops at the primary listener.
+  /// Some broadcasters (particularly the Process broadcaster) fetch events on
+  /// a private Listener, and then forward the event to the Public Listeners
+  /// after some processing.  The Process broadcaster does not want to forward
+  /// to the secondary listeners at the private processing stage.
+  virtual bool ForwardEventToPendingListeners(Event *event_ptr) { return true; }
+
   virtual void DoOnRemoval(Event *event_ptr) {}
 
   EventData(const EventData &) = delete;

lldb/source/Target/Process.cpp

@@ -4248,7 +4248,22 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
   return still_should_stop;
 }
 
+bool Process::ProcessEventData::ForwardEventToPendingListeners(
+    Event *event_ptr) {
+  // STDIO and the other async event notifications should always be forwarded.
+  if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
+    return true;
+
+  // 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;
+}
+
 void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {
+  // We only have work to do for state changed events:
+  if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
+    return;
+
   ProcessSP process_sp(m_process_wp.lock());
 
   if (!process_sp)

lldb/source/Utility/Event.cpp

@@ -83,14 +83,20 @@ void Event::Dump(Stream *s) const {
 void Event::DoOnRemoval() {
   std::lock_guard<std::mutex> guard(m_listeners_mutex);
 
-  if (m_data_sp)
-    m_data_sp->DoOnRemoval(this);
+  if (!m_data_sp)
+    return;
+
+  m_data_sp->DoOnRemoval(this);
+
   // Now that the event has been handled by the primary event Listener, forward
   // it to the other Listeners.
+
   EventSP me_sp = shared_from_this();
-  for (auto listener_sp : m_pending_listeners)
-    listener_sp->AddEvent(me_sp);
-  m_pending_listeners.clear();
+  if (m_data_sp->ForwardEventToPendingListeners(this)) {
+    for (auto listener_sp : m_pending_listeners)
+      listener_sp->AddEvent(me_sp);
+    m_pending_listeners.clear();
+  }
 }
 
 #pragma mark -

lldb/test/API/python_api/event/TestEvents.py

@@ -7,7 +7,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+import random
 
 @skipIfLinux  # llvm.org/pr25924, sometimes generating SIGSEGV
 class EventAPITestCase(TestBase):
@@ -20,6 +20,7 @@ def setUp(self):
         self.line = line_number(
             "main.c", '// Find the line number of function "c" here.'
         )
+        random.seed()
 
     @expectedFailureAll(
         oslist=["linux"], bugnumber="llvm.org/pr23730 Flaky, fails ~1/10 cases"
@@ -318,6 +319,7 @@ def wait_for_next_event(self, expected_state, test_shadow=False):
         """Wait for an event from self.primary & self.shadow listener.
         If test_shadow is true, we also check that the shadow listener only
         receives events AFTER the primary listener does."""
+        import stop_hook
         # Waiting on the shadow listener shouldn't have events yet because
         # we haven't fetched them for the primary listener yet:
         event = lldb.SBEvent()
@@ -328,12 +330,23 @@ def wait_for_next_event(self, expected_state, test_shadow=False):
 
         # But there should be an event for the primary listener:
         success = self.primary_listener.WaitForEvent(5, event)
+
         self.assertTrue(success, "Primary listener got the event")
 
         state = lldb.SBProcess.GetStateFromEvent(event)
+        primary_event_type = event.GetType()
         restart = False
         if state == lldb.eStateStopped:
             restart = lldb.SBProcess.GetRestartedFromEvent(event)
+            # This counter is matching the stop hooks, which don't get run
+            # for auto-restarting stops.
+            if not restart:
+                self.stop_counter += 1
+                self.assertEqual(
+                    stop_hook.StopHook.counter[self.instance],
+                    self.stop_counter,
+                    "matching stop hook",
+                )
 
         if expected_state is not None:
             self.assertEqual(
@@ -344,15 +357,18 @@ def wait_for_next_event(self, expected_state, test_shadow=False):
         # listener:
         success = self.shadow_listener.WaitForEvent(5, event)
         self.assertTrue(success, "Shadow listener got event too")
+        shadow_event_type = event.GetType()
+        self.assertEqual(
+            primary_event_type, shadow_event_type, "It was the same event type"
+        )
         self.assertEqual(
-            state, lldb.SBProcess.GetStateFromEvent(event), "It was the same event"
+            state, lldb.SBProcess.GetStateFromEvent(event), "It was the same state"
         )
         self.assertEqual(
             restart,
             lldb.SBProcess.GetRestartedFromEvent(event),
             "It was the same restarted",
         )
-
         return state, restart
 
     @expectedFlakeyLinux("llvm.org/pr23730")  # Flaky, fails ~1/100 cases
@@ -386,6 +402,20 @@ def test_shadow_listener(self):
         )
         self.dbg.SetAsync(True)
 
+        # Now make our stop hook - we want to ensure it stays up to date with
+        # the events.  We can't get our hands on the stop-hook instance directly,
+        # so we'll pass in an instance key, and use that to retrieve the data from
+        # this instance of the stop hook:
+        self.instance = f"Key{random.randint(0,10000)}"
+        stop_hook_path = os.path.join(self.getSourceDir(), "stop_hook.py")
+        self.runCmd(f"command script import {stop_hook_path}")
+        import stop_hook
+
+        self.runCmd(
+            f"target stop-hook add -P stop_hook.StopHook -k instance -v {self.instance}"
+        )
+        self.stop_counter = 0
+
         self.process = target.Launch(launch_info, error)
         self.assertSuccess(error, "Process launched successfully")
 
@@ -395,6 +425,7 @@ def test_shadow_listener(self):
         # Events in the launch sequence might be platform dependent, so don't
         # expect any particular event till we get the stopped:
         state = lldb.eStateInvalid
+
         while state != lldb.eStateStopped:
             state, restart = self.wait_for_next_event(None, False)
 
@@ -412,8 +443,6 @@ def test_shadow_listener(self):
             self.cur_thread.GetStopReasonDataAtIndex(0),
             "Hit the right breakpoint",
         )
-        # Disable the first breakpoint so it doesn't get in the way...
-        bkpt1.SetEnabled(False)
 
         self.cur_thread.StepOver()
         # We'll run the test for "shadow listener blocked by primary listener
@@ -450,4 +479,9 @@ def test_shadow_listener(self):
             )
             if state == lldb.eStateStopped and not restarted:
                 self.process.Continue()
+
             state, restarted = self.wait_for_next_event(None, False)
+
+        # Now make sure we agree with the stop hook counter:
+        self.assertEqual(self.stop_counter, stop_hook.StopHook.counter[self.instance])
+        self.assertEqual(stop_hook.StopHook.non_stops[self.instance], 0, "No non stops")