labath created this revision.
labath added a reviewer: JDevlieghere.
Herald added a subscriber: krytarowski.
labath requested review of this revision.
Herald added a project: LLDB.

All current callers set the argument to false. monitor_signals=true used
to be used in the Process plugins (which needed to know when the
debugged process gets a signal), but this implementation has several
serious issues, which means that individual process plugins now
orchestrate the monitoring of debugged processes themselves.

This allows us to simplify the implementation (no need to play with
process groups), and the interface (we only catch fatal events, so the
callback is always called just once).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120425

Files:
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Host/HostNativeProcessBase.h
  lldb/include/lldb/Host/HostProcess.h
  lldb/include/lldb/Host/ProcessLaunchInfo.h
  lldb/include/lldb/Host/posix/HostProcessPosix.h
  lldb/include/lldb/Host/windows/HostProcessWindows.h
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/common/HostProcess.cpp
  lldb/source/Host/common/MonitoringProcessLauncher.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Host/posix/HostProcessPosix.cpp
  lldb/source/Host/windows/HostProcessWindows.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -102,8 +102,7 @@
   // TODO: Use this callback to detect botched launches. If lldb-server does not
   // start, we can print a nice error message here instead of hanging in
   // Accept().
-  Info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback,
-                                 false);
+  Info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback);
 
   status = Host::LaunchProcess(Info);
   if (status.Fail())
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -344,10 +344,9 @@
 
   lldb::thread_result_t AsyncThread();
 
-  static bool
+  static void
   MonitorDebugserverProcess(std::weak_ptr<ProcessGDBRemote> process_wp,
-                            lldb::pid_t pid, bool exited, int signo,
-                            int exit_status);
+                            lldb::pid_t pid, int signo, int exit_status);
 
   lldb::StateType SetThreadStopInfo(StringExtractor &stop_packet);
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3366,7 +3366,7 @@
     const std::weak_ptr<ProcessGDBRemote> this_wp =
         std::static_pointer_cast<ProcessGDBRemote>(shared_from_this());
     debugserver_launch_info.SetMonitorProcessCallback(
-        std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
+        std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3));
     debugserver_launch_info.SetUserID(process_info.GetUserID());
 
 #if defined(__APPLE__)
@@ -3445,16 +3445,14 @@
   return error;
 }
 
-bool ProcessGDBRemote::MonitorDebugserverProcess(
+void ProcessGDBRemote::MonitorDebugserverProcess(
     std::weak_ptr<ProcessGDBRemote> process_wp, lldb::pid_t debugserver_pid,
-    bool exited,    // True if the process did exit
     int signo,      // Zero for no signal
     int exit_status // Exit value of process if signal is zero
 ) {
   // "debugserver_pid" argument passed in is the process ID for debugserver
   // that we are tracking...
   Log *log = GetLog(GDBRLog::Process);
-  const bool handled = true;
 
   LLDB_LOGF(log,
             "ProcessGDBRemote::%s(process_wp, pid=%" PRIu64
@@ -3465,7 +3463,7 @@
   LLDB_LOGF(log, "ProcessGDBRemote::%s(process = %p)", __FUNCTION__,
             static_cast<void *>(process_sp.get()));
   if (!process_sp || process_sp->m_debugserver_pid != debugserver_pid)
-    return handled;
+    return;
 
   // Sleep for a half a second to make sure our inferior process has time to
   // set its exit status before we set it incorrectly when both the debugserver
@@ -3499,7 +3497,6 @@
   // Debugserver has exited we need to let our ProcessGDBRemote know that it no
   // longer has a debugserver instance
   process_sp->m_debugserver_pid = LLDB_INVALID_PROCESS_ID;
-  return handled;
 }
 
 void ProcessGDBRemote::KillDebugserverProcess() {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -131,7 +131,7 @@
 private:
   bool KillSpawnedProcess(lldb::pid_t pid);
 
-  bool DebugserverProcessReaped(lldb::pid_t pid);
+  void DebugserverProcessReaped(lldb::pid_t pid);
 
   static const FileSpec &GetDomainSocketDir();
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -188,8 +188,7 @@
   debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
   debugserver_launch_info.SetMonitorProcessCallback(
       std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-                this, std::placeholders::_1),
-      false);
+                this, std::placeholders::_1));
 
   std::ostringstream url;
 // debugserver does not accept the URL scheme prefix.
