Introduce a ProcessStatus monitor abstraction.

[zturner]

Summary

Note: This patch is optional, and was something I arrived at as an intermediate step while trying to implement debugging on Windows in such a way that ProcessWindows::DoLaunch() could reuse Host::LaunchProcess. While I ultimately ended up needing to circumvent the entire Host::LaunchProcess codepath entirely for techncial reasons, making this patch not strictly necessary for my subsequent patch, the abstraction still seems generally useful for creating reusable code, so I'm posting it here anyway.

This is a very general abstraction useful for local debugging as well as remote debugging, that allows implementors to customize the way in which processes are monitored for changes in status after being launched.

Additionally, a Windows implementation is provided that waits for a process to exit and notifies listeners, and Host::LaunchProcess uses this implementation.

This change is intended to provide no functional change, and consists mostly of new code, not altered code. The altered code that does exist only offers a fallback path so that if someone does not specify a ProcessStatusMonitor, it simply falls back to the old code path so that everything behaves as before.

[clayborg]

In include/lldb/Host/MonitoringProcessLauncher.h: please include lldb-forward.h and don't forward declare anything manually

In explicit MonitoringProcessLauncher(lldb::ProcessStatusMonitorSP monitor, lldb::ProcessLauncherUP delegate_launcher); I would change each arg to be a const reference to avoid copies and reference counts being bumped.

Change ProcessStatusMonitorCallbackEntry to not use std::pair and declare a small structure instead. Seeing "a.first" and "a.second" really doesn't help the readability of the code, so please make a struct with good member names ("callback" and "baton").

In lldb-forward.h the spacing of the new stuff you added seems to be using tabs as it isn't indented the same as the others. Please make sure to use spaces instead of tabs.

[zturner]

Gah, thanks for noticing the lldb-forward.h problem. It's clang-format
doing that, it's not smart enough to align variables in cases where you've
chosen to not use standard 1-space separation between type and variable
names.

[zturner]

Fixed issues suggested by Greg.

[clayborg]

Looks good.

[zturner]

I'm going to abandon this revision for now. After the discussion on-list about the high level concept of monitoring processes, and the realization that this makes no sense for windows and is different conceptually from monitoring for debug events, I think a different abstraction is needed, so this doesn't improve anything.

include/lldb/Host/HostNativeProcessBase.h

     {}
     virtual ~HostNativeProcessBase() {}
 
+    lldb::process_t
+    GetProcess() const
+    {
+        return m_process;
+    }
+
     virtual Error Terminate() = 0;
     virtual Error GetMainModule(FileSpec &file_spec) const = 0;
 

include/lldb/Host/HostProcess.h

     Error Terminate();
     Error GetMainModule(FileSpec &file_spec) const;
 
+    lldb::process_t GetProcess() const;
     lldb::pid_t GetProcessId() const;
     bool IsRunning() const;
 

include/lldb/Host/MonitoringProcessLauncher.h

 #define lldb_Host_MonitoringProcessLauncher_h_
 
 #include "lldb/Host/ProcessLauncher.h"
+#include "lldb/lldb-forward.h"
 
 namespace lldb_private
 {

 class MonitoringProcessLauncher : public ProcessLauncher
 {
   public:
-    explicit MonitoringProcessLauncher(std::unique_ptr<ProcessLauncher> delegate_launcher);
+    MonitoringProcessLauncher(lldb::ProcessStatusMonitorSP monitor, ProcessLauncher &delegate_launcher);
 
     virtual HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error);
 
   private:
-    std::unique_ptr<ProcessLauncher> m_delegate_launcher;
+    lldb::ProcessStatusMonitorSP m_monitor;
+    ProcessLauncher &m_delegate_launcher;
 };
 }
 

include/lldb/Host/ProcessStatusMonitor.h

