labath created this revision.
labath added reviewers: jingham, davide.
Herald added a subscriber: emaste.
The Process class was only being referenced because of the last-ditch
effort in the process launchers to set a process exit callback in case
one isn't set already.
Although launching a process for debugging is the most important kind of
"launch" we are doing, it is by far not the only one, so assuming this
particular callback is the one to be used is not a good idea (besides
breaking layering). Instead of assuming a particular exit callback, I
change the launcher code to require the callback to be set by the user (and fix
up the two call sites which did not set the callback already).
https://reviews.llvm.org/D46395
Files:
source/Host/common/MonitoringProcessLauncher.cpp
source/Host/common/NativeProcessProtocol.cpp
source/Host/macosx/Host.mm
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
unittests/tools/lldb-server/tests/TestClient.cpp
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -112,6 +112,11 @@
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(
+ [](lldb::pid_t, bool, int, int) { return true; }, false);
status = Host::LaunchProcess(Info);
if (status.Fail())
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===================================================================
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -866,11 +866,12 @@
if (IsHost()) {
// We are going to hand this process off to debugserver which will be in
- // 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);
+ // 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(
+ [](lldb::pid_t, bool, int, int) { return true; }, monitor_signals);
process_sp = Platform::DebugProcess(launch_info, debugger, target, error);
} else {
if (m_remote_platform_sp)
Index: source/Host/macosx/Host.mm
===================================================================
--- source/Host/macosx/Host.mm
+++ source/Host/macosx/Host.mm
@@ -1497,15 +1497,9 @@
launch_info.SetProcessID(pid);
// Make sure we reap any processes we spawn or we will have zombies.
- 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);
- }
+ bool monitoring = launch_info.MonitorProcess();
+ (void)monitoring;
+ assert(monitoring);
} else {
// Invalid process ID, something didn't go well
if (error.Success())
Index: source/Host/common/NativeProcessProtocol.cpp
===================================================================
--- source/Host/common/NativeProcessProtocol.cpp
+++ source/Host/common/NativeProcessProtocol.cpp
@@ -13,7 +13,6 @@
#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"
Index: source/Host/common/MonitoringProcessLauncher.cpp
===================================================================
--- source/Host/common/MonitoringProcessLauncher.cpp
+++ source/Host/common/MonitoringProcessLauncher.cpp
@@ -9,7 +9,6 @@
#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"
@@ -58,18 +57,9 @@
if (process.GetProcessId() != LLDB_INVALID_PROCESS_ID) {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
- 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);
+ assert(launch_info.GetMonitorProcessCallback());
+ process.StartMonitoring(launch_info.GetMonitorProcessCallback(),
+ launch_info.GetMonitorSignals());
if (log)
log->PutCString("started monitoring child process.");
} else {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