@@ -517,12 +516,11 @@
   return SendPacketNoLock(response.GetString());
 }
 
-bool GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped(
+void GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped(
     lldb::pid_t pid) {
   std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
   m_port_map.FreePortForProcess(pid);
   m_spawned_pids.erase(pid);
-  return true;
 }
 
 Status GDBRemoteCommunicationServerPlatform::LaunchProcess() {
@@ -533,11 +531,9 @@
   // specify the process monitor if not already set.  This should generally be
   // what happens since we need to reap started processes.
   if (!m_process_launch_info.GetMonitorProcessCallback())
-    m_process_launch_info.SetMonitorProcessCallback(
-        std::bind(
-            &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-            this, std::placeholders::_1),
-        false);
+    m_process_launch_info.SetMonitorProcessCallback(std::bind(
+        &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped, this,
+        std::placeholders::_1));
 
   Status error = Host::LaunchProcess(m_process_launch_info);
   if (!error.Success()) {
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===================================================================
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -201,8 +201,7 @@
 
   launch_info.SetLaunchInSeparateProcessGroup(true);
   launch_info.GetFlags().Clear(eLaunchFlagDebug);
-  launch_info.SetMonitorProcessCallback(ProcessLaunchInfo::NoOpMonitorCallback,
-                                        false);
+  launch_info.SetMonitorProcessCallback(ProcessLaunchInfo::NoOpMonitorCallback);
 
   // This is automatically done for host platform in
   // Target::FinalizeFileActions, but we're not a host platform.
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -743,9 +743,8 @@
     // charge of setting the exit status.  However, we still need to reap it
     // from lldb. So, make sure we use a exit callback which does not set exit
     // status.
-    const bool monitor_signals = false;
     launch_info.SetMonitorProcessCallback(
-        &ProcessLaunchInfo::NoOpMonitorCallback, monitor_signals);
+        &ProcessLaunchInfo::NoOpMonitorCallback);
     process_sp = Platform::DebugProcess(launch_info, debugger, target, error);
   } else {
     if (m_remote_platform_sp)
Index: lldb/source/Host/windows/HostProcessWindows.cpp
===================================================================
--- lldb/source/Host/windows/HostProcessWindows.cpp
+++ lldb/source/Host/windows/HostProcessWindows.cpp
@@ -76,7 +76,7 @@
 }
 
 llvm::Expected<HostThread> HostProcessWindows::StartMonitoring(
-    const Host::MonitorChildProcessCallback &callback, bool monitor_signals) {
+    const Host::MonitorChildProcessCallback &callback) {
   HANDLE process_handle;
 
   // Since the life of this HostProcessWindows instance and the life of the
Index: lldb/source/Host/posix/HostProcessPosix.cpp
===================================================================
--- lldb/source/Host/posix/HostProcessPosix.cpp
+++ lldb/source/Host/posix/HostProcessPosix.cpp
@@ -61,7 +61,6 @@
 }
 
 llvm::Expected<HostThread> HostProcessPosix::StartMonitoring(
-    const Host::MonitorChildProcessCallback &callback, bool monitor_signals) {
-  return Host::StartMonitoringChildProcess(callback, m_process,
-                                           monitor_signals);
+    const Host::MonitorChildProcessCallback &callback) {
+  return Host::StartMonitoringChildProcess(callback, m_process);
 }
Index: lldb/source/Host/macosx/objcxx/Host.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -1427,11 +1427,8 @@
 }
 
 llvm::Expected<HostThread> Host::StartMonitoringChildProcess(
-    const Host::MonitorChildProcessCallback &callback, lldb::pid_t pid,
-    bool monitor_signals) {
+    const Host::MonitorChildProcessCallback &callback, lldb::pid_t pid) {
   unsigned long mask = DISPATCH_PROC_EXIT;
-  if (monitor_signals)
-    mask |= DISPATCH_PROC_SIGNAL;
 
   Log *log(GetLog(LLDBLog::Host | LLDBLog::Process));
 
@@ -1440,11 +1437,8 @@
       ::dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0));
 
   LLDB_LOGF(log,
-            "Host::StartMonitoringChildProcess "
-            "(callback, pid=%i, monitor_signals=%i) "
-            "source = %p\n",
-            static_cast<int>(pid), monitor_signals,
-            static_cast<void *>(source));
+            "Host::StartMonitoringChildProcess(callback, pid=%i) source = %p\n",
+            static_cast<int>(pid), static_cast<void *>(source));
 
   if (source) {
     Host::MonitorChildProcessCallback callback_copy = callback;
@@ -1455,27 +1449,20 @@
 
       int status = 0;
       int wait_pid = 0;
-      bool cancel = false;
-      bool exited = false;
       wait_pid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &status, 0);
       if (wait_pid >= 0) {
         int signal = 0;
         int exit_status = 0;
         const char *status_cstr = NULL;
-        if (WIFSTOPPED(status)) {
-          signal = WSTOPSIG(status);
-          status_cstr = "STOPPED";
-        } else if (WIFEXITED(status)) {
+        if (WIFEXITED(status)) {
           exit_status = WEXITSTATUS(status);
           status_cstr = "EXITED";
-          exited = true;
         } else if (WIFSIGNALED(status)) {
           signal = WTERMSIG(status);
           status_cstr = "SIGNALED";
-          exited = true;
           exit_status = -1;
         } else {
-          status_cstr = "???";
+          llvm_unreachable("Unknown status");
         }
 
         LLDB_LOGF(log,
@@ -1484,10 +1471,9 @@
                   pid, wait_pid, status, status_cstr, signal, exit_status);
 
         if (callback_copy)
-          cancel = callback_copy(pid, exited, signal, exit_status);
+          callback_copy(pid, signal, exit_status);
 
-        if (exited || cancel) {
-          ::dispatch_source_cancel(source);
+        ::dispatch_source_cancel(source);
         }
       }
     });
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===================================================================
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -42,8 +42,6 @@
                                      uint32_t launch_flags)
     : ProcessInfo(), m_working_dir(), m_plugin_name(), m_flags(launch_flags),
       m_file_actions(), m_pty(new PseudoTerminal), m_resume_count(0),
