[lldb/Utility] Add opt-in shadow mode to event listeners

[mib]

Summary

This patch augments lldb's event listeners with a new shadow mode.

As the name suggests, this mode allows events to be copied to an
additional listener to perform event monitoring, without interferring
with the event life cycle.

One of our use case for this, is to be able to listen to public process
events while making sure the events will still be delivered to the
default process listener (the debugger listener in most cases).

Signed-off-by: Med Ismail Bennani <[e-mail]>

[bulbazord]

lldb/include/lldb/Utility/Broadcaster.h

520–522

It looks like Broadcaster can have multiple listeners. Do we want multiple passthrough listeners as well? If not, why only one?

lldb/source/API/SBAttachInfo.cpp

260

GetPassthroughListener can return nullptr. I think you want to take advantage of the protected SBListener(const ListenerSP &) constructor here but I think that if GetPassthroughListener returns nullptr you'll end up calling the constructor SBListener(const char *). I don't think this is what you want.

lldb/source/API/SBLaunchInfo.cpp

395

Same issue as SBAttachInfo::GetPassthroughListener

[JDevlieghere]

I find "passthrough" somewhat confusing in this context. When using a listener in this new mode, where does the event go after it has pass trough this event listener? Maybe my understanding of how the event listeners is incomplete, but I was under the impression that normally there's a single listener for a certain kind of event. IIUC, in this new mode, you can have multiple listeners, where you can set one (or as Alex pointed out, possibly more) listeners that can observe events but not necessary deal with them. If that's an accurate description of the interactions, then it seems like there's more of a one-to-many relationship between events and listeners rather than what passthrough implies. I think a better name would be something like "passive listener" or "non-handling" listener or something.

[mib]

In D148397#4275792, @JDevlieghere wrote:

I find "passthrough" somewhat confusing in this context. When using a listener in this new mode, where does the event go after it has pass trough this event listener? Maybe my understanding of how the event listeners is incomplete, but I was under the impression that normally there's a single listener for a certain kind of event. IIUC, in this new mode, you can have multiple listeners, where you can set one (or as Alex pointed out, possibly more) listeners that can observe events but not necessary deal with them. If that's an accurate description of the interactions, then it seems like there's more of a one-to-many relationship between events and listeners rather than what passthrough implies. I think a better name would be something like "passive listener" or "non-handling" listener or something.

@JDevlieghere You could also have multiple listeners for a certain type of event, but events would be considered "handled" only when they're popped from the listener passed during the process creation (or the debugger listener if no listener is provided in the {launch,attach}info). The idea behind passthrough listener was to have a side channel where all the events would be funneled to, that would never interfere with the default listener and therefore not change the process execution behavior. If you really feel strongly about the "passthrough" name, I can rename it to whatever you like but I feel like it's pretty self explanatory.

lldb/include/lldb/Utility/Broadcaster.h

520–522

That's a very good point. When I implemented this, I was thinking that the passthrough listener would act as a funnel for all the events. But having multiple passthrough listener could potentially allow the user to do some filtering on the event. @jingham what do you think ?

[mib]

Fix issue when returning a non-initialized passthrough listener from the SBAPI

[JDevlieghere]

In D148397#4285143, @mib wrote:

In D148397#4275792, @JDevlieghere wrote:

I find "passthrough" somewhat confusing in this context. When using a listener in this new mode, where does the event go after it has pass trough this event listener? Maybe my understanding of how the event listeners is incomplete, but I was under the impression that normally there's a single listener for a certain kind of event. IIUC, in this new mode, you can have multiple listeners, where you can set one (or as Alex pointed out, possibly more) listeners that can observe events but not necessary deal with them. If that's an accurate description of the interactions, then it seems like there's more of a one-to-many relationship between events and listeners rather than what passthrough implies. I think a better name would be something like "passive listener" or "non-handling" listener or something.

@JDevlieghere You could also have multiple listeners for a certain type of event, but events would be considered "handled" only when they're popped from the listener passed during the process creation (or the debugger listener if no listener is provided in the {launch,attach}info). The idea behind passthrough listener was to have a side channel where all the events would be funneled to, that would never interfere with the default listener and therefore not change the process execution behavior. If you really feel strongly about the "passthrough" name, I can rename it to whatever you like but I feel like it's pretty self explanatory.

To me, passthrough means something sitting in-between something else. It's still not clear to me what the passthrough listener is sitting in between. It sounds like it's meant to be a side channel, so maybe shadow listener would be a better name? I don't care so much about the name per se but rather about making it obvious what it does.

