diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -352,6 +352,9 @@ eBroadcastBitProfileData = (1 << 4), eBroadcastBitStructuredData = (1 << 5), }; + // This is all the event bits the public process broadcaster broadcasts. + // The process shadow listener signs up for all these bits... + static const int g_all_event_bits; enum { eBroadcastInternalStateControlStop = (1 << 0), @@ -383,10 +386,6 @@ return GetStaticBroadcasterClass(); } - void SetShadowListener(lldb::ListenerSP listener_sp) override { - Broadcaster::SetShadowListener(listener_sp); - } - /// A notification structure that can be used by clients to listen /// for changes in a process's lifetime. /// @@ -610,6 +609,15 @@ return error; } + /// The "ShadowListener" for a process is just an ordinary Listener that + /// listens for all the Process event bits. It's convenient because you can + /// specify it in the LaunchInfo or AttachInfo, so it will get events from + /// the very start of the process. + void SetShadowListener(lldb::ListenerSP shadow_listener_sp) { + if (shadow_listener_sp) + AddListener(shadow_listener_sp, g_all_event_bits); + } + // FUTURE WORK: GetLoadImageUtilityFunction are the first use we've // had of having other plugins cache data in the Process. This is handy for // long-living plugins - like the Platform - which manage interactions whose diff --git a/lldb/include/lldb/Utility/Broadcaster.h b/lldb/include/lldb/Utility/Broadcaster.h --- a/lldb/include/lldb/Utility/Broadcaster.h +++ b/lldb/include/lldb/Utility/Broadcaster.h @@ -312,8 +312,8 @@ lldb::BroadcasterManagerSP GetManager(); - virtual void SetShadowListener(lldb::ListenerSP listener_sp) { - m_broadcaster_sp->m_shadow_listener = listener_sp; + virtual void SetPrimaryListener(lldb::ListenerSP listener_sp) { + m_broadcaster_sp->m_primary_listener_sp = listener_sp; } protected: @@ -409,6 +409,27 @@ /// event bit. event_names_map m_event_names; + /// A Broadcaster can have zero, one or many listeners. A Broadcaster with + /// zero listeners is a no-op, with one Listener is trivial. + /// In most cases of multiple Listeners,the Broadcaster treats all its + /// Listeners as equal, sending each event to all of the Listeners in no + /// guaranteed order. + /// However, some Broadcasters - in particular the Process broadcaster, can + /// designate one Listener to be the "Primary Listener". In the case of + /// the Process Broadcaster, the Listener passed to the Process constructor + /// will be the Primary Listener. + /// If the broadcaster has a Primary Listener, then the event gets + /// sent first to the Primary Listener, and then when the Primary Listener + /// pulls the event and the the event's DoOnRemoval finishes running, + /// the event is forwarded to all the other Listeners. + /// The other wrinkle is that a Broadcaster may be serving a Hijack + /// Listener. If the Hijack Listener is present, events are only sent to + /// the Hijack Listener. We use that, for instance, to absorb all the + /// events generated by running an expression so that they don't show up to + /// the driver or UI as starts and stops. + /// If a Broadcaster has both a Primary and a Hijack Listener, the top-most + /// Hijack Listener is treated as the current Primary Listener. + /// A list of Listener / event_mask pairs that are listening to this /// broadcaster. collection m_listeners; @@ -416,6 +437,9 @@ /// A mutex that protects \a m_listeners. std::recursive_mutex m_listeners_mutex; + /// See the discussion of Broadcasters and Listeners above. + lldb::ListenerSP m_primary_listener_sp; + /// A simple mechanism to intercept events from a broadcaster std::vector m_hijacking_listeners; @@ -423,10 +447,6 @@ /// for now this is just for private hijacking. std::vector m_hijacking_masks; - /// A optional listener that all private events get also broadcasted to, - /// on top the hijacked / default listeners. - lldb::ListenerSP m_shadow_listener = nullptr; - private: BroadcasterImpl(const BroadcasterImpl &) = delete; const BroadcasterImpl &operator=(const BroadcasterImpl &) = delete; diff --git a/lldb/include/lldb/Utility/Event.h b/lldb/include/lldb/Utility/Event.h --- a/lldb/include/lldb/Utility/Event.h +++ b/lldb/include/lldb/Utility/Event.h @@ -176,7 +176,7 @@ }; // lldb::Event -class Event { +class Event : public std::enable_shared_from_this { friend class Listener; friend class EventData; friend class Broadcaster::BroadcasterImpl; @@ -225,6 +225,12 @@ } void Clear() { m_data_sp.reset(); } + + /// This is used by Broadcasters with Primary Listeners to store the other + /// Listeners till after the Event's DoOnRemoval has completed. + void AddPendingListener(lldb::ListenerSP pending_listener_sp) { + m_pending_listeners.push_back(pending_listener_sp); + }; private: // This is only called by Listener when it pops an event off the queue for @@ -244,6 +250,8 @@ m_broadcaster_wp; // The broadcaster that sent this event uint32_t m_type; // The bit describing this event lldb::EventDataSP m_data_sp; // User specific data for this event + std::vector m_pending_listeners; + std::mutex m_listeners_mutex; Event(const Event &) = delete; const Event &operator=(const Event &) = delete; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -369,6 +369,12 @@ g_process_properties[idx].default_uint_value)); } +const int Process::g_all_event_bits = eBroadcastBitStateChanged + | eBroadcastBitInterrupt + | eBroadcastBitSTDOUT | eBroadcastBitSTDERR + | eBroadcastBitProfileData + | eBroadcastBitStructuredData; + ProcessSP Process::FindPlugin(lldb::TargetSP target_sp, llvm::StringRef plugin_name, ListenerSP listener_sp, @@ -475,10 +481,9 @@ m_private_state_control_broadcaster.SetEventName( eBroadcastInternalStateControlResume, "control-resume"); - m_listener_sp->StartListeningForEvents( - this, eBroadcastBitStateChanged | eBroadcastBitInterrupt | - eBroadcastBitSTDOUT | eBroadcastBitSTDERR | - eBroadcastBitProfileData | eBroadcastBitStructuredData); + // The listener passed into process creation is the primary listener: + // It always listens for all the event bits for Process: + SetPrimaryListener(m_listener_sp); m_private_state_listener_sp->StartListeningForEvents( &m_private_state_broadcaster, @@ -4114,8 +4119,8 @@ if (!still_should_stop && does_anybody_have_an_opinion) { // We've been asked to continue, so do that here. SetRestarted(true); - // Use the public resume method here, since this is just extending a - // public resume. + // Use the private resume method here, since we aren't changing the run + // lock state. process_sp->PrivateResume(); } else { bool hijacked = process_sp->IsHijackedForEvent(eBroadcastBitStateChanged) && diff --git a/lldb/source/Utility/Broadcaster.cpp b/lldb/source/Utility/Broadcaster.cpp --- a/lldb/source/Utility/Broadcaster.cpp +++ b/lldb/source/Utility/Broadcaster.cpp @@ -151,6 +151,10 @@ if (!m_hijacking_listeners.empty() && event_type & m_hijacking_masks.back()) return true; + // The primary listener listens for all event bits: + if (m_primary_listener_sp) + return true; + for (auto &pair : GetListeners()) { if (pair.second & event_type) return true; @@ -222,14 +226,36 @@ event_description.GetData(), unique, static_cast(hijacking_listener_sp.get())); } - - if (hijacking_listener_sp) { - if (unique && hijacking_listener_sp->PeekAtNextEventForBroadcasterWithType( + ListenerSP primary_listener_sp = hijacking_listener_sp; + bool is_hijack = false; + + if (primary_listener_sp) + is_hijack = true; + else + primary_listener_sp = m_primary_listener_sp; + + if (primary_listener_sp) { + if (unique && primary_listener_sp->PeekAtNextEventForBroadcasterWithType( &m_broadcaster, event_type)) return; - hijacking_listener_sp->AddEvent(event_sp); - if (m_shadow_listener) - m_shadow_listener->AddEvent(event_sp); + // Add the pending listeners but not if the event is hijacked, since that + // is given sole access to the event stream it is hijacking. + // Make sure to do this before adding the event to the primary or it might + // start handling the event before we're done adding all the pending + // listeners. + if (!is_hijack) { + for (auto &pair : GetListeners()) { + if (!(pair.second & event_type)) + continue; + if (unique && pair.first->PeekAtNextEventForBroadcasterWithType( + &m_broadcaster, event_type)) + continue; + if (pair.first != hijacking_listener_sp + && pair.first != m_primary_listener_sp) + event_sp->AddPendingListener(pair.first); + } + } + primary_listener_sp->AddEvent(event_sp); } else { for (auto &pair : GetListeners()) { if (!(pair.second & event_type)) @@ -239,8 +265,6 @@ continue; pair.first->AddEvent(event_sp); - if (m_shadow_listener) - m_shadow_listener->AddEvent(event_sp); } } } diff --git a/lldb/source/Utility/Event.cpp b/lldb/source/Utility/Event.cpp --- a/lldb/source/Utility/Event.cpp +++ b/lldb/source/Utility/Event.cpp @@ -11,6 +11,7 @@ #include "lldb/Utility/Broadcaster.h" #include "lldb/Utility/DataExtractor.h" #include "lldb/Utility/Endian.h" +#include "lldb/Utility/Listener.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" #include "lldb/lldb-enumerations.h" @@ -80,8 +81,16 @@ } void Event::DoOnRemoval() { + std::lock_guard guard(m_listeners_mutex); + if (m_data_sp) 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(); } #pragma mark - diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp --- a/lldb/source/Utility/Listener.cpp +++ b/lldb/source/Utility/Listener.cpp @@ -231,8 +231,7 @@ // to return it so it should be okay to get the next event off the queue // here - and it might be useful to do that in the "DoOnRemoval". lock.unlock(); - if (!m_is_shadow) - event_sp->DoOnRemoval(); + event_sp->DoOnRemoval(); } return true; } diff --git a/lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py b/lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py --- a/lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py +++ b/lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py @@ -23,7 +23,7 @@ self.script_file = self.script_module + ".py" @skipUnlessDarwin - @skipIfDarwin + #@skipIfDarwin def test_passthrough_launch(self): """Test a simple pass-through process launch""" self.passthrough_launch() @@ -34,6 +34,15 @@ self.assertSuccess(error, "Resuming multiplexer scripted process") self.assertTrue(self.mux_process.IsValid(), "Got a valid process") + event = lldbutil.fetch_next_event( + self, self.dbg.GetListener(), self.mux_process.GetBroadcaster() + ) + self.assertState(lldb.SBProcess.GetStateFromEvent(event), lldb.eStateRunning) + event = lldbutil.fetch_next_event( + self, self.dbg.GetListener(), self.mux_process.GetBroadcaster() + ) + self.assertState(lldb.SBProcess.GetStateFromEvent(event), lldb.eStateStopped) + event = lldbutil.fetch_next_event( self, self.mux_process_listener, self.mux_process.GetBroadcaster() ) @@ -44,7 +53,7 @@ self.assertState(lldb.SBProcess.GetStateFromEvent(event), lldb.eStateStopped) @skipUnlessDarwin - @skipIfDarwin + #@skipIfDarwin def test_multiplexed_launch(self): """Test a multiple interactive scripted process debugging""" self.passthrough_launch() @@ -178,6 +187,13 @@ ) execution_events[event_target_idx][state] = True + for _ in range((self.dbg.GetNumTargets() - 1) * 2): + fetch_process_event(self, execution_events) + + for target_index, event_states in execution_events.items(): + for state, is_set in event_states.items(): + self.assertTrue(is_set, f"Target {target_index} has state {state} set") + event = lldbutil.fetch_next_event( self, self.mux_process_listener, self.mux_process.GetBroadcaster() ) @@ -188,13 +204,6 @@ ) self.assertState(lldb.SBProcess.GetStateFromEvent(event), lldb.eStateStopped) - for _ in range((self.dbg.GetNumTargets() - 1) * 2): - fetch_process_event(self, execution_events) - - for target_index, event_states in execution_events.items(): - for state, is_set in event_states.items(): - self.assertTrue(is_set, f"Target {target_index} has state {state} set") - def duplicate_target(self, driving_target): exe = driving_target.executable.fullpath triple = driving_target.triple @@ -248,14 +257,14 @@ self.assertSuccess(error, "Launched multiplexer scripted process") self.assertTrue(self.mux_process.IsValid(), "Got a valid process") - # Check that the mux process started running + # Check that the real process started running event = lldbutil.fetch_next_event( - self, self.mux_process_listener, self.mux_process.GetBroadcaster() + self, self.dbg.GetListener(), self.mux_process.GetBroadcaster() ) self.assertState(lldb.SBProcess.GetStateFromEvent(event), lldb.eStateRunning) - # Check that the real process started running + # Check that the mux process started running event = lldbutil.fetch_next_event( - self, self.dbg.GetListener(), self.mux_process.GetBroadcaster() + self, self.mux_process_listener, self.mux_process.GetBroadcaster() ) self.assertState(lldb.SBProcess.GetStateFromEvent(event), lldb.eStateRunning) diff --git a/lldb/test/API/python_api/event/TestEvents.py b/lldb/test/API/python_api/event/TestEvents.py --- a/lldb/test/API/python_api/event/TestEvents.py +++ b/lldb/test/API/python_api/event/TestEvents.py @@ -313,3 +313,121 @@ self.assertEqual( self.state, "stopped", "Both expected state changed events received" ) + + 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.""" + # Waiting on the shadow listener shouldn't have events yet because + # we haven't fetched them for the primary listener yet: + event = lldb.SBEvent() + + if test_shadow: + success = self.shadow_listener.WaitForEvent(1, event) + self.assertFalse(success, "Shadow listener doesn't pull events") + + # 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) + restart = False + if state == lldb.eStateStopped: + restart = lldb.SBProcess.GetRestartedFromEvent(event) + + if expected_state != None: + self.assertEqual(state, expected_state, "Primary thread got the correct event") + + # And after pulling that one there should be an equivalent event for the shadow + # listener: + success = self.shadow_listener.WaitForEvent(5, event) + self.assertTrue(success, "Shadow listener got event too") + self.assertEqual(state, lldb.SBProcess.GetStateFromEvent(event), "It was the same event") + self.assertEqual(restart, lldb.SBProcess.GetRestartedFromEvent(event), "It was the same restarted") + + return state, restart + + def test_shadow_listener(self): + self.build() + exe = self.getBuildArtifact("a.out") + + # Create a target by the debugger. + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + # Now create a breakpoint on main.c by name 'c'. + bkpt1 = target.BreakpointCreateByName("c", "a.out") + self.trace("breakpoint:", bkpt1) + self.assertTrue(bkpt1.GetNumLocations() == 1, VALID_BREAKPOINT) + + self.primary_listener = lldb.SBListener("my listener") + self.shadow_listener = lldb.SBListener("shadow listener") + + self.cur_thread = None + + error = lldb.SBError() + launch_info = target.GetLaunchInfo() + launch_info.SetListener(self.primary_listener) + launch_info.SetShadowListener(self.shadow_listener) + + self.runCmd("settings set target.process.extra-startup-command QSetLogging:bitmask=LOG_PROCESS|LOG_EXCEPTIONS|LOG_RNB_PACKETS|LOG_STEP;") + self.dbg.SetAsync(True) + + self.process = target.Launch(launch_info, error) + self.assertSuccess(error, "Process launched successfully") + + # Keep fetching events from the primary to trigger the do on removal and + # then from the shadow listener, and make sure they match: + + # 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) + + # Okay, we're now at a good stop, so try a next: + self.cur_thread = self.process.threads[0] + + # Make sure we're at our expected breakpoint: + self.assertTrue(self.cur_thread.IsValid(), "Got a zeroth thread") + self.assertEqual(self.cur_thread.stop_reason, lldb.eStopReasonBreakpoint) + self.assertEqual(self.cur_thread.GetStopReasonDataCount(), 2, "Only one breakpoint/loc here") + self.assertEqual(bkpt1.GetID(), 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 + # for the first couple rounds, then we'll skip the 1 second pause... + self.wait_for_next_event(lldb.eStateRunning, True) + self.wait_for_next_event(lldb.eStateStopped, True) + + # Next try an auto-continue breakpoint and make sure the shadow listener got + # the resumed info as well. Note that I'm not explicitly counting + # running events here. At the point when I wrote this lldb sometimes + # emits two running events in a row. Apparently the code to coalesce running + # events isn't working. But that's not what this test is testing, we're really + # testing that the primary & shadow listeners hear the same thing and in the + # right order. + + main_spec = lldb.SBFileSpec("main.c") + bkpt2 = target.BreakpointCreateBySourceRegex("b.2. returns %d", main_spec) + self.assertTrue(bkpt2.GetNumLocations() > 0, "BP2 worked") + bkpt2.SetAutoContinue(True) + + bkpt3 = target.BreakpointCreateBySourceRegex("a.3. returns %d", main_spec) + self.assertTrue(bkpt3.GetNumLocations() > 0, "BP3 worked") + + state = lldb.eStateStopped + restarted = False + + # Put in a counter to make sure we don't spin forever if there is some + # error in the logic. + counter = 0 + while state != lldb.eStateExited: + counter += 1 + self.assertLess(counter, 50, "Took more than 50 events to hit two breakpoints.") + if state == lldb.eStateStopped and not restarted: + self.process.Continue() + state, restarted = self.wait_for_next_event(None, False) +