labath created this revision.
labath added reviewers: DavidSpickett, yinghuitan.
Herald added a subscriber: emaste.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

This is a follow-up to D116372 <https://reviews.llvm.org/D116372>, which had a 
rather unfortunate side
effect of making the processing of a single SIGCHLD quadratic in the
number of threads -- which does not matter for simple applications, but
can get really bad for applications with thousands of threads.

This patch fixes the problem by implementing the other possibility
mentioned in the first patch -- doing waitpid(-1) centrally and then
routing the events to the correct process instance. The "uncollected"
threads are held in the process factory class -- which I've renamed to
Manager for this purpose, as it now does more than creating processes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146977

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===================================================================
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -62,16 +62,16 @@
 
 namespace {
 #if defined(__linux__)
-typedef process_linux::NativeProcessLinux::Factory NativeProcessFactory;
+typedef process_linux::NativeProcessLinux::Manager NativeProcessManager;
 #elif defined(__FreeBSD__)
-typedef process_freebsd::NativeProcessFreeBSD::Factory NativeProcessFactory;
+typedef process_freebsd::NativeProcessFreeBSD::Manager NativeProcessManager;
 #elif defined(__NetBSD__)
-typedef process_netbsd::NativeProcessNetBSD::Factory NativeProcessFactory;
+typedef process_netbsd::NativeProcessNetBSD::Manager NativeProcessManager;
 #elif defined(_WIN32)
-typedef NativeProcessWindows::Factory NativeProcessFactory;
+typedef NativeProcessWindows::Manager NativeProcessManager;
 #else
 // Dummy implementation to make sure the code compiles
-class NativeProcessFactory : public NativeProcessProtocol::Factory {
+class NativeProcessManager : public NativeProcessProtocol::Manager {
 public:
   llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
   Launch(ProcessLaunchInfo &launch_info,
@@ -431,8 +431,8 @@
     return 1;
   }
 
-  NativeProcessFactory factory;
-  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, factory);
+  NativeProcessManager manager(mainloop);
+  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager);
 
   llvm::StringRef host_and_port;
   if (!Inputs.empty()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -35,7 +35,7 @@
   // Constructors and Destructors
   GDBRemoteCommunicationServerLLGS(
       MainLoop &mainloop,
-      const NativeProcessProtocol::Factory &process_factory);
+      NativeProcessProtocol::Manager &process_manager);
 
   void SetLaunchInfo(const ProcessLaunchInfo &info);
 
@@ -99,7 +99,7 @@
 protected:
   MainLoop &m_mainloop;
   MainLoop::ReadHandleUP m_network_handle_up;
-  const NativeProcessProtocol::Factory &m_process_factory;
+  NativeProcessProtocol::Manager &m_process_manager;
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
   NativeProcessProtocol *m_current_process;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -69,9 +69,9 @@
 
 // GDBRemoteCommunicationServerLLGS constructor
 GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
-    MainLoop &mainloop, const NativeProcessProtocol::Factory &process_factory)
+    MainLoop &mainloop, NativeProcessProtocol::Manager &process_manager)
     : GDBRemoteCommunicationServerCommon(), m_mainloop(mainloop),
-      m_process_factory(process_factory), m_current_process(nullptr),
+      m_process_manager(process_manager), m_current_process(nullptr),
       m_continue_process(nullptr), m_stdio_communication() {
   RegisterPacketHandlers();
 }
@@ -286,8 +286,7 @@
     std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
     assert(m_debugged_processes.empty() && "lldb-server creating debugged "
                                            "process but one already exists");
-    auto process_or =
-        m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
+    auto process_or = m_process_manager.Launch(m_process_launch_info, *this);
     if (!process_or)
       return Status(process_or.takeError());
     m_continue_process = m_current_process = process_or->get();
@@ -356,7 +355,7 @@
                   pid, m_current_process->GetID());
 
   // Try to attach.
