Add code to exit the NativeProcessLinux Monitor thread on android

[tberghammer]

Summary

Add code to exit the NativeProcessLinux Monitor thread on android

This CL change the logic used to terminate the monitor thread of NativeProcessLinux to use a signal instead of pthread_cancel as pthread_cancel is not supported on android.

[labath]

Please see my inline comments on the signal handler.

A thought for general discussion: I am wondering whether we could avoid having two methods for shutting down a thread. If the code in question is not likely to be used on windows (which I think it's not as it is just a glorified waitpid() wrapper and waitpid is very posix-y), then all other platforms should support the pthread_kill method and we could use a single approach everywhere. Thoughts?

source/Host/common/Host.cpp

176

Be bold and specify the signal behavior that you expect, don't rely on them being what you expect by default.
If the previous signal handler has SA_RESTART set, you would achieve nothing, as the waitpid call would get restarted. If the previous handler had SA_RESETHAND set, your trick would only work once.

178

There is nothing special about SIGCHLD - any signal can interrupt a system call. Please use a signal which is not overloaded with other meanings, e.g. SIGUSR1.

198

Be careful about race conditions here. What if your signal is not received in the waitpid call, but slightly before? The signal will be handled, but you will still end up blocked forever.
I don't think there is a completely race-free solution to this problem (apart from resending the signal until it works), but we should at least try to decrease the race window by testing the exit condition before and after the call.

[ovyalov]

Please see my comments.

source/Host/common/Host.cpp

178

+1 to use SIGUSR1

198

We may use sigtimedwait instead of waitpid on Android with some conservative timeout (to avoid a possibility to be blocked forever in case of thread race) and if the function timeouts -(callback (callback_baton, -1,..).

source/Plugins/Process/Android/NativeProcessAndroid.cpp

11

Please check whether you do need this include.

64

Could you make it unconditional - m_monitor_thread_running = error.Success () ?

72

ditto.

95

There is a chance that m_monitor_thread_running will updated and read concurrently from different threads - please consider an option to use atomic type here.

source/Plugins/Process/Linux/NativeProcessLinux.h

224

s/DoLaunch/DoAttach

233

I'd rather named this method StopMonitorThread in order to make it complement to StartMonitorThread.

[vharron]

Thanks for the patch! Looking forward to v2!

source/Host/common/Host.cpp

146

Rename to "emptySignalHandler"?

Comment that the signal is only to interrupt the thread from waitpid

178

+1 to use SIGUSR1

194

Please remove all pthread_cancel code paths. Linux and Android should follow the same code paths.

201

Why are we calling a function? It seems like we should just check m_monitor_thread_running? If you want to log something, do it after the check, before the break. Reduces complexity

source/Plugins/Process/Android/NativeProcessAndroid.cpp

95

I don't think that his current usage is a problem but it might cause noise in ThreadSanitizer and an atomic can't hurt.

[tberghammer]

Thanks for the comments. Based on them I created a new patch which use the same (signal based) logic for Linux and Android. I still left the pthread_cancel logic in place in MonitorChildProcessThreadFunction because it is used in several other case also and I don't have the environment set up to update and test the effect of the change in those cases.

source/Host/common/Host.cpp

178

Done

194

Changed Linux to use the same logic used by Android, but other architectures (e.g.: FreeBSD) and Local debugging also use this function, so I still have to leave the pthread_cancel there.

198

We can't specify the pid of the child we waiting for in sigtimedwait what is required in this case. Especially as sometimes we use group pid instead of a process pid what is handled by waitpid, but handling it manually with sigtimedwait would be tricky.

201

We don't have explicit access to the instance of NativeProcessAndroid (we are in a static function in an other module). I can cast callback_baton into NativeProcessAndroid and then check for the flag, but I think that one is a bit nasty solution.

source/Plugins/Process/Linux/NativeProcessLinux.h

224

Done

233

Done

[ovyalov]

source/Host/common/Host.cpp

188

I'd rather put this snippet into a separate function since it's called twice:

bool CheckForCancellation()
{
#ifdef __linux__
        if (g_usr1_signal_thread == ::pthread_self())
        {
            g_usr1_signal_thread = 0;
            return true;
        }
        return false;
#else
        ::pthread_testcancel ();
#endif
}

256

Unrelated but looks weird - could you remove this log re-initialization? It looks unnecessary.

[labath]

I have an issue with one of the comments, but otherwise looks fine.

source/Host/common/Host.cpp

146

Please remove the std::set reference, as it is not a valid solution. There are very few things that can be done safely in a signal handler and std::set manipulation is not one of them. If we ever need to cancel multiple threads, we will need to do something more clever, probably set a flag on NativeProcessAndroid (outside of the signal handler), as @vharron suggseted.

[labath]

source/Host/common/Host.cpp

146

It just occurred to me that this would be a good use case for a thread_local variable. I'm not sure what is the official policy on thread local variables, as I can't find any occurrences in the codebase, but if that's ok then the killer solution would be

static thread_local volatile sig_atomic_t g_usr1_called;

If not then you can at least make your existing variable volatile for some added safety.

[tberghammer]

source/Host/common/Host.cpp

146

Done

188

Done

256

It is necessary to let the user enable the logging after starting the monitor thread.

[labath]

lgtm

[ovyalov]

LGTM

cmake/LLDBDependencies.cmake

 
 # Linux-only libraries
 if ( CMAKE_SYSTEM_NAME MATCHES "Linux" )
   list(APPEND LLDB_USED_LIBS
     lldbPluginProcessLinux
     lldbPluginProcessPOSIX
     lldbPluginProcessElfCore
     lldbPluginJITLoaderGDB
     )
 endif ()
 
+# Android only libraries
+if ( __ANDROID_NDK__ )
+  list(APPEND LLDB_USED_LIBS
+    lldbPluginProcessAndroid
+    )
+endif ()
+
 # FreeBSD-only libraries
 if ( CMAKE_SYSTEM_NAME MATCHES "FreeBSD" )
   list(APPEND LLDB_USED_LIBS
     lldbPluginProcessFreeBSD
     lldbPluginProcessPOSIX
     lldbPluginProcessElfCore
     lldbPluginJITLoaderGDB
     )

source/Host/common/Host.cpp

~ScopedPThreadCancelDisabler()
         if (m_old_state != -1)
             ::pthread_setcancelstate (m_old_state, 0);
     }
 private:
     int m_old_state;    // Save the old cancelability state.
 };
 #endif // __ANDROID_NDK__
 
