This revision was automatically updated to reflect the committed changes.
Closed by commit rGe64cc756819d: [lldb-server/linux] Use waitpid(-1) to collect 
inferior events (authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146977/new/

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,45 @@
 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);
+    }
+
+    // Collect an event for the given tid, waiting for it if necessary.
+    void CollectThread(::pid_t tid);
+
+  private:
+    MainLoop::SignalHandleUP m_sigchld_handle;
+
+    llvm::SmallPtrSet<NativeProcessLinux *, 2> m_processes;
+
+    // Threads (events) which haven't been claimed by any process.
+    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 +172,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 +185,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 +193,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,99 @@
   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;
+
+    // Ask each process whether it wants to handle the event. Each event should
+    // be handled by exactly one process, but thread creation events require
+    // special handling.
+    // Thread creation consists of two events (one on the parent and one on the
+    // child thread) and they can arrive in any order nondeterministically. The
+    // parent event carries the information about the child thread, but not
+    // vice-versa. This means that if the child event arrives first, it may not
+    // be handled by any process (because it doesn't know the thread belongs to
+    // it).
+    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 creation event 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 +442,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 +540,21 @@
   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)) {
+    // The process exited.  We're done monitoring.  Report to delegate.
+    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 +622,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 +944,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 +971,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 +1085,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 +1979,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