[mib]

"Shadow Listener" it is @JDevlieghere

[mib]

More renaming and reformat

[mib]

fix typo

[JDevlieghere]

LGTM

lldb/include/lldb/API/SBAttachInfo.h

public:
 
   /// Set the listener that will be used to receive process events.
   ///
   /// By default the SBDebugger, which has a listener, that the SBTarget
   /// belongs to will listen for the process events. Calling this function
   /// allows a different listener to be used to listen for process events.
   void SetListener(SBListener &listener);
 
+  /// Get the shadow listener that receive public process events,
+  /// additionally to the default process event listener.
+  ///
+  /// If no listener has been set via a call to
+  /// SBLaunchInfo::SetShadowListener(), then an invalid SBListener will
+  /// be returned (SBListener::IsValid() will return false). If a listener
+  /// has been set, then the valid listener object will be returned.
+  SBListener GetShadowListener();
+
+  /// Set the shadow listener that will receive public process events,
+  /// additionally to the default process event listener.
+  ///
+  /// By default a process have no passthrough event listener.
+  /// Calling this function allows public process events to be broadcasted to an
+  /// additional listener on top of the default process event listener.
+  /// If the `listener` argument is invalid (SBListener::IsValid() will
+  /// return false), this will clear the shadow listener.
+  void SetShadowListener(SBListener &listener);
+
   const char *GetScriptedProcessClassName() const;
 
   void SetScriptedProcessClassName(const char *class_name);
 
   lldb::SBStructuredData GetScriptedProcessDictionary() const;
 
   void SetScriptedProcessDictionary(lldb::SBStructuredData dict);
 

lldb/include/lldb/API/SBLaunchInfo.h

#endif
 
   /// Set the listener that will be used to receive process events.
   ///
   /// By default the SBDebugger, which has a listener, that the SBTarget
   /// belongs to will listen for the process events. Calling this function
   /// allows a different listener to be used to listen for process events.
   void SetListener(SBListener &listener);
 
+  /// Get the shadow listener that receive public process events,
+  /// additionally to the default process event listener.
+  ///
+  /// If no listener has been set via a call to
+  /// SBLaunchInfo::SetShadowListener(), then an invalid SBListener will
+  /// be returned (SBListener::IsValid() will return false). If a listener
+  /// has been set, then the valid listener object will be returned.
+  SBListener GetShadowListener();
+
+  /// Set the shadow listener that will receive public process events,
+  /// additionally to the default process event listener.
+  ///
+  /// By default a process have no passthrough event listener.
+  /// Calling this function allows public process events to be broadcasted to an
+  /// additional listener on top of the default process event listener.
+  /// If the `listener` argument is invalid (SBListener::IsValid() will
+  /// return false), this will clear the shadow listener.
+  void SetShadowListener(SBListener &listener);
+
   uint32_t GetNumArguments();
 
   const char *GetArgumentAtIndex(uint32_t idx);
 
   void SetArguments(const char **argv, bool append);
 
   uint32_t GetNumEnvironmentEntries();
 

lldb/include/lldb/Target/Process.h