+//===-- ProcessStatusMonitor.h ----------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_ProcessStatusMonitor_H_
+#define liblldb_Host_ProcessStatusMonitor_H_
+
+#include "lldb/Core/Error.h"
+#include "lldb/Host/HostProcess.h"
+#include "lldb/lldb-types.h"
+
+#include <list>
+#include <utility>
+
+namespace lldb_private
+{
+typedef bool (*ProcessStatusMonitorCallback)(void *baton, lldb::pid_t pid, bool exited,
+                                             int signal,  // Zero for no signal
+                                             int status); // Exit value of process if signal is zero
+
+struct ProcessStatusMonitorCallbackEntry
+{
+    ProcessStatusMonitorCallbackEntry(ProcessStatusMonitorCallback callback, void *baton)
+        : m_callback(callback), m_baton(baton)
+    {
+    }
+    ProcessStatusMonitorCallback m_callback;
+    void *m_baton;
+};
+
+typedef std::list<ProcessStatusMonitorCallbackEntry> ProcessStatusMonitorCallbackList;
+typedef std::unique_ptr<ProcessStatusMonitorCallbackList> ProcessStatusMonitorCallbackListUP;
+
+//----------------------------------------------------------------------
+// ProcessStatusMonitor
+//
+// ProcessStatusMonitor accepts callback registrations and provides an interface
+// for implementors to initiate the monitoring of a given process.  This allows,
+// for example, an implementation that launches one thread per process, or another
+// implementation that can monitor many processes on the same thread.
+//----------------------------------------------------------------------
+class ProcessStatusMonitor
+{
+  public:
+    struct MonitorData
+    {
+        ProcessStatusMonitorCallbackListUP m_callbacks;
+        HostProcess m_process;
+    };
+
+    ProcessStatusMonitor();
+    virtual ~ProcessStatusMonitor() {}
+
+    void AddProcessEventCallback(ProcessStatusMonitorCallback callback, void *baton);
+    virtual void StartMonitoring(const HostProcess &process) = 0;
+
+  protected:
+    ProcessStatusMonitorCallbackListUP m_callbacks;
+};
+}
+
+#endif

include/lldb/Host/windows/ExitStatusMonitorWindows.h

+//===-- ExitStatusMonitor.h -------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_Host_windows_ExitStatusMonitor_H_
+#define liblldb_Host_windows_ExitStatusMonitor_H_
+
+#include "lldb/Host/ProcessStatusMonitor.h"
+
+namespace lldb_private
+{
+//----------------------------------------------------------------------
+// ExitStatusMonitor
+//
+// An simple process status monitor that initiates monitoring on a new thread
+// for every process, and notifies callbacks when a process exits.
+//----------------------------------------------------------------------
+class ExitStatusMonitor : public ProcessStatusMonitor
+{
+  public:
+    virtual ~ExitStatusMonitor() {}
+
+    virtual void StartMonitoring(const HostProcess &process);
+
+  private:
+    static lldb::thread_result_t MonitorThread(void *thread_data);
+};
+}
+
+#endif

include/lldb/Host/windows/HostProcessWindows.h

     virtual HostThread StartMonitoring(HostProcess::MonitorCallback callback, void *callback_baton, bool monitor_signals);
 
   private:
-    static lldb::thread_result_t MonitorThread(void *thread_arg);
 
     void Close();
 };

include/lldb/lldb-forward.h

 class   ProcessInstanceInfo;
 class   ProcessInstanceInfoList;
 class   ProcessInstanceInfoMatch;
+class   ProcessLauncher;
 class   ProcessLaunchInfo;
+class   ProcessStatusMonitor;
+class   ProcessStatusMonitorImpl;
 class   Property;
 struct  PropertyDefinition;
 class   PythonArray;

     typedef std::shared_ptr<lldb_private::OptionValueUUID> OptionValueUUIDSP;
     typedef std::shared_ptr<lldb_private::Platform> PlatformSP;
     typedef std::shared_ptr<lldb_private::Process> ProcessSP;
+    typedef std::unique_ptr<lldb_private::ProcessLauncher> ProcessLauncherUP;
+    typedef std::shared_ptr<lldb_private::ProcessStatusMonitor> ProcessStatusMonitorSP;
+    typedef std::unique_ptr<lldb_private::ProcessStatusMonitorImpl> ProcessStatusMonitorImplUP;
     typedef std::shared_ptr<lldb_private::ProcessAttachInfo> ProcessAttachInfoSP;
     typedef std::shared_ptr<lldb_private::ProcessLaunchInfo> ProcessLaunchInfoSP;
     typedef std::weak_ptr<lldb_private::Process> ProcessWP;

source/Host/CMakeLists.txt

   common/NativeProcessProtocol.cpp
   common/NativeThreadProtocol.cpp
   common/OptionParser.cpp
+  common/ProcessStatusMonitor.cpp
   common/ProcessRunLock.cpp
   common/Socket.cpp
   common/SocketAddress.cpp

     windows/PipeWindows.cpp
     windows/ProcessLauncherWindows.cpp
     windows/ProcessRunLock.cpp