-  auto process_or = m_process_factory.Attach(pid, *this, m_mainloop);
+  auto process_or = m_process_manager.Attach(pid, *this);
   if (!process_or) {
     Status status(process_or.takeError());
     llvm::errs() << llvm::formatv("failed to attach to process {0}: {1}\n", pid,
@@ -4209,7 +4208,7 @@
 
   // report server-only features
   using Extension = NativeProcessProtocol::Extension;
-  Extension plugin_features = m_process_factory.GetSupportedExtensions();
+  Extension plugin_features = m_process_manager.GetSupportedExtensions();
   if (bool(plugin_features & Extension::pass_signals))
     ret.push_back("QPassSignals+");
   if (bool(plugin_features & Extension::auxv))
@@ -4255,7 +4254,7 @@
 void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
     NativeProcessProtocol &process) {
   NativeProcessProtocol::Extension flags = m_extensions_supported;
-  assert(!bool(flags & ~m_process_factory.GetSupportedExtensions()));
+  assert(!bool(flags & ~m_process_manager.GetSupportedExtensions()));
   process.SetEnabledExtensions(flags);
 }
 
Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
@@ -31,15 +31,16 @@
                              public ProcessDebugger {
 
 public:
-  class Factory : public NativeProcessProtocol::Factory {
+  class Manager : public NativeProcessProtocol::Manager {
   public:
+    using NativeProcessProtocol::Manager::Manager;
+
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Launch(ProcessLaunchInfo &launch_info,
+           NativeDelegate &native_delegate) override;
 
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override;
   };
 
   Status Resume(const ResumeActionList &resume_actions) override;
Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -520,7 +520,7 @@
         SetStopReasonForThread(*thread, StopReason::eStopReasonBreakpoint);
 
       // Do not notify the native delegate (e.g. llgs) since at this moment
-      // the program hasn't returned from Factory::Launch() and the delegate
+      // the program hasn't returned from Manager::Launch() and the delegate
       // might not have an valid native process to operate on.
       SetState(eStateStopped, false);
 
@@ -603,10 +603,9 @@
 }
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessWindows::Factory::Launch(
+NativeProcessWindows::Manager::Launch(
     ProcessLaunchInfo &launch_info,
-    NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop) const {
+    NativeProcessProtocol::NativeDelegate &native_delegate) {
   Error E = Error::success();
   auto process_up = std::unique_ptr<NativeProcessWindows>(
       new NativeProcessWindows(launch_info, native_delegate, E));
@@ -616,9 +615,8 @@
 }
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessWindows::Factory::Attach(
-    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop) const {
+NativeProcessWindows::Manager::Attach(
+    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate) {
   Error E = Error::success();
   // Set pty primary fd invalid since it is not available.
   auto process_up = std::unique_ptr<NativeProcessWindows>(
Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
===================================================================
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
@@ -27,15 +27,16 @@
 /// Changes in the inferior process state are broadcasted.
 class NativeProcessNetBSD : public NativeProcessELF {
 public:
-  class Factory : public NativeProcessProtocol::Factory {
+  class Manager : public NativeProcessProtocol::Manager {
   public:
+    using NativeProcessProtocol::Manager::Manager;
+
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Launch(ProcessLaunchInfo &launch_info,
+           NativeDelegate &native_delegate) override;
 
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override;
 
     Extension GetSupportedExtensions() const override;
   };
Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===================================================================
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -56,9 +56,8 @@
 // Public Static Methods
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessNetBSD::Factory::Launch(ProcessLaunchInfo &launch_info,
-                                     NativeDelegate &native_delegate,
-                                     MainLoop &mainloop) const {
+NativeProcessNetBSD::Manager::Launch(ProcessLaunchInfo &launch_info,
+                                     NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
 
   Status status;
@@ -96,7 +95,7 @@
 
   std::unique_ptr<NativeProcessNetBSD> process_up(new NativeProcessNetBSD(
       pid, launch_info.GetPTY().ReleasePrimaryFileDescriptor(), native_delegate,
-      Info.GetArchitecture(), mainloop));
+      Info.GetArchitecture(), m_mainloop));
 
   status = process_up->SetupTrace();
   if (status.Fail())
@@ -110,9 +109,8 @@
 }
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessNetBSD::Factory::Attach(
-    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop) const {
+NativeProcessNetBSD::Manager::Attach(
+    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
   LLDB_LOG(log, "pid = {0:x}", pid);
 
@@ -124,7 +122,7 @@
   }
 
   std::unique_ptr<NativeProcessNetBSD> process_up(new NativeProcessNetBSD(
-      pid, -1, native_delegate, Info.GetArchitecture(), mainloop));
+      pid, -1, native_delegate, Info.GetArchitecture(), m_mainloop));
 
   Status status = process_up->Attach();
   if (!status.Success())
@@ -134,7 +132,7 @@
 }
 
 NativeProcessNetBSD::Extension
-NativeProcessNetBSD::Factory::GetSupportedExtensions() const {
+NativeProcessNetBSD::Manager::GetSupportedExtensions() const {
   return Extension::multiprocess | Extension::fork | Extension::vfork |
          Extension::pass_signals | Extension::auxv | Extension::libraries_svr4 |
          Extension::savecore;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -19,6 +19,7 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
+#include "llvm/ADT/SmallPtrSet.h"
 
 #include "IntelPTCollector.h"
 #include "NativeThreadLinux.h"
@@ -40,20 +41,43 @@
 class NativeProcessLinux : public NativeProcessELF,
                            private NativeProcessSoftwareSingleStep {
 public:
-  class Factory : public NativeProcessProtocol::Factory {
+  class Manager : public NativeProcessProtocol::Manager {
   public:
+    Manager(MainLoop &mainloop);
+
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Launch(ProcessLaunchInfo &launch_info,
+           NativeDelegate &native_delegate) override;
 
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override;
 
     Extension GetSupportedExtensions() const override;
+
+    void AddProcess(NativeProcessLinux &process) {
+      m_processes.insert(&process);
+    }
+
+    void RemoveProcess(NativeProcessLinux &process) {
+      m_processes.erase(&process);
+    }
+
+    void CollectThread(::pid_t tid);
+
+  private:
+    MainLoop::SignalHandleUP m_sigchld_handle;
+
+    llvm::SmallPtrSet<NativeProcessLinux *, 2> m_processes;
+
+    llvm::DenseSet<::pid_t> m_unowned_threads;
+
+    void SigchldHandler();
   };
 
   // NativeProcessProtocol Interface
+
+  ~NativeProcessLinux() override { m_manager.RemoveProcess(*this); }
+
   Status Resume(const ResumeActionList &resume_actions) override;
 
   Status Halt() override;
@@ -146,9 +170,8 @@
   llvm::Expected<uint64_t> Syscall(llvm::ArrayRef<uint64_t> args);
 
 private:
-  MainLoop::SignalHandleUP m_sigchld_handle;
+  Manager &m_manager;
   ArchSpec m_arch;
-  MainLoop& m_main_loop;
 
   LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector<std::pair<MemoryRegionInfo, FileSpec>> m_mem_region_cache;
@@ -160,7 +183,7 @@
 
   // Private Instance Methods
   NativeProcessLinux(::pid_t pid, int terminal_fd, NativeDelegate &delegate,
-                     const ArchSpec &arch, MainLoop &mainloop,
+                     const ArchSpec &arch, Manager &manager,
                      llvm::ArrayRef<::pid_t> tids);
 
   // Returns a list of process threads that we have attached to.
@@ -168,9 +191,9 @@
 
   static Status SetDefaultPtraceOpts(const lldb::pid_t);
 
-  void MonitorCallback(NativeThreadLinux &thread, WaitStatus status);
+  bool TryHandleWaitStatus(lldb::pid_t pid, WaitStatus status);
 
-  void WaitForCloneNotification(::pid_t pid);
+  void MonitorCallback(NativeThreadLinux &thread, WaitStatus status);
 
   void MonitorSIGTRAP(const siginfo_t &info, NativeThreadLinux &thread);
 
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -252,12 +252,17 @@
   }
 }
 
-// Public Static Methods
+NativeProcessLinux::Manager::Manager(MainLoop &mainloop)
+    : NativeProcessProtocol::Manager(mainloop) {
+  Status status;
+  m_sigchld_handle = mainloop.RegisterSignal(
+      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
+  assert(m_sigchld_handle && status.Success());
+}
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessLinux::Factory::Launch(ProcessLaunchInfo &launch_info,
-                                    NativeDelegate &native_delegate,
-                                    MainLoop &mainloop) const {
+NativeProcessLinux::Manager::Launch(ProcessLaunchInfo &launch_info,
+                                    NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
 
   MaybeLogLaunchInfo(launch_info);
@@ -298,13 +303,12 @@
 
   return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
       pid, launch_info.GetPTY().ReleasePrimaryFileDescriptor(), native_delegate,
-      *arch_or, mainloop, {pid}));
+      *arch_or, *this, {pid}));
 }
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessLinux::Factory::Attach(
-    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop) const {
+NativeProcessLinux::Manager::Attach(
+    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
   LLDB_LOG(log, "pid = {0:x}", pid);
 
@@ -317,12 +321,12 @@
   if (!arch_or)
     return arch_or.takeError();
 
-  return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
-      pid, -1, native_delegate, *arch_or, mainloop, tids));
+  return std::unique_ptr<NativeProcessLinux>(
+      new NativeProcessLinux(pid, -1, native_delegate, *arch_or, *this, tids));
 }
 
 NativeProcessLinux::Extension
-NativeProcessLinux::Factory::GetSupportedExtensions() const {
+NativeProcessLinux::Manager::GetSupportedExtensions() const {
   NativeProcessLinux::Extension supported =
       Extension::multiprocess | Extension::fork | Extension::vfork |
       Extension::pass_signals | Extension::auxv | Extension::libraries_svr4 |
@@ -337,24 +341,90 @@
   return supported;
 }
 
+static std::optional<std::pair<lldb::pid_t, WaitStatus>> WaitPid() {
+  Log *log = GetLog(POSIXLog::Process);
+
+  int status;
+  ::pid_t wait_pid = llvm::sys::RetryAfterSignal(
+      -1, ::waitpid, -1, &status, __WALL | __WNOTHREAD | WNOHANG);
+
+  if (wait_pid == 0)
+    return std::nullopt;
+
+  if (wait_pid == -1) {
+    Status error(errno, eErrorTypePOSIX);
+    LLDB_LOG(log, "waitpid(-1, &status, _) failed: {1}", error);
+    return std::nullopt;
+  }
+
+  WaitStatus wait_status = WaitStatus::Decode(status);
+
+  LLDB_LOG(log, "waitpid(-1, &status, _) = {0}, status = {1}", wait_pid,
+           wait_status);
+  return std::make_pair(wait_pid, wait_status);
+}
+
+void NativeProcessLinux::Manager::SigchldHandler() {
+  Log *log = GetLog(POSIXLog::Process);
+  while (true) {
+    auto wait_result = WaitPid();
+    if (!wait_result)
+      return;
+    lldb::pid_t pid = wait_result->first;
+    WaitStatus status = wait_result->second;
+
+    bool handled = llvm::any_of(m_processes, [&](NativeProcessLinux *process) {
+      return process->TryHandleWaitStatus(pid, status);
+    });
+    if (!handled) {
+      if (status.type == WaitStatus::Stop && status.status == SIGSTOP) {
+        // Store the thread for later collection.
+        m_unowned_threads.insert(pid);
+      } else {
+        LLDB_LOG(log, "Ignoring waitpid event {0} for pid {1}", status, pid);
+      }
+    }
+  }
+}
+
+void NativeProcessLinux::Manager::CollectThread(::pid_t tid) {
+  Log *log = GetLog(POSIXLog::Process);
+
+  if (m_unowned_threads.erase(tid))
+    return; // We've encountered this thread already.
+
+  // The TID is not tracked yet, let's wait for it to appear.
+  int status = -1;
+  LLDB_LOG(log,
+           "received clone event for tid {0}. tid not tracked yet, "
+           "waiting for it to appear...",
+           tid);
+  ::pid_t wait_pid =
+      llvm::sys::RetryAfterSignal(-1, ::waitpid, tid, &status, __WALL);
+
+  // It's theoretically possible to get other events if the entire process was
+  // SIGKILLed before we got a chance to check this. In that case, we'll just
+  // clean everything up when we get the process exit event.
+
+  LLDB_LOG(log,
+           "waitpid({0}, &status, __WALL) => {1} (errno: {2}, status = {3})",
+           tid, wait_pid, errno, WaitStatus::Decode(status));
+}
+
 // Public Instance Methods
 
 NativeProcessLinux::NativeProcessLinux(::pid_t pid, int terminal_fd,
                                        NativeDelegate &delegate,
-                                       const ArchSpec &arch, MainLoop &mainloop,
+                                       const ArchSpec &arch, Manager &manager,
                                        llvm::ArrayRef<::pid_t> tids)
-    : NativeProcessELF(pid, terminal_fd, delegate), m_arch(arch),
-      m_main_loop(mainloop), m_intel_pt_collector(*this) {
+    : NativeProcessELF(pid, terminal_fd, delegate), m_manager(manager),
+      m_arch(arch), m_intel_pt_collector(*this) {
+  manager.AddProcess(*this);
   if (m_terminal_fd != -1) {
     Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
     assert(status.Success());
   }
 
-  Status status;
-  m_sigchld_handle = mainloop.RegisterSignal(
-      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
-  assert(m_sigchld_handle && status.Success());
-
   for (const auto &tid : tids) {
     NativeThreadLinux &thread = AddThread(tid, /*resume*/ false);
     ThreadWasCreated(thread);
@@ -363,9 +433,6 @@
   // Let our process instance know the thread has stopped.
   SetCurrentThreadID(tids[0]);
   SetState(StateType::eStateStopped, false);
-
-  // Proccess any signals we received before installing our handler
-  SigchldHandler();
 }
 
 llvm::Expected<std::vector<::pid_t>> NativeProcessLinux::Attach(::pid_t pid) {
@@ -464,7 +531,20 @@
   return PtraceWrapper(PTRACE_SETOPTIONS, pid, nullptr, (void *)ptrace_opts);
 }
 
-// Handles all waitpid events from the inferior process.
+bool NativeProcessLinux::TryHandleWaitStatus(lldb::pid_t pid,
+                                             WaitStatus status) {
+  if (pid == GetID() &&
+      (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal)) {
+    SetExitStatus(status, true);
+    return true;
+  }
+  if (NativeThreadLinux *thread = GetThreadByID(pid)) {
+    MonitorCallback(*thread, status);
+    return true;
+  }
+  return false;
+}
+
 void NativeProcessLinux::MonitorCallback(NativeThreadLinux &thread,
                                          WaitStatus status) {
   Log *log = GetLog(LLDBLog::Process);
@@ -532,27 +612,6 @@
   }
 }
 
-void NativeProcessLinux::WaitForCloneNotification(::pid_t pid) {
-  Log *log = GetLog(POSIXLog::Process);
-
-  // The PID is not tracked yet, let's wait for it to appear.
-  int status = -1;
-  LLDB_LOG(log,
-           "received clone event for pid {0}. pid not tracked yet, "
-           "waiting for it to appear...",
-           pid);
-  ::pid_t wait_pid =
-      llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &status, __WALL);
-
-  // It's theoretically possible to get other events if the entire process was
-  // SIGKILLed before we got a chance to check this. In that case, we'll just
-  // clean everything up when we get the process exit event.
-
-  LLDB_LOG(log,
-           "waitpid({0}, &status, __WALL) => {1} (errno: {2}, status = {3})",
-           pid, wait_pid, errno, WaitStatus::Decode(status));
-}
-
 void NativeProcessLinux::MonitorSIGTRAP(const siginfo_t &info,
                                         NativeThreadLinux &thread) {
   Log *log = GetLog(POSIXLog::Process);
@@ -875,7 +934,7 @@
   LLDB_LOG(log, "parent_tid={0}, child_pid={1}, event={2}", parent.GetID(),
            child_pid, event);
 
-  WaitForCloneNotification(child_pid);
+  m_manager.CollectThread(child_pid);
 
   switch (event) {
   case PTRACE_EVENT_CLONE: {
@@ -902,7 +961,7 @@
     bool is_vfork = event == PTRACE_EVENT_VFORK;
     std::unique_ptr<NativeProcessLinux> child_process{new NativeProcessLinux(
         static_cast<::pid_t>(child_pid), m_terminal_fd, m_delegate, m_arch,
-        m_main_loop, {static_cast<::pid_t>(child_pid)})};
+        m_manager, {static_cast<::pid_t>(child_pid)})};
     if (!is_vfork)
       child_process->m_software_breakpoints = m_software_breakpoints;
 
@@ -1016,9 +1075,6 @@
 Status NativeProcessLinux::Detach() {
   Status error;
 
-  // Stop monitoring the inferior.
-  m_sigchld_handle.reset();
-
   // Tell ptrace to detach from the process.
   if (GetID() == LLDB_INVALID_PROCESS_ID)
     return error;
@@ -1913,67 +1969,6 @@
   }
 }
 
-static std::optional<WaitStatus> HandlePid(::pid_t pid) {
-  Log *log = GetLog(POSIXLog::Process);
-
-  int status;
-  ::pid_t wait_pid = llvm::sys::RetryAfterSignal(
-      -1, ::waitpid, pid, &status, __WALL | __WNOTHREAD | WNOHANG);
-
-  if (wait_pid == 0)
-    return std::nullopt;
-
-  if (wait_pid == -1) {
-    Status error(errno, eErrorTypePOSIX);
-    LLDB_LOG(log, "waitpid({0}, &status, _) failed: {1}", pid,
-             error);
-    return std::nullopt;
-  }
-
-  assert(wait_pid == pid);
-
-  WaitStatus wait_status = WaitStatus::Decode(status);
-
-  LLDB_LOG(log, "waitpid({0})  got status = {1}", pid, wait_status);
-  return wait_status;
-}
-
-void NativeProcessLinux::SigchldHandler() {
-  Log *log = GetLog(POSIXLog::Process);
-
-  // Threads can appear or disappear as a result of event processing, so gather
-  // the events upfront.
-  llvm::DenseMap<lldb::tid_t, WaitStatus> tid_events;
-  bool checked_main_thread = false;
-  for (const auto &thread_up : m_threads) {
-    if (thread_up->GetID() == GetID())
-      checked_main_thread = true;
-
-    if (std::optional<WaitStatus> status = HandlePid(thread_up->GetID()))
-      tid_events.try_emplace(thread_up->GetID(), *status);
-  }
-  // Check the main thread even when we're not tracking it as process exit
-  // events are reported that way.
-  if (!checked_main_thread) {
-    if (std::optional<WaitStatus> status = HandlePid(GetID()))
-      tid_events.try_emplace(GetID(), *status);
-  }
-
-  for (auto &KV : tid_events) {
-    LLDB_LOG(log, "processing {0}({1}) ...", KV.first, KV.second);
-    if (KV.first == GetID() && (KV.second.type == WaitStatus::Exit ||
-                                KV.second.type == WaitStatus::Signal)) {
-
-      // The process exited.  We're done monitoring.  Report to delegate.
-      SetExitStatus(KV.second, true);
-      return;
-    }
-    NativeThreadLinux *thread = GetThreadByID(KV.first);
-    assert(thread && "Why did this thread disappear?");
-    MonitorCallback(*thread, KV.second);
-  }
-}
-
 // Wrapper for ptrace to catch errors and log calls. Note that ptrace sets
 // errno on error because -1 can be a valid result (i.e. for PTRACE_PEEK*)
 Status NativeProcessLinux::PtraceWrapper(int req, lldb::pid_t pid, void *addr,
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
===================================================================
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -30,15 +30,16 @@
 class NativeProcessFreeBSD : public NativeProcessELF,
                              private NativeProcessSoftwareSingleStep {
 public:
-  class Factory : public NativeProcessProtocol::Factory {
+  class Manager : public NativeProcessProtocol::Manager {
   public:
+    using NativeProcessProtocol::Manager::Manager;
+
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Launch(ProcessLaunchInfo &launch_info,
+           NativeDelegate &native_delegate) override;
 
     llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const override;
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override;
 
     Extension GetSupportedExtensions() const override;
   };
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===================================================================
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -51,9 +51,8 @@
 // Public Static Methods
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessFreeBSD::Factory::Launch(ProcessLaunchInfo &launch_info,
-                                      NativeDelegate &native_delegate,
-                                      MainLoop &mainloop) const {
+NativeProcessFreeBSD::Manager::Launch(ProcessLaunchInfo &launch_info,
+                                      NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
 
   Status status;
@@ -91,7 +90,7 @@
 
   std::unique_ptr<NativeProcessFreeBSD> process_up(new NativeProcessFreeBSD(
       pid, launch_info.GetPTY().ReleasePrimaryFileDescriptor(), native_delegate,
-      Info.GetArchitecture(), mainloop));
+      Info.GetArchitecture(), m_mainloop));
 
   status = process_up->SetupTrace();
   if (status.Fail())
@@ -105,9 +104,8 @@
 }
 
 llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-NativeProcessFreeBSD::Factory::Attach(
-    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop) const {
+NativeProcessFreeBSD::Manager::Attach(
+    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
   LLDB_LOG(log, "pid = {0:x}", pid);
 
@@ -119,7 +117,7 @@
   }
 
   std::unique_ptr<NativeProcessFreeBSD> process_up(new NativeProcessFreeBSD(
-      pid, -1, native_delegate, Info.GetArchitecture(), mainloop));
+      pid, -1, native_delegate, Info.GetArchitecture(), m_mainloop));
 
   Status status = process_up->Attach();
   if (!status.Success())
@@ -129,7 +127,7 @@
 }
 
 NativeProcessFreeBSD::Extension
-NativeProcessFreeBSD::Factory::GetSupportedExtensions() const {
+NativeProcessFreeBSD::Manager::GetSupportedExtensions() const {
   return
 #if defined(PT_COREDUMP)
       Extension::savecore |
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===================================================================
--- lldb/source/Host/common/NativeProcessProtocol.cpp
+++ lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -759,4 +759,4 @@
   // Default implementation does nothing.
 }
 
-NativeProcessProtocol::Factory::~Factory() = default;
+NativeProcessProtocol::Manager::~Manager() = default;
Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h
===================================================================
--- lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -256,7 +256,7 @@
   virtual Status GetFileLoadAddress(const llvm::StringRef &file_name,
                                     lldb::addr_t &load_addr) = 0;
 
-  /// Extension flag constants, returned by Factory::GetSupportedExtensions()
+  /// Extension flag constants, returned by Manager::GetSupportedExtensions()
   /// and passed to SetEnabledExtension()
   enum class Extension {
     multiprocess = (1u << 0),
@@ -272,9 +272,14 @@
     LLVM_MARK_AS_BITMASK_ENUM(siginfo_read)
   };
 
-  class Factory {
+  class Manager {
   public:
-    virtual ~Factory();
+    Manager(MainLoop &mainloop) : m_mainloop(mainloop) {}
+    Manager(const Manager &) = delete;
+    Manager &operator=(const Manager &) = delete;
+
+    virtual ~Manager();
+
     /// Launch a process for debugging.
     ///
     /// \param[in] launch_info
@@ -294,8 +299,8 @@
     ///     A NativeProcessProtocol shared pointer if the operation succeeded or
     ///     an error object if it failed.
     virtual llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const = 0;
+    Launch(ProcessLaunchInfo &launch_info,
+           NativeDelegate &native_delegate) = 0;
 
     /// Attach to an existing process.
     ///
@@ -316,14 +321,16 @@
     ///     A NativeProcessProtocol shared pointer if the operation succeeded or
     ///     an error object if it failed.
     virtual llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
-    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-           MainLoop &mainloop) const = 0;
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate) = 0;
 
     /// Get the bitmask of extensions supported by this process plugin.
     ///
     /// \return
     ///     A NativeProcessProtocol::Extension bitmask.
     virtual Extension GetSupportedExtensions() const { return {}; }
+
+  protected:
+    MainLoop &m_mainloop;
   };
 
   /// Notify tracers that the target process will resume
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits]... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... jeffrey tan via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... jeffrey tan via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Nico Weber via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits

Reply via email to