+#ifdef __ANDROID__
+static void SigChldHandler(int) {}
+#endif // __ANDROID__
+
 static thread_result_t
 MonitorChildProcessThreadFunction (void *arg)
 {
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     const char *function = __FUNCTION__;
     if (log)
         log->Printf ("%s (arg = %p) thread starting...", function, arg);
 
MonitorChildProcessThreadFunction (void *arg)
     delete info;
 
     int status = -1;
 #if defined (__FreeBSD__) || defined (__FreeBSD_kernel__)
     #define __WALL 0
 #endif
     const int options = __WALL;
 
+#ifdef __ANDROID__
+    struct sigaction sigChldAction;
+    ::sigaction(SIGCHLD, NULL, &sigChldAction);
+    sigChldAction.sa_handler = SigChldHandler;
+    ::sigaction(SIGCHLD, &sigChldAction, NULL);
+#endif // __ANDROID__    
+
     while (1)
     {
         log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS);
         if (log)
             log->Printf("%s ::waitpid (pid = %" PRIi32 ", &status, options = %i)...", function, pid, options);
 
         // Wait for all child processes
 #if !defined(__ANDROID__) && !defined(__ANDROID_NDK__)
         ::pthread_testcancel ();
 #endif
         // Get signals from all children with same process group of pid
         const ::pid_t wait_pid = ::waitpid (pid, &status, options);
 #if !defined(__ANDROID__) && !defined(__ANDROID_NDK__)
         ::pthread_testcancel ();
 #endif
         if (wait_pid == -1)
         {
             if (errno == EINTR)
+            {
+#ifdef __ANDROID__
+                // On Android we have to call the callback on EINTR also because the monitor thread
+                // is stopped with sending a SIGCHLD and then returning true from the callback
+                if (callback (callback_baton, -1, false, 0, 0))
+                {
+                    if (log)
+                        log->Printf ("%s (arg = %p) thread exiting because callback returned true...", __FUNCTION__, arg);
+                    break;
+                }
+#endif // __ANDROID__
                 continue;
+            }
             else
             {
                 if (log)
                     log->Printf ("%s (arg = %p) thread exiting because waitpid failed (%s)...", __FUNCTION__, arg, strerror(errno));
                 break;
             }
         }
         else if (wait_pid > 0)
#endif // __ANDROID__
             }
 
             // Scope for pthread_cancel_disabler
             {
 #if !defined(__ANDROID__) && !defined(__ANDROID_NDK__)
                 ScopedPThreadCancelDisabler pthread_cancel_disabler;
 #endif
 
                 log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS);
                 if (log)
                     log->Printf ("%s ::waitpid (pid = %" PRIi32 ", &status, options = %i) => pid = %" PRIi32 ", status = 0x%8.8x (%s), signal = %i, exit_state = %i",
                                  function,
                                  pid,
                                  options,
                                  wait_pid,
                                  status,
                                  status_cstr,

source/Plugins/Platform/Android/PlatformAndroid.h

public:
 
         //------------------------------------------------------------
         // lldb_private::Platform functions
         //------------------------------------------------------------
 
         lldb_private::Error
         ConnectRemote (lldb_private::Args& args) override;
 
+        Error
+        LaunchNativeProcess (ProcessLaunchInfo &launch_info,
+                             lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
+                             NativeProcessProtocolSP &process_sp) override;
+
+        Error
+        AttachNativeProcess (lldb::pid_t pid,
+                             lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
+                             NativeProcessProtocolSP &process_sp) override;
+
     private:
         DISALLOW_COPY_AND_ASSIGN (PlatformAndroid);
     };
 } // namespace lldb_private
 
 #endif  // liblldb_PlatformAndroid_h_

