From 6e4f19d440865dd82cfce5eed1a81c6fa7050dab Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 29 Jul 2015 12:33:31 +0000 Subject: [PATCH] [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 llvm-svn: 243513 --- .../GDBRemoteCommunicationServerCommon.cpp | 92 ---------------- .../GDBRemoteCommunicationServerCommon.h | 9 -- .../GDBRemoteCommunicationServerLLGS.cpp | 117 +++++---------------- .../gdb-remote/GDBRemoteCommunicationServerLLGS.h | 10 -- .../GDBRemoteCommunicationServerPlatform.cpp | 91 ++++++++++++++++ .../GDBRemoteCommunicationServerPlatform.h | 10 ++ 6 files changed, 127 insertions(+), 202 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index 698854e..24bc72e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -58,8 +58,6 @@ using namespace lldb_private::process_gdb_remote; //---------------------------------------------------------------------- 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::GDBRemoteCommunicationServerCommon(const cha &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::Handle_qSpeedTest (StringExtractorGDBRemote } 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:")); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h index 62b129b..4eff804 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h @@ -12,7 +12,6 @@ // C Includes // C++ Includes -#include // Other libraries and framework includes #include "lldb/lldb-private-forward.h" @@ -40,8 +39,6 @@ public: ~GDBRemoteCommunicationServerCommon(); protected: - std::set 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); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index dba566e..3a7ad34 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -264,17 +264,6 @@ GDBRemoteCommunicationServerLLGS::LaunchProcess () 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::AttachToProcess (lldb::pid_t pid) 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::HandleInferiorState_Exited (NativeProcessProto 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_qC (StringExtractorGDBRemote &packet) 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_D (StringExtractorGDBRemote &packet) 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_D (StringExtractorGDBRemote &packet) 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_D (StringExtractorGDBRemote &packet) return SendErrorResponse (0x01); } - m_spawned_pids.erase (m_debugged_process_sp->GetID ()); return SendOKResponse (); } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h index 11ee2d1..fab9f81 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -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); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index bc1acee..742dee5 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -42,6 +42,7 @@ using namespace lldb_private::process_gdb_remote; //---------------------------------------------------------------------- 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::GDBRemoteCommunicationServerPlatform() : &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::Handle_qLaunchGDBServer (StringExtractorGD } 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 (); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h index 5c01137..f507a08 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -12,6 +12,8 @@ #include "GDBRemoteCommunicationServerCommon.h" +#include + 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 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 -- 2.7.4