labath created this revision.
labath added reviewers: clayborg, zturner, emaste, krytarowski.
labath added a subscriber: lldb-commits.
This replaces the C-style "void *" baton of the child process monitoring
functions with a more
C++-like API taking a std::function. The motivation for this was that it was
very difficult to
handle the ownership of the object passed into the callback function -- each
caller ended up
implementing his own way of doing it, some doing it better than others. With
the new API, one can
just pass a smart pointer into the callback and all of the lifetime management
will be handled
automatically.
This has enabled me to simplify the rather complicated handshake in
Host::RunShellCommand. I have
left handling of MonitorDebugServerProcess (my original motivation for this
change) to a separate
commit to reduce the scope of this change.
http://reviews.llvm.org/D20106
Files:
include/lldb/Host/Host.h
include/lldb/Host/HostNativeProcessBase.h
include/lldb/Host/HostProcess.h
include/lldb/Host/posix/HostProcessPosix.h
include/lldb/Target/Process.h
include/lldb/Target/ProcessLaunchInfo.h
source/Host/common/Host.cpp
source/Host/common/HostProcess.cpp
source/Host/common/MonitoringProcessLauncher.cpp
source/Host/posix/HostProcessPosix.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
source/Target/ProcessLaunchInfo.cpp
Index: source/Target/ProcessLaunchInfo.cpp
===================================================================
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -244,12 +244,9 @@
}
void
-ProcessLaunchInfo::SetMonitorProcessCallback (Host::MonitorChildProcessCallback callback,
- void *baton,
- bool monitor_signals)
+ProcessLaunchInfo::SetMonitorProcessCallback(const Host::MonitorChildProcessCallback &callback, bool monitor_signals)
{
m_monitor_callback = callback;
- m_monitor_callback_baton = baton;
m_monitor_signals = monitor_signals;
}
@@ -259,7 +256,6 @@
if (m_monitor_callback && ProcessIDIsValid())
{
Host::StartMonitoringChildProcess (m_monitor_callback,
- m_monitor_callback_baton,
GetProcessID(),
m_monitor_signals);
return true;
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1512,21 +1512,15 @@
// found in the global target list (we want to be completely sure that the
// lldb_private::Process doesn't go away before we can deliver the signal.
bool
-Process::SetProcessExitStatus (void *callback_baton,
- lldb::pid_t pid,
- bool exited,
- int signo, // Zero for no signal
- int exit_status // Exit value of process if signal is zero
-)
+Process::SetProcessExitStatus(lldb::pid_t pid, bool exited,
+ int signo, // Zero for no signal
+ int exit_status // Exit value of process if signal is zero
+ )
{
Log *log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PROCESS));
if (log)
- log->Printf ("Process::SetProcessExitStatus (baton=%p, pid=%" PRIu64 ", exited=%i, signal=%i, exit_status=%i)\n",
- callback_baton,
- pid,
- exited,
- signo,
- exit_status);
+ log->Printf("Process::SetProcessExitStatus (pid=%" PRIu64 ", exited=%i, signal=%i, exit_status=%i)\n", pid,
+ exited, signo, exit_status);
if (exited)
{
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -405,11 +405,7 @@
AsyncThread (void *arg);
static bool
- MonitorDebugserverProcess (void *callback_baton,
- lldb::pid_t pid,
- bool exited,
- int signo,
- int exit_status);
+ MonitorDebugserverProcess(ProcessGDBRemote *process, lldb::pid_t pid, bool exited, int signo, int exit_status);
lldb::StateType
SetThreadStopInfo (StringExtractor& stop_packet);
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3584,6 +3584,8 @@
Error
ProcessGDBRemote::LaunchAndConnectToDebugserver (const ProcessInfo &process_info)
{
+ using namespace std::placeholders; // For _1, _2, etc.
+
Error error;
if (m_debugserver_pid == LLDB_INVALID_PROCESS_ID)
{
@@ -3595,7 +3597,8 @@
// special terminal key sequences (^C) don't affect debugserver.
debugserver_launch_info.SetLaunchInSeparateProcessGroup(true);
- debugserver_launch_info.SetMonitorProcessCallback (MonitorDebugserverProcess, this, false);
+ debugserver_launch_info.SetMonitorProcessCallback(std::bind(MonitorDebugserverProcess, this, _1, _2, _3, _4),
+ false);
debugserver_launch_info.SetUserID(process_info.GetUserID());
#if defined (__APPLE__) && (defined (__arm__) || defined (__arm64__) || defined (__aarch64__))
@@ -3657,14 +3660,11 @@
}
bool
-ProcessGDBRemote::MonitorDebugserverProcess
-(
- void *callback_baton,
- 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
-)
+ProcessGDBRemote::MonitorDebugserverProcess(ProcessGDBRemote *process, 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
+ )
{
// The baton is a "ProcessGDBRemote *". Now this class might be gone
// and might not exist anymore, so we need to carefully try to get the
@@ -3680,15 +3680,14 @@
// debugserver that we are tracking...
Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
- ProcessGDBRemote *process = (ProcessGDBRemote *)callback_baton;
-
// Get a shared pointer to the target that has a matching process pointer.
// This target could be gone, or the target could already have a new process
// object inside of it
TargetSP target_sp (Debugger::FindTargetWithProcess(process));
if (log)
- log->Printf ("ProcessGDBRemote::MonitorDebugserverProcess (baton=%p, pid=%" PRIu64 ", signo=%i (0x%x), exit_status=%i)", callback_baton, debugserver_pid, signo, signo, exit_status);
+ log->Printf("ProcessGDBRemote::%s(process=%p, pid=%" PRIu64 ", signo=%i (0x%x), exit_status=%i)", __FUNCTION__,
+ process, debugserver_pid, signo, signo, exit_status);
if (target_sp)
{
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -121,13 +121,6 @@
bool
DebugserverProcessReaped (lldb::pid_t pid);
- static bool
- ReapDebugserverProcess (void *callback_baton,
- lldb::pid_t pid,
- bool exited,
- int signal,
- int status);
-
static const FileSpec&
GetDomainSocketDir();
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -124,7 +124,8 @@
// Do not run in a new session so that it can not linger after the
// platform closes.
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
- debugserver_launch_info.SetMonitorProcessCallback(ReapDebugserverProcess, this, false);
+ debugserver_launch_info.SetMonitorProcessCallback(
+ std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped, this, std::placeholders::_1), false);
std::string platform_scheme;
std::string platform_ip;
@@ -445,18 +446,7 @@
{
Mutex::Locker locker (m_spawned_pids_mutex);
FreePortForProcess(pid);
- return m_spawned_pids.erase(pid) > 0;
-}
-
-bool
-GDBRemoteCommunicationServerPlatform::ReapDebugserverProcess (void *callback_baton,
- lldb::pid_t pid,
- bool exited,
- int signal, // Zero for no signal
- int status) // Exit value of process if signal is zero
-{
- GDBRemoteCommunicationServerPlatform *server = (GDBRemoteCommunicationServerPlatform *)callback_baton;
- server->DebugserverProcessReaped (pid);
+ m_spawned_pids.erase(pid);
return true;
}
@@ -470,7 +460,9 @@
// generally be what happens since we need to reap started
// processes.
if (!m_process_launch_info.GetMonitorProcessCallback ())
- m_process_launch_info.SetMonitorProcessCallback(ReapDebugserverProcess, this, false);
+ m_process_launch_info.SetMonitorProcessCallback(
+ std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped, this, std::placeholders::_1),
+ false);
Error error = m_platform_sp->LaunchProcess (m_process_launch_info);
if (!error.Success ())
Index: source/Host/posix/HostProcessPosix.cpp
===================================================================
--- source/Host/posix/HostProcessPosix.cpp
+++ source/Host/posix/HostProcessPosix.cpp
@@ -107,7 +107,7 @@
}
HostThread
-HostProcessPosix::StartMonitoring(HostProcess::MonitorCallback callback, void *callback_baton, bool monitor_signals)
+HostProcessPosix::StartMonitoring(const HostProcess::MonitorCallback &callback, bool monitor_signals)
{
- return Host::StartMonitoringChildProcess(callback, callback_baton, m_process, monitor_signals);
+ return Host::StartMonitoringChildProcess(callback, m_process, monitor_signals);
}
Index: source/Host/common/MonitoringProcessLauncher.cpp
===================================================================
--- source/Host/common/MonitoringProcessLauncher.cpp
+++ source/Host/common/MonitoringProcessLauncher.cpp
@@ -75,20 +75,18 @@
Host::MonitorChildProcessCallback callback = launch_info.GetMonitorProcessCallback();
- void *baton = nullptr;
bool monitor_signals = false;
if (callback)
{
// If the ProcessLaunchInfo specified a callback, use that.
- baton = launch_info.GetMonitorProcessBaton();
monitor_signals = launch_info.GetMonitorSignals();
}
else
{
callback = Process::SetProcessExitStatus;
}
- process.StartMonitoring(callback, baton, monitor_signals);
+ process.StartMonitoring(callback, monitor_signals);
if (log)
log->PutCString("started monitoring child process.");
}
Index: source/Host/common/HostProcess.cpp
===================================================================
--- source/Host/common/HostProcess.cpp
+++ source/Host/common/HostProcess.cpp
@@ -49,9 +49,9 @@
}
HostThread
-HostProcess::StartMonitoring(HostProcess::MonitorCallback callback, void *callback_baton, bool monitor_signals)
+HostProcess::StartMonitoring(const HostProcess::MonitorCallback &callback, bool monitor_signals)
{
- return m_native_process->StartMonitoring(callback, callback_baton, monitor_signals);
+ return m_native_process->StartMonitoring(callback, monitor_signals);
}
HostNativeProcessBase &HostProcess::GetNativeProcess()
Index: source/Host/common/Host.cpp
===================================================================
--- source/Host/common/Host.cpp
+++ source/Host/common/Host.cpp
@@ -93,21 +93,20 @@
{
lldb::pid_t pid; // The process ID to monitor
Host::MonitorChildProcessCallback callback; // The callback function to call when "pid" exits or signals
- void *callback_baton; // The callback baton for the callback function
bool monitor_signals; // If true, call the callback when "pid" gets signaled.
};
static thread_result_t
MonitorChildProcessThreadFunction (void *arg);
HostThread
-Host::StartMonitoringChildProcess(Host::MonitorChildProcessCallback callback, void *callback_baton, lldb::pid_t pid, bool monitor_signals)
+Host::StartMonitoringChildProcess(const Host::MonitorChildProcessCallback &callback, lldb::pid_t pid,
+ bool monitor_signals)
{
MonitorInfo * info_ptr = new MonitorInfo();
info_ptr->pid = pid;
info_ptr->callback = callback;
- info_ptr->callback_baton = callback_baton;
info_ptr->monitor_signals = monitor_signals;
char thread_name[256];
@@ -184,7 +183,6 @@
MonitorInfo *info = (MonitorInfo *)arg;
const Host::MonitorChildProcessCallback callback = info->callback;
- void * const callback_baton = info->callback_baton;
const bool monitor_signals = info->monitor_signals;
assert (info->pid <= UINT32_MAX);
@@ -285,8 +283,8 @@
{
bool callback_return = false;
if (callback)
- callback_return = callback (callback_baton, wait_pid, exited, signal, exit_status);
-
+ callback_return = callback(wait_pid, exited, signal, exit_status);
+
// If our process exited, then this thread should exit
if (exited && wait_pid == abs(pid))
{
@@ -500,41 +498,30 @@
{
ShellInfo () :
process_reaped (false),
- can_delete (false),
pid (LLDB_INVALID_PROCESS_ID),
signo(-1),
status(-1)
{
}
lldb_private::Predicate<bool> process_reaped;
- lldb_private::Predicate<bool> can_delete;
lldb::pid_t pid;
int signo;
int status;
};
static bool
-MonitorShellCommand (void *callback_baton,
- 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
+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
{
- ShellInfo *shell_info = (ShellInfo *)callback_baton;
shell_info->pid = pid;
shell_info->signo = signo;
shell_info->status = status;
// 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(1, eBroadcastAlways);
- // Now wait for a handshake back from that thread running Host::RunShellCommand
- // so we know that we can delete shell_info_ptr
- shell_info->can_delete.WaitForValueEqualTo(true);
- // Sleep a bit to allow the shell_info->can_delete.SetValue() to complete...
- usleep(1000);
- // Now delete the shell info that was passed into this function
- delete shell_info;
+ shell_info->process_reaped.SetValue(true, eBroadcastAlways);
return true;
}
@@ -617,34 +604,30 @@
launch_info.AppendSuppressFileAction (STDOUT_FILENO, false, true);
launch_info.AppendSuppressFileAction (STDERR_FILENO, false, true);
}
-
- // The process monitor callback will delete the 'shell_info_ptr' below...
- std::unique_ptr<ShellInfo> shell_info_ap (new ShellInfo());
-
+
+ std::shared_ptr<ShellInfo> shell_info_sp(new ShellInfo());
const bool monitor_signals = false;
- launch_info.SetMonitorProcessCallback(MonitorShellCommand, shell_info_ap.get(), monitor_signals);
-
+ launch_info.SetMonitorProcessCallback(std::bind(MonitorShellCommand, shell_info_sp, std::placeholders::_1,
+ std::placeholders::_2, std::placeholders::_3,
+ std::placeholders::_4),
+ monitor_signals);
+
error = LaunchProcess (launch_info);
const lldb::pid_t pid = launch_info.GetProcessID();
if (error.Success() && pid == LLDB_INVALID_PROCESS_ID)
error.SetErrorString("failed to get process ID");
if (error.Success())
{
- // The process successfully launched, so we can defer ownership of
- // "shell_info" to the MonitorShellCommand callback function that will
- // get called when the process dies. We release the unique pointer as it
- // doesn't need to delete the ShellInfo anymore.
- ShellInfo *shell_info = shell_info_ap.release();
TimeValue *timeout_ptr = nullptr;
TimeValue timeout_time(TimeValue::Now());
if (timeout_sec > 0) {
timeout_time.OffsetWithSeconds(timeout_sec);
timeout_ptr = &timeout_time;
}
bool timed_out = false;
- shell_info->process_reaped.WaitForValueEqualTo(true, timeout_ptr, &timed_out);
+ shell_info_sp->process_reaped.WaitForValueEqualTo(true, timeout_ptr, &timed_out);
if (timed_out)
{
error.SetErrorString("timed out waiting for shell command to complete");
@@ -655,16 +638,16 @@
timeout_time = TimeValue::Now();
timeout_time.OffsetWithSeconds(1);
timed_out = false;
- shell_info->process_reaped.WaitForValueEqualTo(true, &timeout_time, &timed_out);
+ shell_info_sp->process_reaped.WaitForValueEqualTo(true, &timeout_time, &timed_out);
}
else
{
if (status_ptr)
- *status_ptr = shell_info->status;
-
+ *status_ptr = shell_info_sp->status;
+
if (signo_ptr)
- *signo_ptr = shell_info->signo;
-
+ *signo_ptr = shell_info_sp->signo;
+
if (command_output_ptr)
{
command_output_ptr->clear();
@@ -685,14 +668,10 @@
}
}
}
- shell_info->can_delete.SetValue(true, eBroadcastAlways);
}
if (FileSystem::GetFileExists(output_file_spec))
FileSystem::Unlink(output_file_spec);
- // Handshake with the monitor thread, or just let it know in advance that
- // it can delete "shell_info" in case we timed out and were not able to kill
- // the process...
return error;
}
Index: include/lldb/Target/ProcessLaunchInfo.h
===================================================================
--- include/lldb/Target/ProcessLaunchInfo.h
+++ include/lldb/Target/ProcessLaunchInfo.h
@@ -148,22 +148,14 @@
int32_t num_resumes);
void
- SetMonitorProcessCallback (Host::MonitorChildProcessCallback callback,
- void *baton,
- bool monitor_signals);
+ SetMonitorProcessCallback(const Host::MonitorChildProcessCallback &callback, bool monitor_signals);
Host::MonitorChildProcessCallback
GetMonitorProcessCallback() const
{
return m_monitor_callback;
}
- void *
- GetMonitorProcessBaton() const
- {
- return m_monitor_callback_baton;
- }
-
bool
GetMonitorSignals() const
{
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -999,16 +999,14 @@
/// Subclasses should call Host::StartMonitoringChildProcess ()
/// with:
/// callback = Process::SetHostProcessExitStatus
- /// callback_baton = nullptr
/// pid = Process::GetID()
/// monitor_signals = false
//------------------------------------------------------------------
static bool
- SetProcessExitStatus(void *callback_baton, // The callback baton which should be set to nullptr
- lldb::pid_t pid, // The process ID we want to monitor
+ SetProcessExitStatus(lldb::pid_t pid, // The process ID we want to monitor
bool exited,
- 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
lldb::ByteOrder
GetByteOrder () const;
Index: include/lldb/Host/posix/HostProcessPosix.h
===================================================================
--- include/lldb/Host/posix/HostProcessPosix.h
+++ include/lldb/Host/posix/HostProcessPosix.h
@@ -39,7 +39,8 @@
lldb::pid_t GetProcessId() const override;
bool IsRunning() const override;
- HostThread StartMonitoring(HostProcess::MonitorCallback callback, void *callback_baton, bool monitor_signals) override;
+ HostThread
+ StartMonitoring(const HostProcess::MonitorCallback &callback, bool monitor_signals) override;
};
} // namespace lldb_private
Index: include/lldb/Host/HostProcess.h
===================================================================
--- include/lldb/Host/HostProcess.h
+++ include/lldb/Host/HostProcess.h
@@ -10,6 +10,7 @@
#ifndef lldb_Host_HostProcess_h_
#define lldb_Host_HostProcess_h_
+#include "lldb/Host/Host.h"
#include "lldb/lldb-types.h"
//----------------------------------------------------------------------
@@ -36,8 +37,8 @@
class HostProcess
{
- public:
- typedef bool (*MonitorCallback)(void *callback_baton, lldb::pid_t process, bool exited, int signal, int status);
+public:
+ typedef Host::MonitorChildProcessCallback MonitorCallback;
HostProcess();
HostProcess(lldb::process_t process);
@@ -49,7 +50,8 @@
lldb::pid_t GetProcessId() const;
bool IsRunning() const;
- HostThread StartMonitoring(MonitorCallback callback, void *callback_baton, bool monitor_signals);
+ HostThread
+ StartMonitoring(const MonitorCallback &callback, bool monitor_signals);
HostNativeProcessBase &GetNativeProcess();
const HostNativeProcessBase &GetNativeProcess() const;
Index: include/lldb/Host/HostNativeProcessBase.h
===================================================================
--- include/lldb/Host/HostNativeProcessBase.h
+++ include/lldb/Host/HostNativeProcessBase.h
@@ -46,9 +46,10 @@
return m_process;
}
- virtual HostThread StartMonitoring(HostProcess::MonitorCallback callback, void *callback_baton, bool monitor_signals) = 0;
+ virtual HostThread
+ StartMonitoring(const HostProcess::MonitorCallback &callback, bool monitor_signals) = 0;
- protected:
+protected:
lldb::process_t m_process;
};
Index: include/lldb/Host/Host.h
===================================================================
--- include/lldb/Host/Host.h
+++ include/lldb/Host/Host.h
@@ -38,12 +38,10 @@
class Host
{
public:
-
- typedef bool (*MonitorChildProcessCallback) (void *callback_baton,
- 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<bool(lldb::pid_t pid, bool exited,
+ int signal, // Zero for no signal
+ int status)> // Exit value of process if signal is zero
+ MonitorChildProcessCallback;
//------------------------------------------------------------------
/// Start monitoring a child process.
@@ -65,10 +63,6 @@
/// A function callback to call when a child receives a signal
/// (if \a monitor_signals is true) or a child exits.
///
- /// @param[in] callback_baton
- /// A void * of user data that will be pass back when
- /// \a callback is called.
- ///
/// @param[in] pid
/// The process ID of a child process to monitor, -1 for all
/// processes.
@@ -84,8 +78,8 @@
///
/// @see static void Host::StopMonitoringChildProcess (uint32_t)
//------------------------------------------------------------------
- static HostThread StartMonitoringChildProcess(MonitorChildProcessCallback callback, void *callback_baton, lldb::pid_t pid,
- bool monitor_signals);
+ static HostThread
+ StartMonitoringChildProcess(const MonitorChildProcessCallback &callback, lldb::pid_t pid, bool monitor_signals);
enum SystemLogType
{
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