source/Plugins/Platform/Android/PlatformAndroid.cpp

 //===-- PlatformAndroid.cpp -------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
 #include "lldb/Core/Log.h"
+#include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/HostInfo.h"
 
 // Project includes
 #include "PlatformAndroid.h"
 #include "PlatformAndroidRemoteGDBServer.h"
 
+#if defined(__ANDROID__)
+#include "../../Process/Android/NativeProcessAndroid.h"
+#endif
+
 using namespace lldb;
 using namespace lldb_private;
 
 static uint32_t g_initialize_count = 0;
 
 void
 PlatformAndroid::Initialize ()
 {
PlatformAndroid::ConnectRemote (Args& args)
     {
         return Error ("can't connect to the host platform '%s', always connected", GetPluginName().GetCString());
     }
 
     if (!m_remote_platform_sp)
         m_remote_platform_sp = PlatformSP(new PlatformAndroidRemoteGDBServer());
     return PlatformLinux::ConnectRemote (args);
 }
+
+Error
+PlatformAndroid::LaunchNativeProcess (
+    ProcessLaunchInfo &launch_info,
+    lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
+    NativeProcessProtocolSP &process_sp)
+{
+#if !defined(__ANDROID__)
+    return Error("Only implemented on Android hosts");
+#else
+    if (!IsHost ())
+        return Error("PlatformAndroid::%s (): cannot launch a debug process when not the host", __FUNCTION__);
+
+    // Retrieve the exe module.
+    lldb::ModuleSP exe_module_sp;
+    ModuleSpec exe_module_spec(launch_info.GetExecutableFile(), launch_info.GetArchitecture());
+
+    Error error = ResolveExecutable (exe_module_spec, exe_module_sp, NULL);
+    if (!error.Success ())
+        return error;
+
+    if (!exe_module_sp)
+        return Error("exe_module_sp could not be resolved for %s", launch_info.GetExecutableFile ().GetPath ().c_str ());
+
+    // Launch it for debugging
+    return NativeProcessAndroid::LaunchProcess (exe_module_sp.get (),
+                                                launch_info,
+                                                native_delegate,
+                                                process_sp);
+#endif
+}
+
+Error
+PlatformAndroid::AttachNativeProcess (lldb::pid_t pid,
+                                    lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
+                                    NativeProcessProtocolSP &process_sp)
+{
+#if !defined(__ANDROID__)
+    return Error("Only implemented on Android hosts");
+#else
+    if (!IsHost ())
+        return Error("PlatformAndroid::%s (): cannot attach to a debug process when not the host", __FUNCTION__);
+
+    // Launch it for debugging
+    return NativeProcessAndroid::AttachToProcess (pid, native_delegate, process_sp);
+#endif
+}
 No newline at end of file

