https://github.com/ashgti created
https://github.com/llvm/llvm-project/pull/145621
This updates MainLoopWindows to support events for reading from a file and a
socket type.
This unifies both handle types using `WaitForMultipleEvents` which can listen
to both sockets and files for change events.
This should allow us to unify how we handle watching files/pipes/sockets on
Windows and Posix systems.
>From 21652f8b18f122027bd62759a799faccbc8c21b8 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Tue, 24 Jun 2025 17:48:54 -0700
Subject: [PATCH] [lldb] Migrating MainLoopWindows to files and sockets.
This updates MainLoopWindows to support events for reading from a file and a
socket type.
This unifies both handle types using `WaitForMultipleEvents` which can listen
to both sockets and files for change events.
This should allow us to unify how we handle watching files/pipes/sockets on
Windows and Posix systems.
---
lldb/include/lldb/Host/JSONTransport.h | 13 +--
.../lldb/Host/windows/MainLoopWindows.h | 3 +-
lldb/include/lldb/Utility/IOObject.h | 7 +-
lldb/source/Host/common/File.cpp | 4 +
lldb/source/Host/common/JSONTransport.cpp | 36 ++----
lldb/source/Host/common/Socket.cpp | 13 ++-
.../posix/ConnectionFileDescriptorPosix.cpp | 13 ++-
lldb/source/Host/windows/MainLoopWindows.cpp | 108 ++++++++++--------
lldb/source/Utility/IOObject.cpp | 8 ++
lldb/tools/lldb-dap/DAP.cpp | 2 +-
lldb/unittests/Host/FileTest.cpp | 2 +-
11 files changed, 112 insertions(+), 97 deletions(-)
diff --git a/lldb/include/lldb/Host/JSONTransport.h
b/lldb/include/lldb/Host/JSONTransport.h
index 4087cdf2b42f7..348431ed662ed 100644
--- a/lldb/include/lldb/Host/JSONTransport.h
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -85,8 +85,8 @@ class JSONTransport {
/// Reads the next message from the input stream.
template <typename T>
- llvm::Expected<T> Read(const std::chrono::microseconds &timeout) {
- llvm::Expected<std::string> message = ReadImpl(timeout);
+ llvm::Expected<T> Read() {
+ llvm::Expected<std::string> message = ReadImpl();
if (!message)
return message.takeError();
return llvm::json::parse<T>(/*JSON=*/*message);
@@ -96,8 +96,7 @@ class JSONTransport {
virtual void Log(llvm::StringRef message);
virtual llvm::Error WriteImpl(const std::string &message) = 0;
- virtual llvm::Expected<std::string>
- ReadImpl(const std::chrono::microseconds &timeout) = 0;
+ virtual llvm::Expected<std::string> ReadImpl() = 0;
lldb::IOObjectSP m_input;
lldb::IOObjectSP m_output;
@@ -112,8 +111,7 @@ class HTTPDelimitedJSONTransport : public JSONTransport {
protected:
virtual llvm::Error WriteImpl(const std::string &message) override;
- virtual llvm::Expected<std::string>
- ReadImpl(const std::chrono::microseconds &timeout) override;
+ virtual llvm::Expected<std::string> ReadImpl() override;
// FIXME: Support any header.
static constexpr llvm::StringLiteral kHeaderContentLength =
@@ -130,8 +128,7 @@ class JSONRPCTransport : public JSONTransport {
protected:
virtual llvm::Error WriteImpl(const std::string &message) override;
- virtual llvm::Expected<std::string>
- ReadImpl(const std::chrono::microseconds &timeout) override;
+ virtual llvm::Expected<std::string> ReadImpl() override;
static constexpr llvm::StringLiteral kMessageSeparator = "\n";
};
diff --git a/lldb/include/lldb/Host/windows/MainLoopWindows.h
b/lldb/include/lldb/Host/windows/MainLoopWindows.h
index 3937a24645d95..d47bf7d5f50b7 100644
--- a/lldb/include/lldb/Host/windows/MainLoopWindows.h
+++ b/lldb/include/lldb/Host/windows/MainLoopWindows.h
@@ -37,11 +37,10 @@ class MainLoopWindows : public MainLoopBase {
void Interrupt() override;
private:
- void ProcessReadObject(IOObject::WaitableHandle handle);
llvm::Expected<size_t> Poll();
struct FdInfo {
- void *event;
+ const lldb::IOObjectSP &object_sp;
Callback callback;
};
llvm::DenseMap<IOObject::WaitableHandle, FdInfo> m_read_fds;
diff --git a/lldb/include/lldb/Utility/IOObject.h
b/lldb/include/lldb/Utility/IOObject.h
index 8cf42992e7be5..de6532a637083 100644
--- a/lldb/include/lldb/Utility/IOObject.h
+++ b/lldb/include/lldb/Utility/IOObject.h
@@ -14,6 +14,7 @@
#include <sys/types.h>
#include "lldb/lldb-private.h"
+#include "lldb/lldb-types.h"
namespace lldb_private {
@@ -24,9 +25,9 @@ class IOObject {
eFDTypeSocket, // Socket requiring send/recv
};
- // TODO: On Windows this should be a HANDLE, and wait should use
- // WaitForMultipleObjects
- typedef int WaitableHandle;
+ // A handle for integrating with the host event loop model.
+ using WaitableHandle = lldb::file_t;
+
static const WaitableHandle kInvalidHandleValue;
IOObject(FDType type) : m_fd_type(type) {}
diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp
index 9aa95ffda44cb..23b6dc9fe850d 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -274,7 +274,11 @@ int NativeFile::GetDescriptor() const {
}
IOObject::WaitableHandle NativeFile::GetWaitableHandle() {
+#ifdef _WIN32
+ return (HANDLE)_get_osfhandle(GetDescriptor());
+#else
return GetDescriptor();
+#endif
}
FILE *NativeFile::GetStream() {
diff --git a/lldb/source/Host/common/JSONTransport.cpp
b/lldb/source/Host/common/JSONTransport.cpp
index 1a0851d5c4365..b5e271f61e17a 100644
--- a/lldb/source/Host/common/JSONTransport.cpp
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -28,31 +28,10 @@ using namespace lldb_private;
/// ReadFull attempts to read the specified number of bytes. If EOF is
/// encountered, an empty string is returned.
static Expected<std::string>
-ReadFull(IOObject &descriptor, size_t length,
- std::optional<std::chrono::microseconds> timeout = std::nullopt) {
+ReadFull(IOObject &descriptor, size_t length) {
if (!descriptor.IsValid())
return llvm::make_error<TransportInvalidError>();
- bool timeout_supported = true;
- // FIXME: SelectHelper does not work with NativeFile on Win32.
-#if _WIN32
- timeout_supported = descriptor.GetFdType() == IOObject::eFDTypeSocket;
-#endif
-
- if (timeout && timeout_supported) {
- SelectHelper sh;
- sh.SetTimeout(*timeout);
- sh.FDSetRead(descriptor.GetWaitableHandle());
- Status status = sh.Select();
- if (status.Fail()) {
- // Convert timeouts into a specific error.
- if (status.GetType() == lldb::eErrorTypePOSIX &&
- status.GetError() == ETIMEDOUT)
- return make_error<TransportTimeoutError>();
- return status.takeError();
- }
- }
-
std::string data;
data.resize(length);
Status status = descriptor.Read(data.data(), length);
@@ -68,13 +47,12 @@ ReadFull(IOObject &descriptor, size_t length,
}
static Expected<std::string>
-ReadUntil(IOObject &descriptor, StringRef delimiter,
- std::optional<std::chrono::microseconds> timeout = std::nullopt) {
+ReadUntil(IOObject &descriptor, StringRef delimiter) {
std::string buffer;
buffer.reserve(delimiter.size() + 1);
while (!llvm::StringRef(buffer).ends_with(delimiter)) {
Expected<std::string> next =
- ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1, timeout);
+ ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1);
if (auto Err = next.takeError())
return std::move(Err);
buffer += *next;
@@ -90,13 +68,13 @@ void JSONTransport::Log(llvm::StringRef message) {
}
Expected<std::string>
-HTTPDelimitedJSONTransport::ReadImpl(const std::chrono::microseconds &timeout)
{
+HTTPDelimitedJSONTransport::ReadImpl() {
if (!m_input || !m_input->IsValid())
return llvm::make_error<TransportInvalidError>();
IOObject *input = m_input.get();
Expected<std::string> message_header =
- ReadFull(*input, kHeaderContentLength.size(), timeout);
+ ReadFull(*input, kHeaderContentLength.size());
if (!message_header)
return message_header.takeError();
if (*message_header != kHeaderContentLength)
@@ -143,13 +121,13 @@ Error HTTPDelimitedJSONTransport::WriteImpl(const
std::string &message) {
}
Expected<std::string>
-JSONRPCTransport::ReadImpl(const std::chrono::microseconds &timeout) {
+JSONRPCTransport::ReadImpl() {
if (!m_input || !m_input->IsValid())
return make_error<TransportInvalidError>();
IOObject *input = m_input.get();
Expected<std::string> raw_json =
- ReadUntil(*input, kMessageSeparator, timeout);
+ ReadUntil(*input, kMessageSeparator);
if (!raw_json)
return raw_json.takeError();
diff --git a/lldb/source/Host/common/Socket.cpp
b/lldb/source/Host/common/Socket.cpp
index 2b23fd1e6e57e..10c28a5456f89 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -313,8 +313,19 @@ Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
}
IOObject::WaitableHandle Socket::GetWaitableHandle() {
- // TODO: On Windows, use WSAEventSelect
+#ifdef _WIN32
+ if (m_socket == INVALID_SOCKET)
+ return IOObject::kInvalidHandleValue;
+
+ WSAEVENT event = WSACreateEvent();
+ assert(event != WSA_INVALID_EVENT);
+ if (WSAEventSelect(m_socket, event, FD_ACCEPT | FD_READ | FD_WRITE) != 0)
+ return IOObject::kInvalidHandleValue;
+
+ return event;
+#else
return m_socket;
+#endif
}
Status Socket::Read(void *buf, size_t &num_bytes) {
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 57dce44812c89..86a23b4f647cf 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -276,7 +276,7 @@ size_t ConnectionFileDescriptor::Read(void *dst, size_t
dst_len,
"%p ConnectionFileDescriptor::Read() fd = %" PRIu64
", dst = %p, dst_len = %" PRIu64 ") => %" PRIu64 ", error = %s",
static_cast<void *>(this),
- static_cast<uint64_t>(m_io_sp->GetWaitableHandle()),
+ static_cast<file_t>(m_io_sp->GetWaitableHandle()),
static_cast<void *>(dst), static_cast<uint64_t>(dst_len),
static_cast<uint64_t>(bytes_read), error.AsCString());
}
@@ -380,7 +380,7 @@ size_t ConnectionFileDescriptor::Write(const void *src,
size_t src_len,
"%p ConnectionFileDescriptor::Write(fd = %" PRIu64
", src = %p, src_len = %" PRIu64 ") => %" PRIu64 " (error = %s)",
static_cast<void *>(this),
- static_cast<uint64_t>(m_io_sp->GetWaitableHandle()),
+ static_cast<file_t>(m_io_sp->GetWaitableHandle()),
static_cast<const void *>(src), static_cast<uint64_t>(src_len),
static_cast<uint64_t>(bytes_sent), error.AsCString());
}
@@ -451,14 +451,17 @@ ConnectionFileDescriptor::BytesAvailable(const
Timeout<std::micro> &timeout,
if (timeout)
select_helper.SetTimeout(*timeout);
- select_helper.FDSetRead(handle);
+ // FIXME: Migrate to MainLoop.
#if defined(_WIN32)
+ if (const auto *sock = static_cast<Socket*>(m_io_sp.get()))
+ select_helper.FDSetRead((socket_t)sock->GetNativeSocket());
// select() won't accept pipes on Windows. The entire Windows codepath
// needs to be converted over to using WaitForMultipleObjects and event
// HANDLEs, but for now at least this will allow ::select() to not return
// an error.
const bool have_pipe_fd = false;
#else
+ select_helper.FDSetRead(handle);
const bool have_pipe_fd = pipe_fd >= 0;
#endif
if (have_pipe_fd)
@@ -493,7 +496,11 @@ ConnectionFileDescriptor::BytesAvailable(const
Timeout<std::micro> &timeout,
break; // Lets keep reading to until we timeout
}
} else {
+#if defined(_WIN32)
+ if (const auto *sock = static_cast<Socket*>(m_io_sp.get());
select_helper.FDIsSetRead(sock->GetNativeSocket()))
+#else
if (select_helper.FDIsSetRead(handle))
+#endif
return eConnectionStatusSuccess;
if (select_helper.FDIsSetRead(pipe_fd)) {
diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp
b/lldb/source/Host/windows/MainLoopWindows.cpp
index f3ab2a710cd01..78e392eea255f 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -8,8 +8,11 @@
#include "lldb/Host/windows/MainLoopWindows.h"
#include "lldb/Host/Config.h"
+#include "lldb/Host/Socket.h"
+#include "llvm/Support/Casting.h"
#include "lldb/Utility/Status.h"
#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/WindowsError.h"
#include <algorithm>
#include <cassert>
#include <cerrno>
@@ -25,12 +28,41 @@ static DWORD
ToTimeout(std::optional<MainLoopWindows::TimePoint> point) {
using namespace std::chrono;
if (!point)
- return WSA_INFINITE;
+ return INFINITE;
nanoseconds dur = (std::max)(*point - steady_clock::now(), nanoseconds(0));
return ceil<milliseconds>(dur).count();
}
+static void CloseSocketWaitHandle(IOObject::WaitableHandle handle) {
+ BOOL result = WSACloseEvent(handle);
+ assert(result == TRUE);
+ UNUSED_IF_ASSERT_DISABLED(result);
+}
+
+static bool BytesAvailable(IOObject::WaitableHandle handle, const IOObjectSP
&object) {
+ DWORD available_bytes = 0;
+ switch (object->GetFdType()) {
+ case IOObject::eFDTypeFile:
+ return !PeekNamedPipe(handle, NULL, 0, NULL, &available_bytes, NULL) ||
+ available_bytes > 0;
+ case IOObject::eFDTypeSocket:
+ auto sock = static_cast<const Socket *>(object.get());
+ if (sock == nullptr)
+ return false;
+ WSANETWORKEVENTS events;
+ if (WSAEnumNetworkEvents(sock->GetNativeSocket(), (WSAEVENT)handle,
&events) != 0)
+ return false;
+ if (events.lNetworkEvents & FD_CLOSE || events.lNetworkEvents & FD_ACCEPT)
{
+ available_bytes = 1;
+ } else if (events.lNetworkEvents & FD_READ) {
+ ioctlsocket(sock->GetNativeSocket(), FIONREAD,
&available_bytes);
+ }
+ return available_bytes > 0;
+ }
+ llvm_unreachable();
+}
+
MainLoopWindows::MainLoopWindows() {
m_interrupt_event = WSACreateEvent();
assert(m_interrupt_event != WSA_INVALID_EVENT);
@@ -38,42 +70,33 @@ MainLoopWindows::MainLoopWindows() {
MainLoopWindows::~MainLoopWindows() {
assert(m_read_fds.empty());
- BOOL result = WSACloseEvent(m_interrupt_event);
- assert(result == TRUE);
- UNUSED_IF_ASSERT_DISABLED(result);
+ CloseSocketWaitHandle(m_interrupt_event);
}
llvm::Expected<size_t> MainLoopWindows::Poll() {
- std::vector<WSAEVENT> events;
+ std::vector<HANDLE> events;
events.reserve(m_read_fds.size() + 1);
- for (auto &[fd, info] : m_read_fds) {
- int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT |
FD_CLOSE);
- assert(result == 0);
- UNUSED_IF_ASSERT_DISABLED(result);
-
- events.push_back(info.event);
+ for (auto &[fd, _] : m_read_fds) {
+ events.push_back(fd);
}
events.push_back(m_interrupt_event);
DWORD result =
- WSAWaitForMultipleEvents(events.size(), events.data(), FALSE,
- ToTimeout(GetNextWakeupTime()), FALSE);
-
- for (auto &fd : m_read_fds) {
- int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
- assert(result == 0);
- UNUSED_IF_ASSERT_DISABLED(result);
- }
-
- if (result >= WSA_WAIT_EVENT_0 && result < WSA_WAIT_EVENT_0 + events.size())
- return result - WSA_WAIT_EVENT_0;
+ WaitForMultipleObjects(events.size(), events.data(), FALSE,
+ ToTimeout(GetNextWakeupTime()));
// A timeout is treated as a (premature) signalization of the interrupt
event.
- if (result == WSA_WAIT_TIMEOUT)
+ if (result == WAIT_TIMEOUT)
return events.size() - 1;
+ if (result == WAIT_FAILED)
+ return llvm::createStringError(llvm::mapLastWindowsError(),
"WaitForMultipleObjects failed");
+
+ if (result >= WAIT_OBJECT_0 && result < WAIT_OBJECT_0 + events.size())
+ return result - WAIT_OBJECT_0;
+
return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "WSAWaitForMultipleEvents failed");
+ "WaitForMultipleEvents failed");
}
MainLoopWindows::ReadHandleUP
@@ -83,28 +106,19 @@ MainLoopWindows::RegisterReadObject(const IOObjectSP
&object_sp,
error = Status::FromErrorString("IO object is not valid.");
return nullptr;
}
- if (object_sp->GetFdType() != IOObject::eFDTypeSocket) {
- error = Status::FromErrorString(
- "MainLoopWindows: non-socket types unsupported on Windows");
- return nullptr;
- }
- WSAEVENT event = WSACreateEvent();
- if (event == WSA_INVALID_EVENT) {
- error =
- Status::FromErrorStringWithFormat("Cannot create monitoring event.");
- return nullptr;
- }
+ IOObject::WaitableHandle waitable_handle = object_sp->GetWaitableHandle();
const bool inserted =
m_read_fds
- .try_emplace(object_sp->GetWaitableHandle(), FdInfo{event, callback})
+ .try_emplace(waitable_handle, FdInfo{object_sp, callback})
.second;
if (!inserted) {
- WSACloseEvent(event);
+ if (object_sp->GetFdType() == IOObject::eFDTypeSocket)
+ CloseSocketWaitHandle(waitable_handle);
error = Status::FromErrorStringWithFormat(
"File descriptor %d already monitored.",
- object_sp->GetWaitableHandle());
+ waitable_handle);
return nullptr;
}
@@ -114,18 +128,11 @@ MainLoopWindows::RegisterReadObject(const IOObjectSP
&object_sp,
void MainLoopWindows::UnregisterReadObject(IOObject::WaitableHandle handle) {
auto it = m_read_fds.find(handle);
assert(it != m_read_fds.end());
- BOOL result = WSACloseEvent(it->second.event);
- assert(result == TRUE);
- UNUSED_IF_ASSERT_DISABLED(result);
+ if (it->second.object_sp->GetFdType() == IOObject::eFDTypeSocket)
+ CloseSocketWaitHandle(handle);
m_read_fds.erase(it);
}
-void MainLoopWindows::ProcessReadObject(IOObject::WaitableHandle handle) {
- auto it = m_read_fds.find(handle);
- if (it != m_read_fds.end())
- it->second.callback(*this); // Do the work
-}
-
Status MainLoopWindows::Run() {
m_terminate_request = false;
@@ -138,8 +145,11 @@ Status MainLoopWindows::Run() {
if (*signaled_event < m_read_fds.size()) {
auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
- WSAResetEvent(KV.second.event);
- ProcessReadObject(KV.first);
+ if (BytesAvailable(KV.first, KV.second.object_sp)) {
+ if (KV.second.object_sp->GetFdType() == IOObject::eFDTypeSocket)
+ WSAResetEvent(KV.first);
+ KV.second.callback(*this); // Do the work.
+ }
} else {
assert(*signaled_event == m_read_fds.size());
WSAResetEvent(m_interrupt_event);
diff --git a/lldb/source/Utility/IOObject.cpp b/lldb/source/Utility/IOObject.cpp
index 964edce0ce10d..69065182715c2 100644
--- a/lldb/source/Utility/IOObject.cpp
+++ b/lldb/source/Utility/IOObject.cpp
@@ -8,7 +8,15 @@
#include "lldb/Utility/IOObject.h"
+#ifdef _WIN32
+#include "lldb/Host/windows/windows.h"
+#endif
+
using namespace lldb_private;
+#ifdef _WIN32
+const IOObject::WaitableHandle IOObject::kInvalidHandleValue =
INVALID_HANDLE_VALUE;
+#else
const IOObject::WaitableHandle IOObject::kInvalidHandleValue = -1;
+#endif
IOObject::~IOObject() = default;
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index c171b55951cb5..7d388436bb1d6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -881,7 +881,7 @@ llvm::Error DAP::Loop() {
while (!disconnecting) {
llvm::Expected<Message> next =
- transport.Read<protocol::Message>(std::chrono::seconds(1));
+ transport.Read<protocol::Message>();
if (next.errorIsA<TransportEOFError>()) {
consumeError(next.takeError());
break;
diff --git a/lldb/unittests/Host/FileTest.cpp b/lldb/unittests/Host/FileTest.cpp
index 35c87bb200fad..529e6a5e12abe 100644
--- a/lldb/unittests/Host/FileTest.cpp
+++ b/lldb/unittests/Host/FileTest.cpp
@@ -32,7 +32,7 @@ TEST(File, GetWaitableHandleFileno) {
ASSERT_TRUE(stream);
NativeFile file(stream, true);
- EXPECT_EQ(file.GetWaitableHandle(), fd);
+ EXPECT_EQ(file.GetWaitableHandle(), (file_t)fd);
}
TEST(File, GetStreamFromDescriptor) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