-      m_monitor_callback(nullptr), m_monitor_callback_baton(nullptr),
-      m_monitor_signals(false), m_listener_sp(), m_hijack_listener_sp(),
       m_scripted_process_class_name(), m_scripted_process_dictionary_sp() {
   if (stdin_file_spec) {
     FileAction file_action;
@@ -177,24 +175,16 @@
   m_scripted_process_dictionary_sp.reset();
 }
 
-void ProcessLaunchInfo::SetMonitorProcessCallback(
-    const Host::MonitorChildProcessCallback &callback, bool monitor_signals) {
-  m_monitor_callback = callback;
-  m_monitor_signals = monitor_signals;
-}
-
-bool ProcessLaunchInfo::NoOpMonitorCallback(lldb::pid_t pid, bool exited, int signal, int status) {
+void ProcessLaunchInfo::NoOpMonitorCallback(lldb::pid_t pid, int signal,
+                                            int status) {
   Log *log = GetLog(LLDBLog::Process);
-  LLDB_LOG(log, "pid = {0}, exited = {1}, signal = {2}, status = {3}", pid,
-           exited, signal, status);
-  return true;
+  LLDB_LOG(log, "pid = {0}, signal = {1}, status = {2}", pid, signal, status);
 }
 
 bool ProcessLaunchInfo::MonitorProcess() const {
   if (m_monitor_callback && ProcessIDIsValid()) {
     llvm::Expected<HostThread> maybe_thread =
-    Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID(),
-                                      m_monitor_signals);
+        Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
     if (!maybe_thread)
       LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
                llvm::toString(maybe_thread.takeError()));
Index: lldb/source/Host/common/MonitoringProcessLauncher.cpp
===================================================================
--- lldb/source/Host/common/MonitoringProcessLauncher.cpp
+++ lldb/source/Host/common/MonitoringProcessLauncher.cpp
@@ -55,8 +55,7 @@
 
     assert(launch_info.GetMonitorProcessCallback());
     llvm::Expected<HostThread> maybe_thread =
-        process.StartMonitoring(launch_info.GetMonitorProcessCallback(),
-                                launch_info.GetMonitorSignals());
+        process.StartMonitoring(launch_info.GetMonitorProcessCallback());
     if (!maybe_thread)
       error.SetErrorStringWithFormatv("failed to launch host thread: {}",
                                       llvm::toString(maybe_thread.takeError()));
Index: lldb/source/Host/common/HostProcess.cpp
===================================================================
--- lldb/source/Host/common/HostProcess.cpp
+++ lldb/source/Host/common/HostProcess.cpp
@@ -28,10 +28,9 @@
 
 bool HostProcess::IsRunning() const { return m_native_process->IsRunning(); }
 
-llvm::Expected<HostThread>
-HostProcess::StartMonitoring(const Host::MonitorChildProcessCallback &callback,
-                             bool monitor_signals) {
-  return m_native_process->StartMonitoring(callback, monitor_signals);
+llvm::Expected<HostThread> HostProcess::StartMonitoring(
+    const Host::MonitorChildProcessCallback &callback) {
+  return m_native_process->StartMonitoring(callback);
 }
 
 HostNativeProcessBase &HostProcess::GetNativeProcess() {
Index: lldb/source/Host/common/Host.cpp
===================================================================
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -94,19 +94,16 @@
 #if !defined(__APPLE__) && !defined(_WIN32)
 static thread_result_t
 MonitorChildProcessThreadFunction(::pid_t pid,
-                                  Host::MonitorChildProcessCallback callback,
-                                  bool monitor_signals);
+                                  Host::MonitorChildProcessCallback callback);
 
 llvm::Expected<HostThread> Host::StartMonitoringChildProcess(
-    const Host::MonitorChildProcessCallback &callback, lldb::pid_t pid,
-    bool monitor_signals) {
+    const Host::MonitorChildProcessCallback &callback, lldb::pid_t pid) {
   char thread_name[256];
   ::snprintf(thread_name, sizeof(thread_name),
              "<lldb.host.wait4(pid=%" PRIu64 ")>", pid);
   assert(pid <= UINT32_MAX);
-  return ThreadLauncher::LaunchThread(thread_name, [pid, callback,
-                                                    monitor_signals] {
-    return MonitorChildProcessThreadFunction(pid, callback, monitor_signals);
+  return ThreadLauncher::LaunchThread(thread_name, [pid, callback] {
+    return MonitorChildProcessThreadFunction(pid, callback);
   });
 }
 
@@ -158,18 +155,11 @@
 
 static thread_result_t
 MonitorChildProcessThreadFunction(::pid_t pid,
-                                  Host::MonitorChildProcessCallback callback,
-                                  bool monitor_signals) {
+                                  Host::MonitorChildProcessCallback callback) {
   Log *log = GetLog(LLDBLog::Process);
-  LLDB_LOG(log, "pid = {0}, monitor_signals = {1}", pid, monitor_signals);
-
-  pid = monitor_signals ? -1 * getpgid(pid) : pid;
+  LLDB_LOG(log, "pid = {0}", pid);
 
   int status = -1;
-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__OpenBSD__)
-#define __WALL 0
-#endif
-  const int options = __WALL;
 
 #ifdef __linux__
   // This signal is only used to interrupt the thread from waitpid
@@ -179,88 +169,49 @@
   ::sigaction(SIGUSR1, &sigUsr1Action, nullptr);
 #endif // __linux__
 
-  while (true) {
+  while(1) {
     log = GetLog(LLDBLog::Process);
-    LLDB_LOG(log, "::waitpid({0}, &status, {1})...", pid, options);
+    LLDB_LOG(log, "::waitpid({0}, &status, 0)...", pid);
 
     if (CheckForMonitorCancellation())
-      break;
+      return nullptr;
 
-    // Get signals from all children with same process group of pid
-    const ::pid_t wait_pid = ::waitpid(pid, &status, options);
+    const ::pid_t wait_pid = ::waitpid(pid, &status, 0);
+
+    LLDB_LOG(log, "::waitpid({0}, &status, 0) => pid = {1}, status = {2:x}", pid,
+             wait_pid, status);
 
     if (CheckForMonitorCancellation())
+      return nullptr;
+
+    if (wait_pid != -1)
       break;
+    if (errno != EINTR) {
+      LLDB_LOG(log, "pid = {0}, thread exiting because waitpid failed ({1})...",
+               pid, llvm::sys::StrError());
+      return nullptr;
+    }
+  }
 
-    if (wait_pid == -1) {
-      if (errno == EINTR)
-        continue;
-      else {
-        LLDB_LOG(log,
-                 "pid = {0}, thread exiting because waitpid failed ({1})...",
-                 pid, llvm::sys::StrError());
-        break;
-      }
-    } else if (wait_pid > 0) {
-      bool exited = false;
-      int signal = 0;
-      int exit_status = 0;
-      const char *status_cstr = nullptr;
-      if (WIFSTOPPED(status)) {
-        signal = WSTOPSIG(status);
-        status_cstr = "STOPPED";
-      } else if (WIFEXITED(status)) {
-        exit_status = WEXITSTATUS(status);
-        status_cstr = "EXITED";
-        exited = true;
-      } else if (WIFSIGNALED(status)) {
-        signal = WTERMSIG(status);
-        status_cstr = "SIGNALED";
-        if (wait_pid == abs(pid)) {
-          exited = true;
-          exit_status = -1;
-        }
-      } else {
-        status_cstr = "(\?\?\?)";
-      }
+  int signal = 0;
+  int exit_status = 0;
+  if (WIFEXITED(status)) {
+    exit_status = WEXITSTATUS(status);
+  } else if (WIFSIGNALED(status)) {
+    signal = WTERMSIG(status);
+    exit_status = -1;
+  } else {
+    llvm_unreachable("Unknown status");
+  }
 
-      // Scope for pthread_cancel_disabler
-      {
+  // Scope for pthread_cancel_disabler
+  {
 #ifndef __linux__
-        ScopedPThreadCancelDisabler pthread_cancel_disabler;
+    ScopedPThreadCancelDisabler pthread_cancel_disabler;
 #endif
 
-        log = GetLog(LLDBLog::Process);
-        LLDB_LOG(log,
-                 "::waitpid({0}, &status, {1}) => pid = {2}, status = {3:x} "
-                 "({4}), signal = {5}, exit_state = {6}",
-                 pid, options, wait_pid, status, status_cstr, signal,
-                 exit_status);
-
-        if (exited || (signal != 0 && monitor_signals)) {
-          bool callback_return = false;
-          if (callback)
-            callback_return = callback(wait_pid, exited, signal, exit_status);
-
-          // If our process exited, then this thread should exit
-          if (exited && wait_pid == abs(pid)) {
-            LLDB_LOG(
-                log,
-                "pid = {0} thread exiting because pid received exit signal...",
-                pid);
-            break;
-          }
-          // If the callback returns true, it means this process should exit
-          if (callback_return) {
-            LLDB_LOG(
-                log,
-                "pid = {0} thread exiting because callback returned true...",
-                pid);
-            break;
-          }
-        }
-      }
-    }
+    if (callback)
+      callback(pid, signal, exit_status);
   }
 
   LLDB_LOG(GetLog(LLDBLog::Process), "pid = {0} thread exiting...", pid);
@@ -434,11 +385,10 @@
   int status = -1;
 };
 
-static bool
+static void
 MonitorShellCommand(std::shared_ptr<ShellInfo> shell_info, lldb::pid_t pid,
-                    bool exited, // True if the process did exit
-                    int signo,   // Zero for no signal
-                    int status)  // Exit value of process if signal is zero
+                    int signo,  // Zero for no signal
+                    int status) // Exit value of process if signal is zero
 {
   shell_info->pid = pid;
   shell_info->signo = signo;
@@ -446,7 +396,6 @@
   // Let the thread running Host::RunShellCommand() know that the process
   // exited and that ShellInfo has been filled in by broadcasting to it
   shell_info->process_reaped.SetValue(true, eBroadcastAlways);
-  return true;
 }
 
 Status Host::RunShellCommand(llvm::StringRef command,
@@ -541,12 +490,9 @@
     launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true);
 
   std::shared_ptr<ShellInfo> shell_info_sp(new ShellInfo());
-  const bool monitor_signals = false;
   launch_info.SetMonitorProcessCallback(
       std::bind(MonitorShellCommand, shell_info_sp, std::placeholders::_1,
-                std::placeholders::_2, std::placeholders::_3,
-                std::placeholders::_4),
-      monitor_signals);
+                std::placeholders::_2, std::placeholders::_3));
 
   error = LaunchProcess(launch_info);
   const lldb::pid_t pid = launch_info.GetProcessID();
Index: lldb/include/lldb/Host/windows/HostProcessWindows.h
===================================================================
--- lldb/include/lldb/Host/windows/HostProcessWindows.h
+++ lldb/include/lldb/Host/windows/HostProcessWindows.h
@@ -30,8 +30,7 @@
   bool IsRunning() const override;
 
   virtual llvm::Expected<HostThread>
-  StartMonitoring(const Host::MonitorChildProcessCallback &callback,
-                  bool monitor_signals) override;
+  StartMonitoring(const Host::MonitorChildProcessCallback &callback) override;
 
 private:
   void Close();
Index: lldb/include/lldb/Host/posix/HostProcessPosix.h
===================================================================
--- lldb/include/lldb/Host/posix/HostProcessPosix.h
+++ lldb/include/lldb/Host/posix/HostProcessPosix.h
@@ -32,8 +32,7 @@
   bool IsRunning() const override;
 
   llvm::Expected<HostThread>
-  StartMonitoring(const Host::MonitorChildProcessCallback &callback,
-                  bool monitor_signals) override;
+  StartMonitoring(const Host::MonitorChildProcessCallback &callback) override;
 };
 
 } // namespace lldb_private
