[Lldb-commits] [lldb] r243513 - [LLGS] Spawned process handling cleanup
Pavel Labath
labath at google.com
Wed Jul 29 05:33:31 PDT 2015
Author: labath
Date: Wed Jul 29 07:33:31 2015
New Revision: 243513
URL: http://llvm.org/viewvc/llvm-project?rev=243513&view=rev
Log:
[LLGS] Spawned process handling cleanup
Summary:
This commit moves the m_spawned_pids member from the common LLGS/Platform class to the plaform
specific part. This enables us to remove LLGS code, which was attempting to manage the
m_spawned_pids contents, but at the same time making sure, there is only one debugged process. If
we ever want to do multi-process debugging, we will probably want to replace this with a set of
NativeProcessProtocolSP anyway. The only functional change is that support for
qKillSpawnedProcess packet is removed from LLGS, but this was not used there anyway (we have the
k packet for that).
Reviewers: ovyalov, clayborg
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D11557
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=243513&r1=243512&r2=243513&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp Wed Jul 29 07:33:31 2015
@@ -58,8 +58,6 @@ using namespace lldb_private::process_gd
//----------------------------------------------------------------------
GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon(const char *comm_name, const char *listener_name) :
GDBRemoteCommunicationServer (comm_name, listener_name),
- m_spawned_pids (),
- m_spawned_pids_mutex (Mutex::eMutexTypeRecursive),
m_process_launch_info (),
m_process_launch_error (),
m_proc_infos (),
@@ -79,8 +77,6 @@ GDBRemoteCommunicationServerCommon::GDBR
&GDBRemoteCommunicationServerCommon::Handle_qGroupName);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qHostInfo,
&GDBRemoteCommunicationServerCommon::Handle_qHostInfo);
- RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qKillSpawnedProcess,
- &GDBRemoteCommunicationServerCommon::Handle_qKillSpawnedProcess);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_QLaunchArch,
&GDBRemoteCommunicationServerCommon::Handle_QLaunchArch);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qLaunchSuccess,
@@ -485,94 +481,6 @@ GDBRemoteCommunicationServerCommon::Hand
}
GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerCommon::Handle_qKillSpawnedProcess (StringExtractorGDBRemote &packet)
-{
- packet.SetFilePos(::strlen ("qKillSpawnedProcess:"));
-
- lldb::pid_t pid = packet.GetU64(LLDB_INVALID_PROCESS_ID);
-
- // verify that we know anything about this pid.
- // Scope for locker
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- {
- // not a pid we know about
- return SendErrorResponse (10);
- }
- }
-
- // go ahead and attempt to kill the spawned process
- if (KillSpawnedProcess (pid))
- return SendOKResponse ();
- else
- return SendErrorResponse (11);
-}
-
-bool
-GDBRemoteCommunicationServerCommon::KillSpawnedProcess (lldb::pid_t pid)
-{
- // make sure we know about this process
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- return false;
- }
-
- // first try a SIGTERM (standard kill)
- Host::Kill (pid, SIGTERM);
-
- // check if that worked
- for (size_t i=0; i<10; ++i)
- {
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- {
- // it is now killed
- return true;
- }
- }
- usleep (10000);
- }
-
- // check one more time after the final usleep
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- return true;
- }
-
- // the launched process still lives. Now try killing it again,
- // this time with an unblockable signal.
- Host::Kill (pid, SIGKILL);
-
- for (size_t i=0; i<10; ++i)
- {
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- {
- // it is now killed
- return true;
- }
- }
- usleep (10000);
- }
-
- // check one more time after the final usleep
- // Scope for locker
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- return true;
- }
-
- // no luck - the process still lives
- return false;
-}
-
-GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerCommon::Handle_vFile_Open (StringExtractorGDBRemote &packet)
{
packet.SetFilePos(::strlen("vFile:open:"));
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h?rev=243513&r1=243512&r2=243513&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h Wed Jul 29 07:33:31 2015
@@ -12,7 +12,6 @@
// C Includes
// C++ Includes
-#include <set>
// Other libraries and framework includes
#include "lldb/lldb-private-forward.h"
@@ -40,8 +39,6 @@ public:
~GDBRemoteCommunicationServerCommon();
protected:
- std::set<lldb::pid_t> m_spawned_pids;
- Mutex m_spawned_pids_mutex;
ProcessLaunchInfo m_process_launch_info;
Error m_process_launch_error;
ProcessInstanceInfoList m_proc_infos;
@@ -74,9 +71,6 @@ protected:
Handle_qSpeedTest (StringExtractorGDBRemote &packet);
PacketResult
- Handle_qKillSpawnedProcess (StringExtractorGDBRemote &packet);
-
- PacketResult
Handle_vFile_Open (StringExtractorGDBRemote &packet);
PacketResult
@@ -160,9 +154,6 @@ protected:
PacketResult
Handle_QLaunchArch (StringExtractorGDBRemote &packet);
- bool
- KillSpawnedProcess (lldb::pid_t pid);
-
static void
CreateProcessInfoResponse (const ProcessInstanceInfo &proc_info,
StreamString &response);
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=243513&r1=243512&r2=243513&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Wed Jul 29 07:33:31 2015
@@ -264,17 +264,6 @@ GDBRemoteCommunicationServerLLGS::Launch
printf ("Launched '%s' as process %" PRIu64 "...\n", m_process_launch_info.GetArguments ().GetArgumentAtIndex (0), m_process_launch_info.GetProcessID ());
- // Add to list of spawned processes.
- lldb::pid_t pid;
- if ((pid = m_process_launch_info.GetProcessID ()) != LLDB_INVALID_PROCESS_ID)
- {
- // add to spawned pids
- Mutex::Locker locker (m_spawned_pids_mutex);
- // On an lldb-gdbserver, we would expect there to be only one.
- assert (m_spawned_pids.empty () && "lldb-gdbserver adding tracked process but one already existed");
- m_spawned_pids.insert (pid);
- }
-
return error;
}
@@ -287,48 +276,37 @@ GDBRemoteCommunicationServerLLGS::Attach
if (log)
log->Printf ("GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64, __FUNCTION__, pid);
- // Scope for mutex locker.
+ // Before we try to attach, make sure we aren't already monitoring something else.
+ if (m_debugged_process_sp && m_debugged_process_sp->GetID() != LLDB_INVALID_PROCESS_ID)
+ return Error("cannot attach to a process %" PRIu64 " when another process with pid %" PRIu64 " is being debugged.", pid, m_debugged_process_sp->GetID());
+
+ // Try to attach.
+ error = NativeProcessProtocol::Attach(pid, *this, m_mainloop, m_debugged_process_sp);
+ if (!error.Success ())
{
- // Before we try to attach, make sure we aren't already monitoring something else.
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (!m_spawned_pids.empty ())
- {
- error.SetErrorStringWithFormat ("cannot attach to a process %" PRIu64 " when another process with pid %" PRIu64 " is being debugged.", pid, *m_spawned_pids.begin());
- return error;
- }
+ fprintf (stderr, "%s: failed to attach to process %" PRIu64 ": %s", __FUNCTION__, pid, error.AsCString ());
+ return error;
+ }
- // Try to attach.
- error = NativeProcessProtocol::Attach(pid, *this, m_mainloop, m_debugged_process_sp);
- if (!error.Success ())
- {
- fprintf (stderr, "%s: failed to attach to process %" PRIu64 ": %s", __FUNCTION__, pid, error.AsCString ());
+ // Setup stdout/stderr mapping from inferior.
+ auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor ();
+ if (terminal_fd >= 0)
+ {
+ if (log)
+ log->Printf ("ProcessGDBRemoteCommunicationServerLLGS::%s setting inferior STDIO fd to %d", __FUNCTION__, terminal_fd);
+ error = SetSTDIOFileDescriptor (terminal_fd);
+ if (error.Fail ())
return error;
- }
-
- // Setup stdout/stderr mapping from inferior.
- auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor ();
- if (terminal_fd >= 0)
- {
- if (log)
- log->Printf ("ProcessGDBRemoteCommunicationServerLLGS::%s setting inferior STDIO fd to %d", __FUNCTION__, terminal_fd);
- error = SetSTDIOFileDescriptor (terminal_fd);
- if (error.Fail ())
- return error;
- }
- else
- {
- if (log)
- log->Printf ("ProcessGDBRemoteCommunicationServerLLGS::%s ignoring inferior STDIO since terminal fd reported as %d", __FUNCTION__, terminal_fd);
- }
-
- printf ("Attached to process %" PRIu64 "...\n", pid);
+ }
+ else
+ {
+ if (log)
+ log->Printf ("ProcessGDBRemoteCommunicationServerLLGS::%s ignoring inferior STDIO since terminal fd reported as %d", __FUNCTION__, terminal_fd);
+ }
- // Add to list of spawned processes.
- assert (m_spawned_pids.empty () && "lldb-gdbserver adding tracked process but one already existed");
- m_spawned_pids.insert (pid);
+ printf ("Attached to process %" PRIu64 "...\n", pid);
- return error;
- }
+ return error;
}
void
@@ -830,17 +808,6 @@ GDBRemoteCommunicationServerLLGS::Handle
log->Printf ("GDBRemoteCommunicationServerLLGS::%s failed to send stop notification for PID %" PRIu64 ", state: eStateExited", __FUNCTION__, process->GetID ());
}
- // Remove the process from the list of spawned pids.
- {
- Mutex::Locker locker (m_spawned_pids_mutex);
- if (m_spawned_pids.erase (process->GetID ()) < 1)
- {
- if (log)
- log->Printf ("GDBRemoteCommunicationServerLLGS::%s failed to remove PID %" PRIu64 " from the spawned pids list", __FUNCTION__, process->GetID ());
-
- }
- }
-
// Close the pipe to the inferior terminal i/o if we launched it
// and set one up.
MaybeCloseInferiorTerminalConnection ();
@@ -1119,26 +1086,6 @@ GDBRemoteCommunicationServerLLGS::Handle
return SendPacketNoLock (response.GetData(), response.GetSize());
}
-bool
-GDBRemoteCommunicationServerLLGS::DebuggedProcessReaped (lldb::pid_t pid)
-{
- // reap a process that we were debugging (but not debugserver)
- Mutex::Locker locker (m_spawned_pids_mutex);
- return m_spawned_pids.erase(pid) > 0;
-}
-
-bool
-GDBRemoteCommunicationServerLLGS::ReapDebuggedProcess (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
-{
- GDBRemoteCommunicationServerLLGS *server = (GDBRemoteCommunicationServerLLGS *)callback_baton;
- server->DebuggedProcessReaped (pid);
- return true;
-}
-
GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::Handle_k (StringExtractorGDBRemote &packet)
{
@@ -2737,9 +2684,6 @@ GDBRemoteCommunicationServerLLGS::Handle
StopSTDIOForwarding();
- // Scope for mutex locker.
- Mutex::Locker locker (m_spawned_pids_mutex);
-
// Fail if we don't have a current process.
if (!m_debugged_process_sp || (m_debugged_process_sp->GetID () == LLDB_INVALID_PROCESS_ID))
{
@@ -2748,14 +2692,6 @@ GDBRemoteCommunicationServerLLGS::Handle
return SendErrorResponse (0x15);
}
- if (m_spawned_pids.find(m_debugged_process_sp->GetID ()) == m_spawned_pids.end())
- {
- if (log)
- log->Printf ("GDBRemoteCommunicationServerLLGS::%s failed to find PID %" PRIu64 " in spawned pids list",
- __FUNCTION__, m_debugged_process_sp->GetID ());
- return SendErrorResponse (0x1);
- }
-
lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
// Consume the ';' after D.
@@ -2786,7 +2722,6 @@ GDBRemoteCommunicationServerLLGS::Handle
return SendErrorResponse (0x01);
}
- m_spawned_pids.erase (m_debugged_process_sp->GetID ());
return SendOKResponse ();
}
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h?rev=243513&r1=243512&r2=243513&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h Wed Jul 29 07:33:31 2015
@@ -269,16 +269,6 @@ protected:
FindModuleFile (const std::string& module_path, const ArchSpec& arch) override;
private:
- bool
- DebuggedProcessReaped (lldb::pid_t pid);
-
- static bool
- ReapDebuggedProcess (void *callback_baton,
- lldb::pid_t pid,
- bool exited,
- int signal,
- int status);
-
void
HandleInferiorState_Exited (NativeProcessProtocol *process);
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=243513&r1=243512&r2=243513&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp Wed Jul 29 07:33:31 2015
@@ -42,6 +42,7 @@ using namespace lldb_private::process_gd
//----------------------------------------------------------------------
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform() :
GDBRemoteCommunicationServerCommon ("gdb-remote.server", "gdb-remote.server.rx_packet"),
+ m_spawned_pids_mutex (Mutex::eMutexTypeRecursive),
m_platform_sp (Platform::GetHostPlatform ()),
m_port_map (),
m_port_offset(0)
@@ -52,6 +53,8 @@ GDBRemoteCommunicationServerPlatform::GD
&GDBRemoteCommunicationServerPlatform::Handle_qGetWorkingDir);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qLaunchGDBServer,
&GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer);
+ RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qKillSpawnedProcess,
+ &GDBRemoteCommunicationServerPlatform::Handle_qKillSpawnedProcess);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qProcessInfo,
&GDBRemoteCommunicationServerPlatform::Handle_qProcessInfo);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_QSetWorkingDir,
@@ -184,6 +187,94 @@ GDBRemoteCommunicationServerPlatform::Ha
}
GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerPlatform::Handle_qKillSpawnedProcess (StringExtractorGDBRemote &packet)
+{
+ packet.SetFilePos(::strlen ("qKillSpawnedProcess:"));
+
+ lldb::pid_t pid = packet.GetU64(LLDB_INVALID_PROCESS_ID);
+
+ // verify that we know anything about this pid.
+ // Scope for locker
+ {
+ Mutex::Locker locker (m_spawned_pids_mutex);
+ if (m_spawned_pids.find(pid) == m_spawned_pids.end())
+ {
+ // not a pid we know about
+ return SendErrorResponse (10);
+ }
+ }
+
+ // go ahead and attempt to kill the spawned process
+ if (KillSpawnedProcess (pid))
+ return SendOKResponse ();
+ else
+ return SendErrorResponse (11);
+}
+
+bool
+GDBRemoteCommunicationServerPlatform::KillSpawnedProcess (lldb::pid_t pid)
+{
+ // make sure we know about this process
+ {
+ Mutex::Locker locker (m_spawned_pids_mutex);
+ if (m_spawned_pids.find(pid) == m_spawned_pids.end())
+ return false;
+ }
+
+ // first try a SIGTERM (standard kill)
+ Host::Kill (pid, SIGTERM);
+
+ // check if that worked
+ for (size_t i=0; i<10; ++i)
+ {
+ {
+ Mutex::Locker locker (m_spawned_pids_mutex);
+ if (m_spawned_pids.find(pid) == m_spawned_pids.end())
+ {
+ // it is now killed
+ return true;
+ }
+ }
+ usleep (10000);
+ }
+
+ // check one more time after the final usleep
+ {
+ Mutex::Locker locker (m_spawned_pids_mutex);
+ if (m_spawned_pids.find(pid) == m_spawned_pids.end())
+ return true;
+ }
+
+ // the launched process still lives. Now try killing it again,
+ // this time with an unblockable signal.
+ Host::Kill (pid, SIGKILL);
+
+ for (size_t i=0; i<10; ++i)
+ {
+ {
+ Mutex::Locker locker (m_spawned_pids_mutex);
+ if (m_spawned_pids.find(pid) == m_spawned_pids.end())
+ {
+ // it is now killed
+ return true;
+ }
+ }
+ usleep (10000);
+ }
+
+ // check one more time after the final usleep
+ // Scope for locker
+ {
+ Mutex::Locker locker (m_spawned_pids_mutex);
+ if (m_spawned_pids.find(pid) == m_spawned_pids.end())
+ return true;
+ }
+
+ // no luck - the process still lives
+ return false;
+}
+
+GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerPlatform::Handle_qProcessInfo (StringExtractorGDBRemote &packet)
{
lldb::pid_t pid = m_process_launch_info.GetProcessID ();
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h?rev=243513&r1=243512&r2=243513&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h Wed Jul 29 07:33:31 2015
@@ -12,6 +12,8 @@
#include "GDBRemoteCommunicationServerCommon.h"
+#include <set>
+
namespace lldb_private {
namespace process_gdb_remote {
@@ -59,6 +61,8 @@ public:
SetPortOffset (uint16_t port_offset);
protected:
+ Mutex m_spawned_pids_mutex;
+ std::set<lldb::pid_t> m_spawned_pids;
lldb::PlatformSP m_platform_sp;
PortMap m_port_map;
@@ -68,6 +72,9 @@ protected:
Handle_qLaunchGDBServer (StringExtractorGDBRemote &packet);
PacketResult
+ Handle_qKillSpawnedProcess (StringExtractorGDBRemote &packet);
+
+ PacketResult
Handle_qProcessInfo (StringExtractorGDBRemote &packet);
PacketResult
@@ -84,6 +91,9 @@ protected:
private:
bool
+ KillSpawnedProcess (lldb::pid_t pid);
+
+ bool
DebugserverProcessReaped (lldb::pid_t pid);
static bool
More information about the lldb-commits
mailing list