static constexpr llvm::StringRef LaunchSynchronousHijackListenerName =
       "lldb.internal.Process.LaunchSynchronous.hijack";
   static constexpr llvm::StringRef ResumeSynchronousHijackListenerName =
       "lldb.internal.Process.ResumeSynchronous.hijack";
 
   ConstString &GetBroadcasterClass() const override {
     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.
 ///
 /// \see RegisterNotificationCallbacks (const Notifications&) @see
 /// UnregisterNotificationCallbacks (const Notifications&)
   typedef struct {
     void *baton;
     void (*initialize)(void *baton, Process *process);

lldb/include/lldb/Utility/Broadcaster.h

public:
   /// with the broadcaster manager and do broadcaster class matching.
   /// FIXME: Probably should make a ManagedBroadcaster subclass with all the
   /// bits needed to work with the BroadcasterManager, so that it is clearer
   /// how to add one.
   virtual ConstString &GetBroadcasterClass() const;
 
   lldb::BroadcasterManagerSP GetManager();
 
+  virtual void SetShadowListener(lldb::ListenerSP listener_sp) {
+    m_broadcaster_sp->m_shadow_listener = listener_sp;
+  }
+
 protected:
   /// BroadcasterImpl contains the actual Broadcaster implementation.  The
   /// Broadcaster makes a BroadcasterImpl which lives as long as it does.  The
   /// Listeners & the Events hold a weak pointer to the BroadcasterImpl, so
   /// that they can survive if a Broadcaster they were listening to is
   /// destroyed w/o their being able to unregister from it (which can happen if
   /// the Broadcasters & Listeners are being destroyed on separate threads
   /// simultaneously. The Broadcaster itself can't be shared out as a weak
protected:
 
     /// A simple mechanism to intercept events from a broadcaster
     std::vector<lldb::ListenerSP> m_hijacking_listeners;
 
     /// At some point we may want to have a stack or Listener collections, but
     /// for now this is just for private hijacking.
     std::vector<uint32_t> 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;
   };
 
   typedef std::shared_ptr<BroadcasterImpl> BroadcasterImplSP;
   typedef std::weak_ptr<BroadcasterImpl> BroadcasterImplWP;
 

lldb/include/lldb/Utility/Listener.h

public:
 
   bool GetEventForBroadcasterWithType(Broadcaster *broadcaster,
                                       uint32_t event_type_mask,
                                       lldb::EventSP &event_sp,
                                       const Timeout<std::micro> &timeout);
 
   size_t HandleBroadcastEvent(lldb::EventSP &event_sp);
 
+  void SetShadow(bool is_shadow) { m_is_shadow = is_shadow; }
+
 private:
   // Classes that inherit from Listener can see and modify these
   struct BroadcasterInfo {
     BroadcasterInfo(uint32_t mask, HandleBroadcastCallback cb = nullptr,
                     void *ud = nullptr)
         : event_mask(mask), callback(cb), callback_user_data(ud) {}
 
     uint32_t event_mask;
private:
 
   std::string m_name;
   broadcaster_collection m_broadcasters;
   std::recursive_mutex m_broadcasters_mutex; // Protects m_broadcasters
   event_collection m_events;
   std::mutex m_events_mutex; // Protects m_broadcasters and m_events
   std::condition_variable m_events_condition;
   broadcaster_manager_collection m_broadcaster_managers;
+  bool m_is_shadow = false;
 
   void BroadcasterWillDestruct(Broadcaster *);
 
   void BroadcasterManagerWillDestruct(lldb::BroadcasterManagerSP manager_sp);
 
   //    broadcaster_collection::iterator
   //    FindBroadcasterWithMask (Broadcaster *broadcaster,
   //                             uint32_t event_mask,

lldb/include/lldb/Utility/ProcessInfo.h

public:
   }
 
   lldb::ListenerSP GetHijackListener() const { return m_hijack_listener_sp; }
 
   void SetHijackListener(const lldb::ListenerSP &listener_sp) {
     m_hijack_listener_sp = listener_sp;
   }
 
+  lldb::ListenerSP GetShadowListener() const { return m_shadow_listener_sp; }
+
+  void SetShadowListener(const lldb::ListenerSP &listener_sp) {
+    m_shadow_listener_sp = listener_sp;
+  }
+
 protected:
   FileSpec m_executable;
   std::string m_arg0; // argv[0] if supported. If empty, then use m_executable.
   // Not all process plug-ins support specifying an argv[0] that differs from
   // the resolved platform executable (which is in m_executable)
   Args m_arguments; // All program arguments except argv[0]
   Environment m_environment;
   uint32_t m_uid = UINT32_MAX;
   uint32_t m_gid = UINT32_MAX;
   ArchSpec m_arch;
   lldb::pid_t m_pid = LLDB_INVALID_PROCESS_ID;
   lldb::ScriptedMetadataSP m_scripted_metadata_sp = nullptr;
   lldb::ListenerSP m_listener_sp = nullptr;
   lldb::ListenerSP m_hijack_listener_sp = nullptr;
+  lldb::ListenerSP m_shadow_listener_sp = nullptr;
 };
 
 // ProcessInstanceInfo
 //
 // Describes an existing process and any discoverable information that pertains
 // to that process.
 class ProcessInstanceInfo : public ProcessInfo {
 public:

lldb/source/API/SBAttachInfo.cpp

 }
 
 void SBAttachInfo::SetListener(SBListener &listener) {
   LLDB_INSTRUMENT_VA(this, listener);
 
   m_opaque_sp->SetListener(listener.GetSP());
 }
 