Index: lldb/include/lldb/Host/ProcessLaunchInfo.h
===================================================================
--- lldb/include/lldb/Host/ProcessLaunchInfo.h
+++ lldb/include/lldb/Host/ProcessLaunchInfo.h
@@ -99,21 +99,18 @@
                                            bool first_arg_is_full_shell_command,
                                            uint32_t num_resumes);
 
-  void
-  SetMonitorProcessCallback(const Host::MonitorChildProcessCallback &callback,
-                            bool monitor_signals);
+  void SetMonitorProcessCallback(Host::MonitorChildProcessCallback callback) {
+    m_monitor_callback = std::move(callback);
+  }
 
-  Host::MonitorChildProcessCallback GetMonitorProcessCallback() const {
+  const Host::MonitorChildProcessCallback &GetMonitorProcessCallback() const {
     return m_monitor_callback;
   }
 
   /// A Monitor callback which does not take any action on process events. Use
   /// this if you don't need to take any particular action when the process
   /// terminates, but you still need to reap it.
-  static bool NoOpMonitorCallback(lldb::pid_t pid, bool exited, int signal,
-                                  int status);
-
-  bool GetMonitorSignals() const { return m_monitor_signals; }
+  static void NoOpMonitorCallback(lldb::pid_t pid, int signal, int status);
 
   // If the LaunchInfo has a monitor callback, then arrange to monitor the
   // process. Return true if the LaunchInfo has taken care of monitoring the
@@ -178,8 +175,6 @@
   std::shared_ptr<PseudoTerminal> m_pty;
   uint32_t m_resume_count = 0; // How many times do we resume after launching
   Host::MonitorChildProcessCallback m_monitor_callback;
-  void *m_monitor_callback_baton = nullptr;
-  bool m_monitor_signals = false;
   std::string m_event_data; // A string passed to the plugin launch, having no
                             // meaning to the upper levels of lldb.
   lldb::ListenerSP m_listener_sp;
Index: lldb/include/lldb/Host/HostProcess.h
===================================================================
--- lldb/include/lldb/Host/HostProcess.h
+++ lldb/include/lldb/Host/HostProcess.h
@@ -42,8 +42,7 @@
   bool IsRunning() const;
 
   llvm::Expected<HostThread>
-  StartMonitoring(const Host::MonitorChildProcessCallback &callback,
-                  bool monitor_signals);
+  StartMonitoring(const Host::MonitorChildProcessCallback &callback);
 
   HostNativeProcessBase &GetNativeProcess();
   const HostNativeProcessBase &GetNativeProcess() const;
Index: lldb/include/lldb/Host/HostNativeProcessBase.h
===================================================================
--- lldb/include/lldb/Host/HostNativeProcessBase.h
+++ lldb/include/lldb/Host/HostNativeProcessBase.h
@@ -37,8 +37,7 @@
   lldb::process_t GetSystemHandle() const { return m_process; }
 
   virtual llvm::Expected<HostThread>
-  StartMonitoring(const Host::MonitorChildProcessCallback &callback,
-                  bool monitor_signals) = 0;
+  StartMonitoring(const Host::MonitorChildProcessCallback &callback) = 0;
 
 protected:
   lldb::process_t m_process;
Index: lldb/include/lldb/Host/Host.h
===================================================================
--- lldb/include/lldb/Host/Host.h
+++ lldb/include/lldb/Host/Host.h
@@ -61,47 +61,30 @@
 /// Host is a class that answers information about the host operating system.
 class Host {
 public:
-  typedef std::function<bool(
-      lldb::pid_t pid, bool exited,
-      int signal,  // Zero for no signal
-      int status)> // Exit value of process if signal is zero
+  typedef std::function<void(lldb::pid_t pid,
+                             int signal,  // Zero for no signal
+                             int status)> // Exit value of process if signal is
+                                          // zero
       MonitorChildProcessCallback;
 
   /// Start monitoring a child process.
   ///
   /// Allows easy monitoring of child processes. \a callback will be called
-  /// when the child process exits or if it gets a signal. The callback will
-  /// only be called with signals if \a monitor_signals is \b true. \a
-  /// callback will usually be called from another thread so the callback
-  /// function must be thread safe.
-  ///
-  /// When the callback gets called, the return value indicates if monitoring
-  /// should stop. If \b true is returned from \a callback the information
-  /// will be removed. If \b false is returned then monitoring will continue.
-  /// If the child process exits, the monitoring will automatically stop after
-  /// the callback returned regardless of the callback return value.
+  /// when the child process exits or if it dies from a signal.
   ///
   /// \param[in] callback
   ///     A function callback to call when a child receives a signal
-  ///     (if \a monitor_signals is true) or a child exits.
+  ///     or exits.
   ///
   /// \param[in] pid
-  ///     The process ID of a child process to monitor, -1 for all
-  ///     processes.
-  ///
-  /// \param[in] monitor_signals
-  ///     If \b true the callback will get called when the child
-  ///     process gets a signal. If \b false, the callback will only
-  ///     get called if the child process exits.
+  ///     The process ID of a child process to monitor.
   ///
   /// \return
   ///     A thread handle that can be used to cancel the thread that
   ///     was spawned to monitor \a pid.
-  ///
-  /// \see static void Host::StopMonitoringChildProcess (uint32_t)
   static llvm::Expected<HostThread>
   StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
-                              lldb::pid_t pid, bool monitor_signals);
+                              lldb::pid_t pid);
 
   enum SystemLogType { eSystemLogWarning, eSystemLogError };
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to