source/Plugins/Process/Android/CMakeLists.txt

+set(LLVM_NO_RTTI 1)
+
+include_directories(.)
+include_directories(../Linux)
+
+add_lldb_library(lldbPluginProcessAndroid
+  NativeProcessAndroid.cpp
+  )
+

source/Plugins/Process/Android/Makefile

+##===- source/Plugins/Process/Android/Makefile -------------*- Makefile -*-===##
+# 
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+# 
+##===----------------------------------------------------------------------===##
+
+LLDB_LEVEL := ../../../..
+LIBRARYNAME := lldbPluginProcessAndroid
+BUILD_ARCHIVE = 1
+
+include $(LLDB_LEVEL)/Makefile

source/Plugins/Process/Android/NativeProcessAndroid.h

+//===-- NativeProcessAndroid.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_NativeProcessAndroid_H_
+#define liblldb_NativeProcessAndroid_H_
+
+#include "NativeProcessLinux.h"
+
+namespace lldb_private
+{
+
+    class NativeProcessAndroid : public NativeProcessLinux
+    {
+    public:
+        // ---------------------------------------------------------------------
+        // Public Static Methods
+        // ---------------------------------------------------------------------
+        static Error
+        LaunchProcess (Module *exe_module,
+                       ProcessLaunchInfo &launch_info,
+                       NativeProcessProtocol::NativeDelegate &native_delegate,
+                       NativeProcessProtocolSP &native_process_sp);
+
+        static Error
+        AttachToProcess (lldb::pid_t pid,
+                         NativeProcessProtocol::NativeDelegate &native_delegate,
+                         NativeProcessProtocolSP &native_process_sp);
+
+    protected:
+        // Launches an inferior process ready for debugging.  Forms the
+        // implementation of Process::DoLaunch.
+        void
+        LaunchInferior (Module *module,
+                        char const *argv[],
+                        char const *envp[],
+                        const std::string &stdin_path,
+                        const std::string &stdout_path,
+                        const std::string &stderr_path,
+                        const char *working_dir,
+                        const ProcessLaunchInfo &launch_info,
+                        Error &error) override;
+
+        // Attaches to an existing process.  Forms the
+        // implementation of Process::DoLaunch.
+        void
+        AttachToInferior (lldb::pid_t pid, Error &error) override;
+
+        HostThread
+        StartMonitorThread () override;
+
+        // Stops the child monitor thread.
+        void
+        StopMonitoringChildProcess() override;
+
+    private:
+        bool m_monitor_thread_running;
+
+        NativeProcessAndroid();
+
+        static bool
+        MonitorCallback(void *callback_baton, lldb::pid_t pid, bool exited, int signal, int status);
+    };
+
+} // end of namespace lldb_private
+
+#endif // liblldb_NativeProcessAndroid_H_

source/Plugins/Process/Android/NativeProcessAndroid.cpp