+    windows/ExitStatusMonitorWindows.cpp
     windows/ThisThread.cpp
     windows/Windows.cpp
     )

source/Host/common/Host.cpp

 
 #if defined(_WIN32)
 #include "lldb/Host/windows/ProcessLauncherWindows.h"
+#include "lldb/Host/windows/ExitStatusMonitorWindows.h"
 #else
 #include "lldb/Host/posix/ProcessLauncherPosix.h"
 #endif

 Error
 Host::LaunchProcess (ProcessLaunchInfo &launch_info)
 {
-    std::unique_ptr<ProcessLauncher> delegate_launcher;
+    ProcessStatusMonitorSP delegate_monitor;
 #if defined(_WIN32)
-    delegate_launcher.reset(new ProcessLauncherWindows());
+    ProcessLauncherWindows delegate_launcher;
+    delegate_monitor.reset(new ExitStatusMonitor());
 #else
-    delegate_launcher.reset(new ProcessLauncherPosix());
+    ProcessLauncherPosix delegate_launcher;
 #endif
-    MonitoringProcessLauncher launcher(std::move(delegate_launcher));
+    MonitoringProcessLauncher launcher(delegate_monitor, delegate_launcher);
 
     Error error;
     HostProcess process = launcher.LaunchProcess(launch_info, error);

source/Host/common/HostProcess.cpp

     return m_native_process->GetMainModule(file_spec);
 }
 
+lldb::process_t
+HostProcess::GetProcess() const
+{
+    return m_native_process->GetProcess();
+}
+
 lldb::pid_t HostProcess::GetProcessId() const
 {
     return m_native_process->GetProcessId();

source/Host/common/MonitoringProcessLauncher.cpp

 #include "lldb/Core/Module.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/MonitoringProcessLauncher.h"
+#include "lldb/Host/ProcessStatusMonitor.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/ProcessLaunchInfo.h"

 using namespace lldb;
 using namespace lldb_private;
 
-MonitoringProcessLauncher::MonitoringProcessLauncher(std::unique_ptr<ProcessLauncher> delegate_launcher)
-    : m_delegate_launcher(std::move(delegate_launcher))
+MonitoringProcessLauncher::MonitoringProcessLauncher(ProcessStatusMonitorSP monitor, ProcessLauncher &delegate_launcher)
+    : m_monitor(monitor)
+    , m_delegate_launcher(delegate_launcher)
 {
 }
 

     resolved_info.SetExecutableFile(exe_spec, false);
     assert(!resolved_info.GetFlags().Test(eLaunchFlagLaunchInTTY));
 
-    HostProcess process = m_delegate_launcher->LaunchProcess(resolved_info, error);
+    HostProcess process = m_delegate_launcher.LaunchProcess(resolved_info, error);
 
     if (process.GetProcessId() != LLDB_INVALID_PROCESS_ID)
     {

             callback = Process::SetProcessExitStatus;
         }
 
-        process.StartMonitoring(callback, baton, monitor_signals);
+        if (m_monitor)
+        {
+            // If the ProcessLaunchInfo specified a callback, make sure it's registered in
+            // addition to anything else that may have been registered by the creator of the
+            // status monitor.
+            m_monitor->AddProcessEventCallback(callback, baton);
+            m_monitor->StartMonitoring(process);
+        }
+        else
+        {
+            // FIXME: This branch should go away if everything used a ProcessStatusMonitor and we
+            // could assume that m_monitor was non-null.
+            process.StartMonitoring(callback, baton, monitor_signals);
+        }
+
         if (log)
             log->PutCString("started monitoring child process.");
     }

source/Host/common/ProcessStatusMonitor.cpp

+//===-- ProcessStatusMonitor.cpp --------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/ProcessStatusMonitor.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+ProcessStatusMonitor::ProcessStatusMonitor()
+    : m_callbacks(new ProcessStatusMonitorCallbackList())
+{
+}
+
+void
+ProcessStatusMonitor::AddProcessEventCallback(ProcessStatusMonitorCallback callback, void *baton)
+{
+    m_callbacks->push_back(ProcessStatusMonitorCallbackEntry(callback, baton));
+}

source/Host/windows/ExitStatusMonitorWindows.cpp

