https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/100670
>From 0870140c8b8d465570a696b35b59fbf55610919a Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassil...@accesssoftek.com>
Date: Thu, 25 Jul 2024 00:34:34 +0400
Subject: [PATCH 1/2] [lldb] Multithreading lldb-server works on Windows now;
fixed gdb port mapping
Removed fork(). Used threads and the common thread-safe port map for all
platform connections.
Updated lldb::FileSystem to use llvm::vfs::createPhysicalFileSystem() with an
own virtual working directory per thread.
This patch depends on #100659, #100666.
This patch fixes #97537, #90923, #56346.
lldb-server has been tested on Windows with 50 connections and 100 processes
launched simultaneously. Tested also the cross build with Linux x86_64 host and
Linux Aarch64 target.
---
lldb/include/lldb/Host/FileSystem.h | 7 +
lldb/source/Host/common/FileSystem.cpp | 8 +
lldb/source/Host/posix/PipePosix.cpp | 12 +
.../GDBRemoteCommunicationServerCommon.cpp | 15 +-
.../GDBRemoteCommunicationServerPlatform.cpp | 309 ++++++++++++------
.../GDBRemoteCommunicationServerPlatform.h | 31 +-
.../tools/lldb-server/LLDBServerUtilities.cpp | 2 +
lldb/tools/lldb-server/lldb-platform.cpp | 99 ++----
8 files changed, 300 insertions(+), 183 deletions(-)
diff --git a/lldb/include/lldb/Host/FileSystem.h
b/lldb/include/lldb/Host/FileSystem.h
index 640f3846e448c..5e25414a894d3 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -47,6 +47,12 @@ class FileSystem {
static FileSystem &Instance();
+ static void InitializePerThread() {
+ lldbassert(!InstancePerThread() && "Already initialized.");
+
InstancePerThread().emplace(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>(
+ llvm::vfs::createPhysicalFileSystem().release()));
+ }
+
template <class... T> static void Initialize(T &&...t) {
lldbassert(!InstanceImpl() && "Already initialized.");
InstanceImpl().emplace(std::forward<T>(t)...);
@@ -206,6 +212,7 @@ class FileSystem {
private:
static std::optional<FileSystem> &InstanceImpl();
+ static std::optional<FileSystem> &InstancePerThread();
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
std::unique_ptr<TildeExpressionResolver> m_tilde_resolver;
std::string m_home_directory;
diff --git a/lldb/source/Host/common/FileSystem.cpp
b/lldb/source/Host/common/FileSystem.cpp
index 5153a0a9ec513..cb76086616d6b 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -49,7 +49,15 @@ void FileSystem::Terminate() {
InstanceImpl().reset();
}
+std::optional<FileSystem> &FileSystem::InstancePerThread() {
+ static thread_local std::optional<FileSystem> t_fs;
+ return t_fs;
+}
+
std::optional<FileSystem> &FileSystem::InstanceImpl() {
+ std::optional<FileSystem> &fs = InstancePerThread();
+ if (fs)
+ return fs;
static std::optional<FileSystem> g_fs;
return g_fs;
}
diff --git a/lldb/source/Host/posix/PipePosix.cpp
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..1aa02efe86610 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -324,6 +324,18 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
bytes_read += result;
if (bytes_read == size || result == 0)
break;
+
+ // This is the workaround for the following bug in Linux multithreading
+ // select() https://bugzilla.kernel.org/show_bug.cgi?id=546
+ // ReadWithTimeout() with a non-zero timeout is used only to
+ // read the port number from the gdbserver pipe
+ // in GDBRemoteCommunication::StartDebugserverProcess().
+ // The port number may be "1024\0".."65535\0".
+ if (timeout.count() > 0 && size == 6 && bytes_read == 5 &&
+ static_cast<char *>(buf)[4] == '\0') {
+ break;
+ }
+
} else if (errno == EINTR) {
continue;
} else {
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index f9d37490e16ae..cef836e001adf 100644
---
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -646,7 +646,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Size(
packet.GetHexByteString(path);
if (!path.empty()) {
uint64_t Size;
- if (llvm::sys::fs::file_size(path, Size))
+ FileSpec file_spec(path);
+ FileSystem::Instance().Resolve(file_spec);
+ if (llvm::sys::fs::file_size(file_spec.GetPath(), Size))
return SendErrorResponse(5);
StreamString response;
response.PutChar('F');
@@ -725,7 +727,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_unlink(
packet.SetFilePos(::strlen("vFile:unlink:"));
std::string path;
packet.GetHexByteString(path);
- Status error(llvm::sys::fs::remove(path));
+ FileSpec file_spec(path);
+ FileSystem::Instance().Resolve(file_spec);
+ Status error(llvm::sys::fs::remove(file_spec.GetPath()));
StreamString response;
response.Printf("F%x,%x", error.GetError(), error.GetError());
return SendPacketNoLock(response.GetString());
@@ -744,6 +748,13 @@ GDBRemoteCommunicationServerCommon::Handle_qPlatform_shell(
// uint32_t timeout = packet.GetHexMaxU32(false, 32);
if (packet.GetChar() == ',')
packet.GetHexByteString(working_dir);
+ else {
+ auto cwd = FileSystem::Instance()
+ .GetVirtualFileSystem()
+ ->getCurrentWorkingDirectory();
+ if (cwd)
+ working_dir = *cwd;
+ }
int status, signo;
std::string output;
FileSpec working_spec(working_dir);
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 65f1cc12ba307..6e3b7b4a351e0 100644
---
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -18,13 +18,13 @@
#include <sstream>
#include <thread>
-#include "llvm/Support/FileSystem.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/Threading.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/FileAction.h"
+#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Interpreter/CommandCompletions.h"
@@ -44,8 +44,17 @@ using namespace lldb;
using namespace lldb_private::process_gdb_remote;
using namespace lldb_private;
+// Copy assignment operator to avoid copying m_mutex
+GDBRemoteCommunicationServerPlatform::PortMap &
+GDBRemoteCommunicationServerPlatform::PortMap::operator=(
+ const GDBRemoteCommunicationServerPlatform::PortMap &o) {
+ m_port_map = std::move(o.m_port_map);
+ return *this;
+}
+
GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port,
- uint16_t max_port) {
+ uint16_t max_port)
+ : m_mutex() {
assert(min_port);
for (; min_port < max_port; ++min_port)
m_port_map[min_port] = LLDB_INVALID_PROCESS_ID;
@@ -54,11 +63,13 @@
GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port,
void GDBRemoteCommunicationServerPlatform::PortMap::AllowPort(uint16_t port) {
assert(port);
// Do not modify existing mappings
+ std::lock_guard<std::mutex> guard(m_mutex);
m_port_map.insert({port, LLDB_INVALID_PROCESS_ID});
}
llvm::Expected<uint16_t>
GDBRemoteCommunicationServerPlatform::PortMap::GetNextAvailablePort() {
+ std::lock_guard<std::mutex> guard(m_mutex);
if (m_port_map.empty())
return 0; // Bind to port zero and get a port, we didn't have any
// limitations
@@ -75,6 +86,7 @@
GDBRemoteCommunicationServerPlatform::PortMap::GetNextAvailablePort() {
bool GDBRemoteCommunicationServerPlatform::PortMap::AssociatePortWithProcess(
uint16_t port, lldb::pid_t pid) {
+ std::lock_guard<std::mutex> guard(m_mutex);
auto pos = m_port_map.find(port);
if (pos != m_port_map.end()) {
pos->second = pid;
@@ -84,6 +96,7 @@ bool
GDBRemoteCommunicationServerPlatform::PortMap::AssociatePortWithProcess(
}
bool GDBRemoteCommunicationServerPlatform::PortMap::FreePort(uint16_t port) {
+ std::lock_guard<std::mutex> guard(m_mutex);
std::map<uint16_t, lldb::pid_t>::iterator pos = m_port_map.find(port);
if (pos != m_port_map.end()) {
pos->second = LLDB_INVALID_PROCESS_ID;
@@ -94,6 +107,7 @@ bool
GDBRemoteCommunicationServerPlatform::PortMap::FreePort(uint16_t port) {
bool GDBRemoteCommunicationServerPlatform::PortMap::FreePortForProcess(
lldb::pid_t pid) {
+ std::lock_guard<std::mutex> guard(m_mutex);
if (!m_port_map.empty()) {
for (auto &pair : m_port_map) {
if (pair.second == pid) {
@@ -106,15 +120,22 @@ bool
GDBRemoteCommunicationServerPlatform::PortMap::FreePortForProcess(
}
bool GDBRemoteCommunicationServerPlatform::PortMap::empty() const {
+ std::lock_guard<std::mutex> guard(m_mutex);
return m_port_map.empty();
}
+GDBRemoteCommunicationServerPlatform::PortMap
+ GDBRemoteCommunicationServerPlatform::g_port_map;
+std::set<lldb::pid_t> GDBRemoteCommunicationServerPlatform::g_spawned_pids;
+std::mutex GDBRemoteCommunicationServerPlatform::g_spawned_pids_mutex;
+
// GDBRemoteCommunicationServerPlatform constructor
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
- const Socket::SocketProtocol socket_protocol, const char *socket_scheme)
- : GDBRemoteCommunicationServerCommon(),
- m_socket_protocol(socket_protocol), m_socket_scheme(socket_scheme),
- m_spawned_pids_mutex(), m_port_map(), m_port_offset(0) {
+ const Socket::SocketProtocol socket_protocol, const char *socket_scheme,
+ const lldb_private::Args &args, uint16_t port_offset)
+ : GDBRemoteCommunicationServerCommon(), m_socket_protocol(socket_protocol),
+ m_socket_scheme(socket_scheme), m_inferior_arguments(args),
+ m_port_offset(port_offset) {
m_pending_gdb_server.pid = LLDB_INVALID_PROCESS_ID;
m_pending_gdb_server.port = 0;
@@ -159,11 +180,72 @@
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
+lldb::thread_result_t GDBRemoteCommunicationServerPlatform::ThreadProc() {
+ // We need a virtual working directory per thread.
+ FileSystem::InitializePerThread();
+
+ Log *log = GetLog(LLDBLog::Platform);
+
+ if (IsConnected()) {
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "Thread started...",
+ __FUNCTION__);
+
+ if (m_inferior_arguments.GetArgumentCount() > 0) {
+ lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+ std::optional<uint16_t> port;
+ std::string socket_name;
+ Status error = LaunchGDBServer(m_inferior_arguments,
+ "", // hostname
+ pid, port, socket_name);
+ if (error.Success())
+ SetPendingGdbServer(pid, *port, socket_name);
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+ if (GetPacketAndSendResponse(std::nullopt, error, interrupt, done) !=
+ GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ if (error.Fail()) {
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "GetPacketAndSendResponse: %s",
+ __FUNCTION__, error.AsCString());
+ }
+ }
+
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "Disconnected. Killing child processes...",
+ __FUNCTION__);
+ for (lldb::pid_t pid : m_spawned_pids)
+ KillSpawnedProcess(pid);
+
+ // Do do not wait for child processes. See comments in
+ // DebugserverProcessReaped() for details.
+
+ FileSystem::Terminate();
+
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "Thread exited.",
+ __FUNCTION__);
+
+ delete this;
+ return {};
+}
+
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
std::optional<uint16_t> &port, std::string &socket_name) {
if (!port) {
- llvm::Expected<uint16_t> available_port =
m_port_map.GetNextAvailablePort();
+ llvm::Expected<uint16_t> available_port =
g_port_map.GetNextAvailablePort();
if (available_port)
port = *available_port;
else
@@ -181,23 +263,25 @@ Status
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
if (hostname.empty())
hostname = "127.0.0.1";
- Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
- *port);
+ auto cwd = FileSystem::Instance()
+ .GetVirtualFileSystem()
+ ->getCurrentWorkingDirectory();
+ if (cwd)
+ debugserver_launch_info.SetWorkingDirectory(FileSpec(*cwd));
// 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(
-
std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
- this, std::placeholders::_1));
+ &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
std::ostringstream url;
// debugserver does not accept the URL scheme prefix.
#if !defined(__APPLE__)
url << m_socket_scheme << "://";
#endif
- uint16_t *port_ptr = &*port;
+ uint16_t child_port = *port;
+ uint16_t *port_ptr = &child_port;
if (m_socket_protocol == Socket::ProtocolTcp) {
std::string platform_uri = GetConnection()->GetURI();
std::optional<URI> parsed_uri = URI::Parse(platform_uri);
@@ -208,19 +292,44 @@ Status
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
port_ptr = nullptr;
}
+ Log *log = GetLog(LLDBLog::Platform);
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "Host %s launching debugserver with: %s...",
+ __FUNCTION__, hostname.c_str(), url.str().c_str());
+
Status error = StartDebugserverProcess(
url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args,
-1);
pid = debugserver_launch_info.GetProcessID();
+
+ if (error.Success()) {
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "debugserver launched successfully as pid %" PRIu64,
+ __FUNCTION__, pid);
+ } else {
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "debugserver launch failed: %s",
+ __FUNCTION__, error.AsCString());
+ }
+
+ // TODO: Be sure gdbserver uses the requested port.
+ // assert(!port_ptr || *port == 0 || *port == child_port)
+ // Use only the original *port returned by GetNextAvailablePort()
+ // for AssociatePortWithProcess() or FreePort() below.
+
if (pid != LLDB_INVALID_PROCESS_ID) {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- m_spawned_pids.insert(pid);
+ AddSpawnedProcess(pid);
if (*port > 0)
- m_port_map.AssociatePortWithProcess(*port, pid);
+ g_port_map.AssociatePortWithProcess(*port, pid);
} else {
if (*port > 0)
- m_port_map.FreePort(*port);
+ g_port_map.FreePort(*port);
}
+ if (port_ptr)
+ *port = child_port;
return error;
}
@@ -230,10 +339,6 @@
GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
// Spawn a local debugserver as a platform so we can then attach or launch a
// process...
- Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "GDBRemoteCommunicationServerPlatform::%s() called",
- __FUNCTION__);
-
ConnectionFileDescriptor file_conn;
std::string hostname;
packet.SetFilePos(::strlen("qLaunchGDBServer;"));
@@ -255,18 +360,9 @@
GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
Status error =
LaunchGDBServer(Args(), hostname, debugserver_pid, port, socket_name);
if (error.Fail()) {
- LLDB_LOGF(log,
- "GDBRemoteCommunicationServerPlatform::%s() debugserver "
- "launch failed: %s",
- __FUNCTION__, error.AsCString());
return SendErrorResponse(9);
}
- LLDB_LOGF(log,
- "GDBRemoteCommunicationServerPlatform::%s() debugserver "
- "launched successfully as pid %" PRIu64,
- __FUNCTION__, debugserver_pid);
-
StreamGDBRemote response;
assert(port);
response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid,
@@ -317,28 +413,45 @@
GDBRemoteCommunicationServerPlatform::Handle_qKillSpawnedProcess(
lldb::pid_t pid = packet.GetU64(LLDB_INVALID_PROCESS_ID);
+ if (SpawnedProcessFinished(pid))
+ m_spawned_pids.erase(pid);
+
// verify that we know anything about this pid. Scope for locker
- {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end()) {
- // not a pid we know about
- return SendErrorResponse(10);
- }
+ if ((m_spawned_pids.find(pid) == m_spawned_pids.end())) {
+ // not a pid we know about
+ return SendErrorResponse(10); // ECHILD
}
// go ahead and attempt to kill the spawned process
- if (KillSpawnedProcess(pid))
+ if (KillSpawnedProcess(pid)) {
+ m_spawned_pids.erase(pid);
return SendOKResponse();
- else
- return SendErrorResponse(11);
+ } else
+ return SendErrorResponse(11); // EDEADLK
+}
+
+void GDBRemoteCommunicationServerPlatform::AddSpawnedProcess(lldb::pid_t pid) {
+ std::lock_guard<std::mutex> guard(g_spawned_pids_mutex);
+
+ // If MonitorChildProcessThreadFunction() failed hope the system will not
+ // reuse pid of zombie processes.
+ // assert(g_spawned_pids.find(pid) == g_spawned_pids.end());
+
+ g_spawned_pids.insert(pid);
+ m_spawned_pids.insert(pid);
+}
+
+bool GDBRemoteCommunicationServerPlatform::SpawnedProcessFinished(
+ lldb::pid_t pid) {
+ std::lock_guard<std::mutex> guard(g_spawned_pids_mutex);
+ return (g_spawned_pids.find(pid) == g_spawned_pids.end());
}
bool GDBRemoteCommunicationServerPlatform::KillSpawnedProcess(lldb::pid_t pid)
{
// make sure we know about this process
- {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- return false;
+ if (SpawnedProcessFinished(pid)) {
+ // it seems the process has been finished recently
+ return true;
}
// first try a SIGTERM (standard kill)
@@ -346,46 +459,30 @@ bool
GDBRemoteCommunicationServerPlatform::KillSpawnedProcess(lldb::pid_t pid) {
// check if that worked
for (size_t i = 0; i < 10; ++i) {
- {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end()) {
- // it is now killed
- return true;
- }
+ if (SpawnedProcessFinished(pid)) {
+ // it is now killed
+ return true;
}
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
- {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- return true;
- }
+ if (SpawnedProcessFinished(pid))
+ 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) {
- {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end()) {
- // it is now killed
- return true;
- }
+ if (SpawnedProcessFinished(pid)) {
+ // it is now killed
+ return true;
}
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
// check one more time after the final sleep
- {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- if (m_spawned_pids.find(pid) == m_spawned_pids.end())
- return true;
- }
-
- // no luck - the process still lives
- return false;
+ return SpawnedProcessFinished(pid);
}
GDBRemoteCommunication::PacketResult
@@ -442,12 +539,14 @@ GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerPlatform::Handle_qGetWorkingDir(
StringExtractorGDBRemote &packet) {
- llvm::SmallString<64> cwd;
- if (std::error_code ec = llvm::sys::fs::current_path(cwd))
- return SendErrorResponse(ec.value());
+ auto cwd = FileSystem::Instance()
+ .GetVirtualFileSystem()
+ ->getCurrentWorkingDirectory();
+ if (!cwd)
+ return SendErrorResponse(cwd.getError());
StreamString response;
- response.PutBytesAsRawHex8(cwd.data(), cwd.size());
+ response.PutBytesAsRawHex8(cwd->data(), cwd->size());
return SendPacketNoLock(response.GetString());
}
@@ -458,7 +557,9 @@ GDBRemoteCommunicationServerPlatform::Handle_QSetWorkingDir(
std::string path;
packet.GetHexByteString(path);
- if (std::error_code ec = llvm::sys::fs::set_current_path(path))
+ if (std::error_code ec = FileSystem::Instance()
+ .GetVirtualFileSystem()
+ ->setCurrentWorkingDirectory(path))
return SendErrorResponse(ec.value());
return SendOKResponse();
}
@@ -519,10 +620,25 @@ GDBRemoteCommunicationServerPlatform::Handle_jSignalsInfo(
}
void GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped(
- lldb::pid_t pid) {
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- m_port_map.FreePortForProcess(pid);
- m_spawned_pids.erase(pid);
+ lldb::pid_t pid, int signal, int status) {
+
+ // Note MonitoringProcessLauncher::LaunchProcess() does not store the monitor
+ // thread and we cannot control it. The child process monitor thread will
call
+ // static DebugserverProcessReaped() callback. It may happen after destroying
+ // the GDBRemoteCommunicationServerPlatform instance.
+ // HostProcessWindows::MonitorThread() calls the callback anyway when the
+ // process is finished. But MonitorChildProcessThreadFunction() in
+ // common/Host.cpp can fail and exit w/o calling the callback. So g_port_map
+ // and g_spawned_pids may leak in this case. The system must not reuse pid
+ // of zombie processes, so leaking g_spawned_pids shouldn't be a problem.
+ // But we can do nothing with g_port_map in this case.
+
+ g_port_map.FreePortForProcess(pid);
+
+ {
+ std::lock_guard<std::mutex> guard(g_spawned_pids_mutex);
+ g_spawned_pids.erase(pid);
+ }
}
Status GDBRemoteCommunicationServerPlatform::LaunchProcess() {
@@ -530,38 +646,51 @@ Status
GDBRemoteCommunicationServerPlatform::LaunchProcess() {
return Status("%s: no process command line specified to launch",
__FUNCTION__);
+ auto cwd = FileSystem::Instance()
+ .GetVirtualFileSystem()
+ ->getCurrentWorkingDirectory();
+ if (cwd)
+ m_process_launch_info.SetWorkingDirectory(FileSpec(*cwd));
+
// specify the process monitor if not already set. This should generally be
// what happens since we need to reap started processes.
if (!m_process_launch_info.GetMonitorProcessCallback())
- m_process_launch_info.SetMonitorProcessCallback(std::bind(
- &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped, this,
- std::placeholders::_1));
+ m_process_launch_info.SetMonitorProcessCallback(
+ &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
+
+ Log *log = GetLog(LLDBLog::Platform);
Status error = Host::LaunchProcess(m_process_launch_info);
if (!error.Success()) {
- fprintf(stderr, "%s: failed to launch executable %s", __FUNCTION__,
- m_process_launch_info.GetArguments().GetArgumentAtIndex(0));
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "Failed to launch executable %s: %s",
+ __FUNCTION__,
+ m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
+ error.AsCString());
return error;
}
- printf("Launched '%s' as process %" PRIu64 "...\n",
- m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
- m_process_launch_info.GetProcessID());
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "Launched '%s' as process %" PRIu64,
+ __FUNCTION__,
+ m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
+ m_process_launch_info.GetProcessID());
// add to list of spawned processes. On an lldb-gdbserver, we would expect
// there to be only one.
const auto pid = m_process_launch_info.GetProcessID();
if (pid != LLDB_INVALID_PROCESS_ID) {
// add to spawned pids
- std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
- m_spawned_pids.insert(pid);
+ AddSpawnedProcess(pid);
}
return error;
}
void GDBRemoteCommunicationServerPlatform::SetPortMap(PortMap &&port_map) {
- m_port_map = std::move(port_map);
+ g_port_map = std::move(port_map);
}
const FileSpec &GDBRemoteCommunicationServerPlatform::GetDomainSocketDir() {
@@ -594,10 +723,6 @@
GDBRemoteCommunicationServerPlatform::GetDomainSocketPath(const char *prefix) {
return FileSpec(socket_path.c_str());
}
-void GDBRemoteCommunicationServerPlatform::SetPortOffset(uint16_t port_offset)
{
- m_port_offset = port_offset;
-}
-
void GDBRemoteCommunicationServerPlatform::SetPendingGdbServer(
lldb::pid_t pid, uint16_t port, const std::string &socket_name) {
m_pending_gdb_server.pid = pid;
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
index 1853025466cf0..357ac2beba64d 100644
---
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -32,12 +32,15 @@ class GDBRemoteCommunicationServerPlatform
// communicate on.
// Construct an empty map, where empty means any port is allowed.
- PortMap() = default;
+ PortMap() : m_mutex() {};
// Make a port map with a range of free ports
// from min_port to max_port-1.
PortMap(uint16_t min_port, uint16_t max_port);
+ // Copy assignment operator to avoid copying m_mutex
+ PortMap &operator=(const PortMap &o);
+
// Add a port to the map. If it is already in the map do not modify
// its mapping. (used ports remain used, new ports start as free)
void AllowPort(uint16_t port);
@@ -70,10 +73,12 @@ class GDBRemoteCommunicationServerPlatform
private:
std::map<uint16_t, lldb::pid_t> m_port_map;
+ mutable std::mutex m_mutex;
};
GDBRemoteCommunicationServerPlatform(
- const Socket::SocketProtocol socket_protocol, const char *socket_scheme);
+ const Socket::SocketProtocol socket_protocol, const char *socket_scheme,
+ const lldb_private::Args &args, uint16_t port_offset = 0);
~GDBRemoteCommunicationServerPlatform() override;
@@ -81,11 +86,7 @@ class GDBRemoteCommunicationServerPlatform
// Set both ports to zero to let the platform automatically bind to
// a port chosen by the OS.
- void SetPortMap(PortMap &&port_map);
-
- void SetPortOffset(uint16_t port_offset);
-
- void SetInferiorArguments(const lldb_private::Args &args);
+ static void SetPortMap(PortMap &&port_map);
// Set port if you want to use a specific port number.
// Otherwise port will be set to the port that was chosen for you.
@@ -96,14 +97,18 @@ class GDBRemoteCommunicationServerPlatform
void SetPendingGdbServer(lldb::pid_t pid, uint16_t port,
const std::string &socket_name);
+ lldb::thread_result_t ThreadProc();
+
protected:
const Socket::SocketProtocol m_socket_protocol;
const std::string m_socket_scheme;
- std::recursive_mutex m_spawned_pids_mutex;
+ const lldb_private::Args m_inferior_arguments;
+ const uint16_t m_port_offset;
std::set<lldb::pid_t> m_spawned_pids;
+ static std::set<lldb::pid_t> g_spawned_pids;
+ static std::mutex g_spawned_pids_mutex;
- PortMap m_port_map;
- uint16_t m_port_offset;
+ static PortMap g_port_map;
struct {
lldb::pid_t pid;
uint16_t port;
@@ -129,9 +134,11 @@ class GDBRemoteCommunicationServerPlatform
PacketResult Handle_jSignalsInfo(StringExtractorGDBRemote &packet);
private:
- bool KillSpawnedProcess(lldb::pid_t pid);
+ void AddSpawnedProcess(lldb::pid_t pid);
+ static bool SpawnedProcessFinished(lldb::pid_t pid);
+ static bool KillSpawnedProcess(lldb::pid_t pid);
- void DebugserverProcessReaped(lldb::pid_t pid);
+ static void DebugserverProcessReaped(lldb::pid_t pid, int signal, int
status);
static const FileSpec &GetDomainSocketDir();
diff --git a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
index c3a8df19e969e..5facfbf3105e9 100644
--- a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
+++ b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler {
: m_stream_sp(stream_sp) {}
void Emit(llvm::StringRef message) override {
+ std::lock_guard<std::mutex> guard(m_mutex);
(*m_stream_sp) << message;
m_stream_sp->flush();
}
private:
+ std::mutex m_mutex;
std::shared_ptr<raw_ostream> m_stream_sp;
};
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 7148a1d2a3094..1cbadb0a01750 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -32,9 +32,11 @@
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostGetOpt.h"
#include "lldb/Host/OptionParser.h"
+#include "lldb/Host/ThreadLauncher.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/UriParser.h"
using namespace lldb;
using namespace lldb_private;
@@ -282,10 +284,10 @@ int main_platform(int argc, char *argv[]) {
}
}
- GDBRemoteCommunicationServerPlatform platform(
- acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
- if (port_offset > 0)
- platform.SetPortOffset(port_offset);
+ if (!gdbserver_portmap.empty()) {
+ GDBRemoteCommunicationServerPlatform::SetPortMap(
+ std::move(gdbserver_portmap));
+ }
do {
const bool children_inherit_accept_socket = true;
@@ -297,85 +299,28 @@ int main_platform(int argc, char *argv[]) {
}
printf("Connection established.\n");
+ GDBRemoteCommunicationServerPlatform *platform =
+ new GDBRemoteCommunicationServerPlatform(
+ acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme(),
+ inferior_arguments, port_offset);
+ platform->SetConnection(std::unique_ptr<Connection>(conn));
+
if (g_server) {
- // Collect child zombie processes.
-#if !defined(_WIN32)
- ::pid_t waitResult;
- while ((waitResult = waitpid(-1, nullptr, WNOHANG)) > 0) {
- // waitResult is the child pid
- gdbserver_portmap.FreePortForProcess(waitResult);
- }
-#endif
- // TODO: Clean up portmap for Windows when children die
- // See https://github.com/llvm/llvm-project/issues/90923
-
- // After collecting zombie ports, get the next available
- GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child;
- llvm::Expected<uint16_t> available_port =
- gdbserver_portmap.GetNextAvailablePort();
- if (available_port) {
- // GetNextAvailablePort() may return 0 if gdbserver_portmap is empty.
- if (*available_port)
- portmap_for_child.AllowPort(*available_port);
- } else {
- llvm::consumeError(available_port.takeError());
- fprintf(stderr,
- "no available gdbserver port for connection - dropping...\n");
- delete conn;
- continue;
- }
- platform.SetPortMap(std::move(portmap_for_child));
-
- auto childPid = fork();
- if (childPid) {
- gdbserver_portmap.AssociatePortWithProcess(*available_port, childPid);
- // Parent doesn't need a connection to the lldb client
- delete conn;
-
- // Parent will continue to listen for new connections.
- continue;
- } else {
- // Child process will handle the connection and exit.
- g_server = 0;
- // Listening socket is owned by parent process.
- acceptor_up.release();
+ std::optional<URI> uri = URI::Parse(conn->GetURI());
+ const std::string thread_name =
+ uri ? llvm::formatv("conn:{0}>", uri->port) : std::string("conn:?");
+ auto maybe_thread = ThreadLauncher::LaunchThread(
+ thread_name, [platform] { return platform->ThreadProc(); });
+ if (!maybe_thread) {
+ WithColor::error() << "failed to start thread: "
+ << maybe_thread.takeError() << '\n';
+ delete platform;
}
} else {
// If not running as a server, this process will not accept
// connections while a connection is active.
acceptor_up.reset();
-
- // When not running in server mode, use all available ports
- platform.SetPortMap(std::move(gdbserver_portmap));
- }
-
- platform.SetConnection(std::unique_ptr<Connection>(conn));
-
- if (platform.IsConnected()) {
- if (inferior_arguments.GetArgumentCount() > 0) {
- lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
- std::optional<uint16_t> port;
- std::string socket_name;
- Status error = platform.LaunchGDBServer(inferior_arguments,
- "", // hostname
- pid, port, socket_name);
- if (error.Success())
- platform.SetPendingGdbServer(pid, *port, socket_name);
- else
- fprintf(stderr, "failed to start gdbserver: %s\n",
error.AsCString());
- }
-
- bool interrupt = false;
- bool done = false;
- while (!interrupt && !done) {
- if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
- done) !=
- GDBRemoteCommunication::PacketResult::Success)
- break;
- }
-
- if (error.Fail())
- WithColor::error() << error.AsCString() << '\n';
+ platform->ThreadProc();
}
} while (g_server);
>From e69a04ee18adadad58ce5f8dbf99761b313984fa Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassil...@accesssoftek.com>
Date: Mon, 29 Jul 2024 20:55:21 +0400
Subject: [PATCH 2/2] Merged with #100659
---
.../Host/posix/ProcessLauncherPosixFork.cpp | 10 +++++--
.../Host/windows/ProcessLauncherWindows.cpp | 26 +++++++++++++++----
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
index 0a832ebad13a7..fd0f2a0a55f31 100644
--- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -201,7 +201,7 @@ struct ForkLaunchInfo {
execve(info.argv[0], const_cast<char *const *>(info.argv), info.envp);
#if defined(__linux__)
- if (errno == ETXTBSY) {
+ for (int i = 0; i < 50; ++i) {
// On android M and earlier we can get this error because the adb daemon
// can hold a write handle on the executable even after it has finished
// uploading it. This state lasts only a short time and happens only when
@@ -210,7 +210,13 @@ struct ForkLaunchInfo {
// shell" command in the fork() child before it has had a chance to exec.)
// Since this state should clear up quickly, wait a while and then give it
// one more go.
- usleep(50000);
+ // If `lldb-server platform` copies the executable in one thread and
+ // launches gdbserver in another thread (fork+execve), the FD may stay
+ // opened in the forked child process until execve() even if the first
+ // thread closed the file. Let's wait a while.
+ if (errno != ETXTBSY)
+ break;
+ usleep(100000);
execve(info.argv[0], const_cast<char *const *>(info.argv), info.envp);
}
#endif
diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
index baa422c15cae2..ee5f8fda1d492 100644
--- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp
+++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp
@@ -113,14 +113,30 @@ ProcessLauncherWindows::LaunchProcess(const
ProcessLaunchInfo &launch_info,
// command line is not empty, its contents may be modified by CreateProcessW.
WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : &wcommandLine[0];
- BOOL result = ::CreateProcessW(
- wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
- wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
- &startupinfo, &pi);
+ BOOL result;
+ DWORD last_error = 0;
+ // This is the workaround for the error "The process cannot access the file
+ // because it is being used by another process". Note the executable file is
+ // installed to the target by the process `lldb-server platform`, but
launched
+ // by the process `lldb-server gdbserver`. Sometimes system may block the
file
+ // for some time after copying.
+ for (int i = 0; i < 50; ++i) {
+ result = ::CreateProcessW(
+ wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
+ wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
+ &startupinfo, &pi);
+ if (!result) {
+ last_error = ::GetLastError();
+ if (last_error != ERROR_SHARING_VIOLATION)
+ break;
+ ::Sleep(100);
+ } else
+ break;
+ }
if (!result) {
// Call GetLastError before we make any other system calls.
- error.SetError(::GetLastError(), eErrorTypeWin32);
+ error.SetError(last_error, eErrorTypeWin32);
// Note that error 50 ("The request is not supported") will occur if you
// try debug a 64-bit inferior from a 32-bit LLDB.
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