+//===-- NativeProcessAndroid.cpp ------------------------------ -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "NativeProcessAndroid.h"
+#include "NativeProcessLinux.h"
+#include "ThreadStateCoordinator.h"
+
+// Other libraries and framework includes
+#include "lldb/Core/Error.h"
+#include "lldb/Host/HostNativeThread.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+NativeProcessAndroid::NativeProcessAndroid() :
+    m_monitor_thread_running (false)
+{
+}
+
+lldb_private::Error
+NativeProcessAndroid::LaunchProcess (Module *exe_module,
+                                     ProcessLaunchInfo &launch_info,
+                                     NativeProcessProtocol::NativeDelegate &native_delegate,
+                                     NativeProcessProtocolSP &native_process_sp)
+{
+    native_process_sp.reset (new NativeProcessAndroid ());
+    return NativeProcessLinux::LaunchProcessHelper (exe_module, launch_info, native_delegate,
+                                                    native_process_sp);
+}
+
+lldb_private::Error
+NativeProcessAndroid::AttachToProcess (lldb::pid_t pid,
+                                       NativeProcessProtocol::NativeDelegate &native_delegate,
+                                       NativeProcessProtocolSP &native_process_sp)
+{
+    native_process_sp.reset (new NativeProcessAndroid ());
+    return NativeProcessLinux::AttachToProcessHelper (pid, native_delegate, native_process_sp);
+}
+
+void
+NativeProcessAndroid::LaunchInferior (Module *module,
+                                      const char *argv[],
+                                      const char *envp[],
+                                      const std::string &stdin_path,
+                                      const std::string &stdout_path,
+                                      const std::string &stderr_path,
+                                      const char *working_dir,
+                                      const lldb_private::ProcessLaunchInfo &launch_info,
+                                      lldb_private::Error &error)
+{
+    NativeProcessLinux::LaunchInferior (module, argv, envp, stdin_path, stdout_path, stderr_path,
+                                        working_dir, launch_info, error);
+    if (error.Success ())
+        m_monitor_thread_running = true;
+}
+
+void
+NativeProcessAndroid::AttachToInferior (lldb::pid_t pid, lldb_private::Error &error)
+{
+    NativeProcessLinux::AttachToInferior (pid, error);
+    if (error.Success ())
+        m_monitor_thread_running = true;
+}
+
+bool
+NativeProcessAndroid::MonitorCallback (void *callback_baton,
+                                       lldb::pid_t pid,
+                                       bool exited,
+                                       int signal,
+                                       int status)
+{
+  if ((::pid_t)pid == -1)
+  {
+      Log *log (GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PROCESS));
+
+      NativeProcessAndroid *const process = static_cast<NativeProcessAndroid*>(callback_baton);
+      assert (process && "process is null");
+      if (!process)
+      {
+          if (log)
+              log->Printf ("NativeProcessAndroid::%s pid %" PRIu64 " callback_baton was null, can't determine process to use", __FUNCTION__, pid);
+          return true;
+      }
+
+      return !process->m_monitor_thread_running;
+  }
+
+  return NativeProcessLinux::MonitorCallback (callback_baton, pid, exited, signal, status);
+}
+
+void
+NativeProcessAndroid::StopMonitoringChildProcess()
+{
+    if (m_monitor_thread.IsJoinable())
+    {
+        m_monitor_thread_running = false;
+        ::pthread_kill(m_monitor_thread.GetNativeThread().GetSystemHandle(), SIGCHLD);
+        m_monitor_thread.Join(nullptr);
+    }
+}
+
+HostThread
+NativeProcessAndroid::StartMonitorThread ()
+{
+    return Host::StartMonitoringChildProcess (NativeProcessAndroid::MonitorCallback, this, GetID (), true);
+}

source/Plugins/Process/CMakeLists.txt

 if (CMAKE_SYSTEM_NAME MATCHES "Linux")
+  if (__ANDROID_NDK__)
+    add_subdirectory(Android)
+  endif()
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
   add_subdirectory(FreeBSD)
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_subdirectory(Windows)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(MacOSX-Kernel)
 endif()
 add_subdirectory(gdb-remote)
 add_subdirectory(Utility)
 add_subdirectory(mach-core)
 add_subdirectory(elf-core)

source/Plugins/Process/Linux/NativeProcessLinux.h

public:
         /// Writes generic floating point registers into the specified buffer.
         Error
         WriteFPR(lldb::tid_t tid, void *buf, size_t buf_size);
 
         /// Writes the specified register set into the specified buffer.
         /// For instance, the extended floating-point register set.
         Error
         WriteRegisterSet(lldb::tid_t tid, void *buf, size_t buf_size, unsigned int regset);
-        
     protected:
