diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index ceaf547ebddaf..c8475db8ae160 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -465,6 +465,8 @@ class Process : public std::enable_shared_from_this, 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; } diff --git a/lldb/include/lldb/Utility/Event.h b/lldb/include/lldb/Utility/Event.h index 461d711b8c3f2..4f58f257d4a26 100644 --- a/lldb/include/lldb/Utility/Event.h +++ b/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; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 6fac0df1d7a66..7139385fe1eca 100644 --- a/lldb/source/Target/Process.cpp +++ b/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) diff --git a/lldb/source/Utility/Event.cpp b/lldb/source/Utility/Event.cpp index 863167e56bce6..5f431c0a6dd89 100644 --- a/lldb/source/Utility/Event.cpp +++ b/lldb/source/Utility/Event.cpp @@ -83,14 +83,20 @@ void Event::Dump(Stream *s) const { void Event::DoOnRemoval() { std::lock_guard 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 - diff --git a/lldb/test/API/python_api/event/TestEvents.py b/lldb/test/API/python_api/event/TestEvents.py index d8d3dd2d2b01b..fb1a7e3bc6d3a 100644 --- a/lldb/test/API/python_api/event/TestEvents.py +++ b/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")