+//===-- ExitStatusMonitorWindows.cpp ----------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/HostProcess.h"
+#include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Host/windows/ExitStatusMonitorWindows.h"
+#include "lldb/Host/windows/windows.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+void
+ExitStatusMonitor::StartMonitoring(const HostProcess &process)
+{
+    if (!m_callbacks)
+        return;
+
+    MonitorData *thread_data = new MonitorData();
+    thread_data->m_callbacks = std::move(m_callbacks);
+    thread_data->m_process = process;
+    ThreadLauncher::LaunchThread("ExitStatusMonitorThread", MonitorThread, thread_data, nullptr);
+}
+
+lldb::thread_result_t
+ExitStatusMonitor::MonitorThread(void *data)
+{
+    MonitorData *thread_data = static_cast<MonitorData *>(data);
+    HostProcess process(thread_data->m_process);
+    std::unique_ptr<ProcessStatusMonitorCallbackList> callbacks(std::move(thread_data->m_callbacks));
+
+    DWORD exit_code = 0;
+    DWORD wait_result = ::WaitForSingleObject(process.GetProcess(), INFINITE);
+    ::GetExitCodeProcess(process.GetProcess(), &exit_code);
+
+    for (const auto &entry : *callbacks)
+        entry.m_callback(entry.m_baton, process.GetProcessId(), true, 0, exit_code);
+
+    delete thread_data;
+
+    return 0;
+}

source/Host/windows/HostProcessWindows.cpp

 
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Host/HostThread.h"
+#include "lldb/Host/ProcessStatusMonitor.h"
 #include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Host/windows/windows.h"
+#include "lldb/Host/windows/ExitStatusMonitorWindows.h"
 #include "lldb/Host/windows/HostProcessWindows.h"
+#include "lldb/Host/windows/windows.h"
 
 #include "llvm/ADT/STLExtras.h"
 

 
 using namespace lldb_private;
 
-namespace
-{
-struct MonitorInfo
-{
-    HostProcess::MonitorCallback callback;
-    void *baton;
-    HANDLE process_handle;
-};
-}
-
 HostProcessWindows::HostProcessWindows()
     : HostNativeProcessBase()
 {

 HostThread
 HostProcessWindows::StartMonitoring(HostProcess::MonitorCallback callback, void *callback_baton, bool monitor_signals)
 {
-    HostThread monitor_thread;
-    MonitorInfo *info = new MonitorInfo;
-    info->callback = callback;
-    info->baton = callback_baton;
-
-    // Since the life of this HostProcessWindows instance and the life of the process may be different, duplicate the handle so that
-    // the monitor thread can have ownership over its own copy of the handle.
-    HostThread result;
-    if (::DuplicateHandle(GetCurrentProcess(), m_process, GetCurrentProcess(), &info->process_handle, 0, FALSE, DUPLICATE_SAME_ACCESS))
-        result = ThreadLauncher::LaunchThread("ChildProcessMonitor", HostProcessWindows::MonitorThread, info, nullptr);
-    return result;
-}
-
-lldb::thread_result_t
-HostProcessWindows::MonitorThread(void *thread_arg)
-{
-    DWORD exit_code;
-
-    MonitorInfo *info = static_cast<MonitorInfo *>(thread_arg);
-    if (info)
-    {
-        DWORD wait_result = ::WaitForSingleObject(info->process_handle, INFINITE);
-        ::GetExitCodeProcess(info->process_handle, &exit_code);
-        info->callback(info->baton, ::GetProcessId(info->process_handle), true, 0, exit_code);
-        ::CloseHandle(info->process_handle);
-        delete (info);
-    }
-    return 0;
+    // Not implemented.  Remove this after converting to ProcessStatusMonitor.
+    return HostThread();
 }
 
 void HostProcessWindows::Close()

source/Host/windows/ProcessLauncherWindows.cpp

     startupinfo.hStdInput = stdin_handle;
     startupinfo.hStdOutput = stdout_handle;
 
+    DWORD flags = CREATE_NEW_CONSOLE;
+    if (launch_info.GetFlags().Test(eLaunchFlagDebug))
+        flags |= DEBUG_ONLY_THIS_PROCESS;
+
     executable = launch_info.GetExecutableFile().GetPath();
     launch_info.GetArguments().GetQuotedCommandString(commandLine);
-    BOOL result = ::CreateProcessA(executable.c_str(), const_cast<char *>(commandLine.c_str()), NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL,
+    BOOL result = ::CreateProcessA(executable.c_str(), const_cast<char *>(commandLine.c_str()), NULL, NULL, TRUE, flags, NULL,
                                    launch_info.GetWorkingDirectory(), &startupinfo, &pi);
     if (result)
     {