-        // ---------------------------------------------------------------------
-        // NativeProcessProtocol protected interface
-        // ---------------------------------------------------------------------
-        Error
-        GetSoftwareBreakpointTrapOpcode (size_t trap_opcode_size_hint, size_t &actual_opcode_size, const uint8_t *&trap_opcode_bytes) override;
-
-    private:
-
         lldb_private::ArchSpec m_arch;
 
         HostThread m_operation_thread;
         HostThread m_monitor_thread;
 
         // current operation which must be executed on the priviliged thread
         void *m_operation;
         lldb_private::Mutex m_operation_mutex;
 
         // semaphores notified when Operation is ready to be processed and when
         // the operation is complete.
         sem_t m_operation_pending;
         sem_t m_operation_done;
 
         lldb_private::LazyBool m_supports_mem_region;
         std::vector<MemoryRegionInfo> m_mem_region_cache;
         lldb_private::Mutex m_mem_region_cache_mutex;
 
         std::unique_ptr<ThreadStateCoordinator> m_coordinator_up;
         HostThread m_coordinator_thread;
 
+        static lldb_private::Error
+        LaunchProcessHelper (Module *exe_module,
+                             ProcessLaunchInfo &launch_info,
+                             NativeProcessProtocol::NativeDelegate &native_delegate,
+                             NativeProcessProtocolSP &native_process_sp);
+
+        static lldb_private::Error
+        AttachToProcessHelper (lldb::pid_t pid,
+                               NativeProcessProtocol::NativeDelegate &native_delegate,
+                               NativeProcessProtocolSP &native_process_sp);
+
+        NativeProcessLinux ();
+
+        // ---------------------------------------------------------------------
+        // NativeProcessProtocol protected interface
+        // ---------------------------------------------------------------------
+        Error
+        GetSoftwareBreakpointTrapOpcode (size_t trap_opcode_size_hint,
+                                         size_t &actual_opcode_size,
+                                         const uint8_t *&trap_opcode_bytes) override;
+
+        /// Launches an inferior process ready for debugging.  Forms the
+        /// implementation of Process::DoLaunch.
+        virtual void
+        LaunchInferior (Module *module,
+                        char const *argv[],
+                        char const *envp[],
+                        const std::string &stdin_path,
+                        const std::string &stdout_path,
+                        const std::string &stderr_path,
+                        const char *working_dir,
+                        const lldb_private::ProcessLaunchInfo &launch_info,
+                        Error &error);
+
+        /// Attaches to an existing process.  Forms the
+        /// implementation of Process::DoLaunch.
+        virtual void
+        AttachToInferior (lldb::pid_t pid, Error &error);
+
+        virtual HostThread
+        StartMonitorThread ();
+
+        /// Stops the child monitor thread.
+        virtual void
+        StopMonitoringChildProcess();
+
+        static bool
+        MonitorCallback(void *callback_baton, lldb::pid_t pid, bool exited, int signal, int status);
+
+    private:
         struct OperationArgs
         {
             OperationArgs(NativeProcessLinux *monitor);
 
             ~OperationArgs();
 
             NativeProcessLinux *m_monitor;      // The monitor performing the attach.
             sem_t m_semaphore;              // Posted to once operation complete.
private:
             ~AttachArgs();
 
             lldb::pid_t m_pid;              // pid of the process to be attached.
         };
 
         // ---------------------------------------------------------------------
         // Private Instance Methods
         // ---------------------------------------------------------------------
-        NativeProcessLinux ();
-
-        /// Launches an inferior process ready for debugging.  Forms the
-        /// implementation of Process::DoLaunch.
-        void
-        LaunchInferior (
-            Module *module,
-            char const *argv[],
-            char const *envp[],
-            const std::string &stdin_path,
-            const std::string &stdout_path,
-            const std::string &stderr_path,
-            const char *working_dir,
-            const lldb_private::ProcessLaunchInfo &launch_info,
-            Error &error);
-
-        /// Attaches to an existing process.  Forms the
-        /// implementation of Process::DoLaunch.
-        void
-        AttachToInferior (lldb::pid_t pid, Error &error);
-
         void
         StartLaunchOpThread(LaunchArgs *args, lldb_private::Error &error);
 
         static void *
         LaunchOpThread(void *arg);
 
         static bool
         Launch(LaunchArgs *args);