+SBListener SBAttachInfo::GetShadowListener() {
+  LLDB_INSTRUMENT_VA(this);
+
+  lldb::ListenerSP shadow_sp = m_opaque_sp->GetShadowListener();
+  if (!shadow_sp)
+    return SBListener();
+  return SBListener(shadow_sp);
+}
+
+void SBAttachInfo::SetShadowListener(SBListener &listener) {
+  LLDB_INSTRUMENT_VA(this, listener);
+
+  ListenerSP listener_sp = listener.GetSP();
+  if (listener_sp && listener.IsValid())
+    listener_sp->SetShadow(true);
+  else
+    listener_sp = nullptr;
+
+  m_opaque_sp->SetShadowListener(listener_sp);
+}
+
 const char *SBAttachInfo::GetScriptedProcessClassName() const {
   LLDB_INSTRUMENT_VA(this);
 
   ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
 
   if (!metadata_sp || !*metadata_sp)
     return nullptr;
 

lldb/source/API/SBLaunchInfo.cpp

 #include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStructuredData.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
+#include "lldb/Utility/Listener.h"
 #include "lldb/Utility/ScriptedMetadata.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 class lldb_private::SBLaunchInfoImpl : public ProcessLaunchInfo {
 public:
   SBLaunchInfoImpl() : m_envp(GetEnvironment().getEnvp()) {}
void SBLaunchInfo::SetScriptedProcessDictionary(lldb::SBStructuredData dict) {
   if (!dict_sp || dict_sp->GetType() == lldb::eStructuredDataTypeInvalid)
     return;
 
   ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
   llvm::StringRef class_name = metadata_sp ? metadata_sp->GetClassName() : "";
   metadata_sp = std::make_shared<ScriptedMetadata>(class_name, dict_sp);
   m_opaque_sp->SetScriptedMetadata(metadata_sp);
 }
+
+SBListener SBLaunchInfo::GetShadowListener() {
+  LLDB_INSTRUMENT_VA(this);
+
+  lldb::ListenerSP shadow_sp = m_opaque_sp->GetShadowListener();
+  if (!shadow_sp)
+    return SBListener();
+  return SBListener(shadow_sp);
+}
+
+void SBLaunchInfo::SetShadowListener(SBListener &listener) {
+  LLDB_INSTRUMENT_VA(this, listener);
+
+  ListenerSP listener_sp = listener.GetSP();
+  if (listener_sp && listener.IsValid())
+    listener_sp->SetShadow(true);
+  else
+    listener_sp = nullptr;
+
+  m_opaque_sp->SetShadowListener(listener_sp);
+}

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

if (target && error.Success()) {
       if (process_sp) {
         ListenerSP listener_sp = attach_info.GetHijackListener();
         if (listener_sp == nullptr) {
           listener_sp =
               Listener::MakeListener("lldb.PlatformPOSIX.attach.hijack");
           attach_info.SetHijackListener(listener_sp);
         }
         process_sp->HijackProcessEvents(listener_sp);
+        process_sp->SetShadowListener(attach_info.GetShadowListener());
         error = process_sp->Attach(attach_info);
       }
     }
   } else {
     if (m_remote_platform_sp)
       process_sp =
           m_remote_platform_sp->Attach(attach_info, debugger, target, error);
     else
if (!process_sp) {
     error.SetErrorString("CreateProcess() failed for gdb-remote process");
     LLDB_LOG(log, "error: {0}", error);
     return process_sp;
   }
 
   LLDB_LOG(log, "successfully created process");
 
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetShadowListener(launch_info.GetShadowListener());
 
   // Log file actions.
   if (log) {
     LLDB_LOG(log, "launching process with the following file actions:");
     StreamString stream;
     size_t i = 0;
     const FileAction *file_action;
     while ((file_action = launch_info.GetFileActionAtIndex(i++)) != nullptr) {

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

if (IsConnected()) {
       } else {
         // The darwin always currently uses the GDB remote debugger plug-in
         // so even when debugging locally we are debugging remotely!
         process_sp = target.CreateProcess(launch_info.GetListener(),
                                           "gdb-remote", nullptr, true);
 
         if (process_sp) {
           process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+          process_sp->SetShadowListener(launch_info.GetShadowListener());
 
           error = process_sp->ConnectRemote(connect_url.c_str());
           // Retry the connect remote one time...
           if (error.Fail())
             error = process_sp->ConnectRemote(connect_url.c_str());
           if (error.Success())
             error = process_sp->Launch(launch_info);
           else if (debugserver_pid != LLDB_INVALID_PROCESS_ID) {
if (IsConnected()) {
               target->CreateProcess(attach_info.GetListenerForProcess(debugger),
                                     "gdb-remote", nullptr, true);
           if (process_sp) {
             error = process_sp->ConnectRemote(connect_url.c_str());
             if (error.Success()) {
               ListenerSP listener_sp = attach_info.GetHijackListener();
               if (listener_sp)
                 process_sp->HijackProcessEvents(listener_sp);
+              process_sp->SetShadowListener(attach_info.GetShadowListener());
               error = process_sp->Attach(attach_info);
             }
 
             if (error.Fail() && debugserver_pid != LLDB_INVALID_PROCESS_ID) {
               KillSpawnedProcess(debugserver_pid);
             }
           }
         }

lldb/source/Target/Target.cpp

if (state == eStateConnected) {
       // Use a Process plugin to construct the process.
       const char *plugin_name = launch_info.GetProcessPluginName();
       CreateProcess(launch_info.GetListener(), plugin_name, nullptr, false);
     }
 
     // Since we didn't have a platform launch the process, launch it here.
     if (m_process_sp) {
       m_process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+      m_process_sp->SetShadowListener(launch_info.GetShadowListener());
       error = m_process_sp->Launch(launch_info);
     }
   }
 
   if (!m_process_sp && error.Success())
     error.SetErrorString("failed to launch or debug process");
 
   if (!error.Success())

lldb/source/Utility/Broadcaster.cpp

LLDB_LOGF(log,
               static_cast<void *>(hijacking_listener_sp.get()));
   }
 
   if (hijacking_listener_sp) {
     if (unique && hijacking_listener_sp->PeekAtNextEventForBroadcasterWithType(
                       &m_broadcaster, event_type))
       return;
     hijacking_listener_sp->AddEvent(event_sp);
+    if (m_shadow_listener)
+      m_shadow_listener->AddEvent(event_sp);
   } else {
     for (auto &pair : GetListeners()) {
       if (!(pair.second & event_type))
         continue;
       if (unique && pair.first->PeekAtNextEventForBroadcasterWithType(
                         &m_broadcaster, event_type))
         continue;
 
       pair.first->AddEvent(event_sp);
+      if (m_shadow_listener)
+        m_shadow_listener->AddEvent(event_sp);
     }
   }
 }
 
 void Broadcaster::BroadcasterImpl::BroadcastEvent(uint32_t event_type,
                                                   EventData *event_data) {
   auto event_sp = std::make_shared<Event>(event_type, event_data);
   PrivateBroadcastEvent(event_sp, false);

lldb/source/Utility/Listener.cpp

public:
   }
 
   BroadcasterManagerSP m_manager_sp;
 };
 } // anonymous namespace
 
 Listener::Listener(const char *name)
     : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-      m_events_mutex() {
+      m_events_mutex(), m_is_shadow() {
   Log *log = GetLog(LLDBLog::Object);
   if (log != nullptr)
     LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast<void *>(this),
               m_name.c_str());
 }
 
 Listener::~Listener() {
   Log *log = GetLog(LLDBLog::Object);
if (log != nullptr)
                 static_cast<void *>(event_sp.get()));
 
     if (remove) {
       m_events.erase(pos);
       // Unlock the event queue here.  We've removed this event and are about
       // 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();
     }
     return true;
   }
 
   event_sp.reset();
   return false;
 }
 

lldb/source/Utility/ProcessInfo.cpp

 #include <climits>
 #include <optional>
 
 using namespace lldb;
 using namespace lldb_private;
 
 ProcessInfo::ProcessInfo()
     : m_executable(), m_arguments(), m_environment(), m_arch(), m_listener_sp(),
-      m_hijack_listener_sp(), m_passthrough_listener_sp() {}
+      m_hijack_listener_sp(), m_shadow_listener_sp() {}
 
 ProcessInfo::ProcessInfo(const char *name, const ArchSpec &arch,
                          lldb::pid_t pid)
     : m_executable(name), m_arguments(), m_environment(), m_arch(arch),
       m_pid(pid), m_listener_sp(), m_hijack_listener_sp(),
-      m_passthrough_listener_sp() {}
+      m_shadow_listener_sp() {}
 
 void ProcessInfo::Clear() {
   m_executable.Clear();
   m_arguments.Clear();
   m_environment.clear();
   m_uid = UINT32_MAX;
   m_gid = UINT32_MAX;
   m_arch.Clear();