diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h index 459e9f563d1e..c6d5c3c4b67a 100644 --- a/lldb/include/lldb/Host/Host.h +++ b/lldb/include/lldb/Host/Host.h @@ -199,9 +199,6 @@ public: static const lldb::UnixSignalsSP &GetUnixSignals(); - /// Launch the process specified in launch_info. The monitoring callback in - /// launch_info must be set, and it will be called when the process - /// terminates. static Status LaunchProcess(ProcessLaunchInfo &launch_info); //------------------------------------------------------------------ diff --git a/lldb/include/lldb/Host/MonitoringProcessLauncher.h b/lldb/include/lldb/Host/MonitoringProcessLauncher.h index 341284800a4e..9ad36e90a779 100644 --- a/lldb/include/lldb/Host/MonitoringProcessLauncher.h +++ b/lldb/include/lldb/Host/MonitoringProcessLauncher.h @@ -24,9 +24,6 @@ public: explicit MonitoringProcessLauncher( std::unique_ptr delegate_launcher); - /// Launch the process specified in launch_info. The monitoring callback in - /// launch_info must be set, and it will be called when the process - /// terminates. HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Status &error) override; diff --git a/lldb/include/lldb/Target/ProcessLaunchInfo.h b/lldb/include/lldb/Target/ProcessLaunchInfo.h index 92c517a3e460..fc7cdea04dfe 100644 --- a/lldb/include/lldb/Target/ProcessLaunchInfo.h +++ b/lldb/include/lldb/Target/ProcessLaunchInfo.h @@ -107,12 +107,6 @@ public: 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; } // If the LaunchInfo has a monitor callback, then arrange to monitor the diff --git a/lldb/source/Host/common/MonitoringProcessLauncher.cpp b/lldb/source/Host/common/MonitoringProcessLauncher.cpp index 76c11454f573..efc10004b5fd 100644 --- a/lldb/source/Host/common/MonitoringProcessLauncher.cpp +++ b/lldb/source/Host/common/MonitoringProcessLauncher.cpp @@ -9,6 +9,7 @@ #include "lldb/Host/MonitoringProcessLauncher.h" #include "lldb/Host/HostProcess.h" +#include "lldb/Target/Process.h" #include "lldb/Target/ProcessLaunchInfo.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/Status.h" @@ -57,9 +58,18 @@ MonitoringProcessLauncher::LaunchProcess(const ProcessLaunchInfo &launch_info, if (process.GetProcessId() != LLDB_INVALID_PROCESS_ID) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS)); - assert(launch_info.GetMonitorProcessCallback()); - process.StartMonitoring(launch_info.GetMonitorProcessCallback(), - launch_info.GetMonitorSignals()); + Host::MonitorChildProcessCallback callback = + launch_info.GetMonitorProcessCallback(); + + bool monitor_signals = false; + if (callback) { + // If the ProcessLaunchInfo specified a callback, use that. + monitor_signals = launch_info.GetMonitorSignals(); + } else { + callback = Process::SetProcessExitStatus; + } + + process.StartMonitoring(callback, monitor_signals); if (log) log->PutCString("started monitoring child process."); } else { diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 3e8648f81473..cc998e6da9ff 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -13,6 +13,7 @@ #include "lldb/Host/common/NativeRegisterContext.h" #include "lldb/Host/common/NativeThreadProtocol.h" #include "lldb/Host/common/SoftwareBreakpoint.h" +#include "lldb/Target/Process.h" #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/Log.h" #include "lldb/lldb-enumerations.h" diff --git a/lldb/source/Host/macosx/Host.mm b/lldb/source/Host/macosx/Host.mm index a1b6987070fb..d87c6be915ce 100644 --- a/lldb/source/Host/macosx/Host.mm +++ b/lldb/source/Host/macosx/Host.mm @@ -57,7 +57,7 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostInfo.h" #include "lldb/Host/ThreadLauncher.h" -#include "lldb/Target/ProcessLaunchInfo.h" +#include "lldb/Target/Process.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/CleanUp.h" #include "lldb/Utility/DataBufferHeap.h" @@ -68,7 +68,6 @@ #include "lldb/Utility/NameMatches.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StructuredData.h" -#include "lldb/lldb-defines.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Errno.h" @@ -1498,9 +1497,15 @@ Status Host::LaunchProcess(ProcessLaunchInfo &launch_info) { launch_info.SetProcessID(pid); // Make sure we reap any processes we spawn or we will have zombies. - bool monitoring = launch_info.MonitorProcess()); - UNUSED_IF_ASSERT_DISABLED(monitoring); - assert(monitoring); + if (!launch_info.MonitorProcess()) { + const bool monitor_signals = false; + Host::MonitorChildProcessCallback callback = nullptr; + + if (!launch_info.GetFlags().Test(lldb::eLaunchFlagDontSetExitStatus)) + callback = Process::SetProcessExitStatus; + + StartMonitoringChildProcess(callback, pid, monitor_signals); + } } else { // Invalid process ID, something didn't go well if (error.Success()) diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index eeff8f3518ae..9b2c86a5f686 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -868,12 +868,11 @@ PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger, if (IsHost()) { // We are going to hand this process off to debugserver which will be in - // 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); + // charge of setting the exit status. We still need to reap it from lldb + // but if we let the monitor thread also set the exit status, we set up a + // race between debugserver & us for who will find out about the debugged + // process's death. + launch_info.GetFlags().Set(eLaunchFlagDontSetExitStatus); process_sp = Platform::DebugProcess(launch_info, debugger, target, error); } else { if (m_remote_platform_sp) diff --git a/lldb/source/Target/ProcessLaunchInfo.cpp b/lldb/source/Target/ProcessLaunchInfo.cpp index 9569750bc5fd..60fe26bb60da 100644 --- a/lldb/source/Target/ProcessLaunchInfo.cpp +++ b/lldb/source/Target/ProcessLaunchInfo.cpp @@ -189,13 +189,6 @@ void ProcessLaunchInfo::SetMonitorProcessCallback( m_monitor_signals = monitor_signals; } -bool ProcessLaunchInfo::NoOpMonitorCallback(lldb::pid_t pid, bool exited, int signal, int status) { - Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS); - LLDB_LOG(log, "pid = {0}, exited = {1}, signal = {2}, status = {3}", pid, - exited, signal, status); - return true; -} - bool ProcessLaunchInfo::MonitorProcess() const { if (m_monitor_callback && ProcessIDIsValid()) { Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID(), diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp index 3dd5be2c6e8e..2410b6270f28 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp @@ -97,11 +97,6 @@ Expected> TestClient::launchCustom(StringRef Log, Ar Info.SetArchitecture(arch_spec); Info.SetArguments(args, true); Info.GetEnvironment() = Host::GetEnvironment(); - // 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); status = Host::LaunchProcess(Info); if (status.Fail())