private:
         SetDefaultPtraceOpts(const lldb::pid_t);
 
         static void
         ServeOperation(OperationArgs *args);
 
         static bool
         DupDescriptor(const char *path, int fd, int flags);
 
-        static bool
-        MonitorCallback(void *callback_baton,
-                lldb::pid_t pid, bool exited, int signal, int status);
-
         void
         MonitorSIGTRAP(const siginfo_t *info, lldb::pid_t pid);
 
         void
         MonitorSignal(const siginfo_t *info, lldb::pid_t pid, bool exited);
 
 #if 0
         static ::ProcessMessage::CrashReason
         GetCrashReasonForSIGSEGV(const siginfo_t *info);
 
         static ::ProcessMessage::CrashReason
         GetCrashReasonForSIGILL(const siginfo_t *info);
 
         static ::ProcessMessage::CrashReason
         GetCrashReasonForSIGFPE(const siginfo_t *info);
 
         static ::ProcessMessage::CrashReason
         GetCrashReasonForSIGBUS(const siginfo_t *info);
 #endif
 
         void
         DoOperation(void *op);
 
-        /// Stops the child monitor thread.
-        void
-        StopMonitoringChildProcess();
-
         /// Stops the operation thread used to attach/launch a process.
         void
         StopOpThread();
 
         Error
         StartCoordinatorThread ();
 
         static void*

source/Plugins/Process/Linux/NativeProcessLinux.cpp

 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Scalar.h"
 #include "lldb/Core/State.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Host/linux/HostThreadLinux.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/PseudoTerminal.h"
 
 #include "lldb/Host/common/NativeBreakpoint.h"
 #include "Utility/StringExtractor.h"

 
 lldb_private::Error
 NativeProcessLinux::LaunchProcess (
     lldb_private::Module *exe_module,
     lldb_private::ProcessLaunchInfo &launch_info,
     lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
     NativeProcessProtocolSP &native_process_sp)
 {
+    native_process_sp.reset (new NativeProcessLinux ());
+    return NativeProcessLinux::LaunchProcessHelper (exe_module, launch_info, native_delegate,
+                                                    native_process_sp);
+}
+
+lldb_private::Error
+NativeProcessLinux::AttachToProcess (
+    lldb::pid_t pid,
+    lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
+    NativeProcessProtocolSP &native_process_sp)
+{
+    native_process_sp.reset (new NativeProcessLinux ());
+    return NativeProcessLinux::AttachToProcessHelper (pid, native_delegate, native_process_sp);
+}
+
+// -----------------------------------------------------------------------------
+// Protected Static Methods
+// -----------------------------------------------------------------------------
+
+lldb_private::Error
+NativeProcessLinux::LaunchProcessHelper (
+    lldb_private::Module *exe_module,
+    lldb_private::ProcessLaunchInfo &launch_info,
+    lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
+    NativeProcessProtocolSP &native_process_sp)
+{
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
     Error error;
 
     // Verify the working directory is valid if one was specified.
     const char* working_dir = launch_info.GetWorkingDirectory ();
     if (working_dir)
     {
if (log)
             log->Printf ("NativeProcessLinux::%s leaving STDOUT as is", __FUNCTION__);
 
         if (!stderr_path.empty ())
             log->Printf ("NativeProcessLinux::%s setting STDERR to '%s'", __FUNCTION__, stderr_path.c_str ());
         else
             log->Printf ("NativeProcessLinux::%s leaving STDERR as is", __FUNCTION__);
     }
 
-    // Create the NativeProcessLinux in launch mode.
-    native_process_sp.reset (new NativeProcessLinux ());
-
     if (log)
     {
         int i = 0;
         for (const char **args = launch_info.GetArguments ().GetConstArgumentVector (); *args; ++args, ++i)
         {
             log->Printf ("NativeProcessLinux::%s arg %d: \"%s\"", __FUNCTION__, i, *args ? *args : "nullptr");
             ++i;
         }
NativeProcessLinux::LaunchProcessHelper (
     }
 
     launch_info.SetProcessID (native_process_sp->GetID ());
 
     return error;
 }
 
 lldb_private::Error
-NativeProcessLinux::AttachToProcess (
+NativeProcessLinux::AttachToProcessHelper (
     lldb::pid_t pid,
     lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
     NativeProcessProtocolSP &native_process_sp)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     if (log && log->GetMask ().Test (POSIX_LOG_VERBOSE))
         log->Printf ("NativeProcessLinux::%s(pid = %" PRIi64 ")", __FUNCTION__, pid);
 
     // Grab the current platform architecture.  This should be Linux,
     // since this code is only intended to run on a Linux host.
     PlatformSP platform_sp (Platform::GetHostPlatform ());
     if (!platform_sp)
         return Error("failed to get a valid default platform");
 
     // Retrieve the architecture for the running process.
     ArchSpec process_arch;
     Error error = ResolveProcessArchitecture (pid, *platform_sp.get (), process_arch);
     if (!error.Success ())
         return error;
 
-    std::shared_ptr<NativeProcessLinux> native_process_linux_sp (new NativeProcessLinux ());
+    std::shared_ptr<NativeProcessLinux> native_process_linux_sp (
+        std::static_pointer_cast<NativeProcessLinux> (native_process_sp));
 
     if (!native_process_linux_sp->RegisterNativeDelegate (native_delegate))
     {
         error.SetErrorStringWithFormat ("failed to register the native delegate");
         return error;
     }
 
     native_process_linux_sp->AttachToInferior (pid, error);
     if (!error.Success ())
         return error;
 
-    native_process_sp = native_process_linux_sp;
     return error;
 }
 
 // -----------------------------------------------------------------------------
 // Public Instance Methods
 // -----------------------------------------------------------------------------
 
 NativeProcessLinux::NativeProcessLinux () :
WAIT_AGAIN:
     {
         StopOpThread();
         StopCoordinatorThread ();
         error = args->m_error;
         return;
     }
 
     // Finally, start monitoring the child process for change in state.
-    m_monitor_thread = Host::StartMonitoringChildProcess(
-        NativeProcessLinux::MonitorCallback, this, GetID(), true);
+    m_monitor_thread = StartMonitorThread();
     if (!m_monitor_thread.IsJoinable())
     {
         error.SetErrorToGenericError();
         error.SetErrorString ("Process attach failed to create monitor thread for NativeProcessLinux::MonitorCallback.");
         return;
     }
 }
 
WAIT_AGAIN:
     {
         StopOpThread ();
         StopCoordinatorThread ();
         error = args->m_error;
         return;
     }
 
     // Finally, start monitoring the child process for change in state.
-    m_monitor_thread = Host::StartMonitoringChildProcess (
-        NativeProcessLinux::MonitorCallback, this, GetID (), true);
+    m_monitor_thread = StartMonitorThread();
     if (!m_monitor_thread.IsJoinable())
     {
         error.SetErrorToGenericError ();
         error.SetErrorString ("Process attach failed to create monitor thread for NativeProcessLinux::MonitorCallback.");
         return;
     }
 }
 
+HostThread
+NativeProcessLinux::StartMonitorThread ()
+{
+    return Host::StartMonitoringChildProcess (NativeProcessLinux::MonitorCallback, this, GetID (), true);
+}
+
 void
 NativeProcessLinux::Terminate ()
 {
     StopMonitor();
 }
 
 //------------------------------------------------------------------------------
 // Thread setup and tear down.
else
         // We don't know what this is.
         return ExitType::eExitTypeInvalid;
     }
 }
 
 // Main process monitoring waitpid-loop handler.
 bool
 NativeProcessLinux::MonitorCallback(void *callback_baton,
                                     lldb::pid_t pid,
                                     bool exited,
                                     int signal,
                                     int status)
 {
     Log *log (GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PROCESS));
 
     NativeProcessLinux *const process = static_cast<NativeProcessLinux*>(callback_baton);
     assert (process && "process is null");
     if (!process)
     {
         if (log)