https://github.com/ashgti updated
https://github.com/llvm/llvm-project/pull/145621
>From 97b8cf6986847bb73fa9ceb6f0c182d8c79737d4 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Wed, 25 Jun 2025 18:03:39 -0700
Subject: [PATCH] [lldb] Adding file and pipe support to
lldb_private::MainLoopWindows.
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/File.h | 2 +
lldb/include/lldb/Host/Socket.h | 2 +
.../lldb/Host/windows/MainLoopWindows.h | 3 +-
lldb/include/lldb/Utility/IOObject.h | 8 +-
lldb/source/Host/common/File.cpp | 18 +++
lldb/source/Host/common/Socket.cpp | 40 +++++-
.../posix/ConnectionFileDescriptorPosix.cpp | 14 ++-
lldb/source/Host/windows/MainLoopWindows.cpp | 114 +++++++++---------
lldb/source/Utility/IOObject.cpp | 9 ++
lldb/unittests/Host/FileTest.cpp | 16 ++-
lldb/unittests/Host/MainLoopTest.cpp | 26 ++++
11 files changed, 180 insertions(+), 72 deletions(-)
diff --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index 9e2d0abe0b1af..36cb192281289 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -127,6 +127,7 @@ class File : public IOObject {
/// \return
/// a valid handle or IOObject::kInvalidHandleValue
WaitableHandle GetWaitableHandle() override;
+ bool HasReadableData() override;
/// Get the file specification for this file, if possible.
///
@@ -400,6 +401,7 @@ class NativeFile : public File {
Status Write(const void *buf, size_t &num_bytes) override;
Status Close() override;
WaitableHandle GetWaitableHandle() override;
+ bool HasReadableData() override;
Status GetFileSpec(FileSpec &file_spec) const override;
int GetDescriptor() const override;
FILE *GetStream() override;
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 89953ee7fd5b6..6569e9e6ea818 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -158,6 +158,7 @@ class Socket : public IOObject {
bool IsValid() const override { return m_socket != kInvalidSocketValue; }
WaitableHandle GetWaitableHandle() override;
+ bool HasReadableData() override;
static llvm::Expected<HostAndPort>
DecodeHostAndPort(llvm::StringRef host_and_port);
@@ -185,6 +186,7 @@ class Socket : public IOObject {
SocketProtocol m_protocol;
NativeSocket m_socket;
+ WaitableHandle m_waitable_handle;
bool m_should_close_fd;
};
diff --git a/lldb/include/lldb/Host/windows/MainLoopWindows.h
b/lldb/include/lldb/Host/windows/MainLoopWindows.h
index 3937a24645d95..43b7d13a0e445 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;
+ 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..48a8a2076581f 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) {}
@@ -40,6 +41,7 @@ class IOObject {
FDType GetFdType() const { return m_fd_type; }
virtual WaitableHandle GetWaitableHandle() = 0;
+ virtual bool HasReadableData() = 0;
protected:
FDType m_fd_type;
diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp
index 9aa95ffda44cb..2d33f9e2028c4 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -118,6 +118,8 @@ IOObject::WaitableHandle File::GetWaitableHandle() {
return IOObject::kInvalidHandleValue;
}
+bool File::HasReadableData() { return false; }
+
Status File::GetFileSpec(FileSpec &file_spec) const {
file_spec.Clear();
return std::error_code(ENOTSUP, std::system_category());
@@ -274,7 +276,23 @@ int NativeFile::GetDescriptor() const {
}
IOObject::WaitableHandle NativeFile::GetWaitableHandle() {
+#ifdef _WIN32
+ return (HANDLE)_get_osfhandle(GetDescriptor());
+#else
return GetDescriptor();
+#endif
+}
+
+bool NativeFile::HasReadableData() {
+#ifdef _WIN32
+ DWORD available_bytes = 0;
+ return !PeekNamedPipe((HANDLE)_get_osfhandle(GetDescriptor()), NULL, 0, NULL,
+ &available_bytes, NULL) ||
+ available_bytes > 0;
+#else
+ size_t buffer_size = 0;
+ return ioctl(GetDescriptor(), FIONREAD, buffer_size) != -1 && buffer_size >
0;
+#endif
}
FILE *NativeFile::GetStream() {
diff --git a/lldb/source/Host/common/Socket.cpp
b/lldb/source/Host/common/Socket.cpp
index 2b23fd1e6e57e..d7f57095ee311 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -31,8 +31,11 @@
#include <netdb.h>
#include <netinet/in.h>
#include <netinet/tcp.h>
+#include <sys/ioctl.h>
#include <sys/socket.h>
+#include <sys/stat.h>
#include <sys/un.h>
+#include <termios.h>
#include <unistd.h>
#endif
@@ -169,7 +172,9 @@ bool Socket::FindProtocolByScheme(const char *scheme,
Socket::Socket(SocketProtocol protocol, bool should_close)
: IOObject(eFDTypeSocket), m_protocol(protocol),
- m_socket(kInvalidSocketValue), m_should_close_fd(should_close) {}
+ m_socket(kInvalidSocketValue),
+ m_waitable_handle(IOObject::kInvalidHandleValue),
+ m_should_close_fd(should_close) {}
Socket::~Socket() { Close(); }
@@ -313,8 +318,39 @@ Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
}
IOObject::WaitableHandle Socket::GetWaitableHandle() {
- // TODO: On Windows, use WSAEventSelect
+#ifdef _WIN32
+ if (m_socket == kInvalidSocketValue)
+ return kInvalidHandleValue;
+
+ if (m_waitable_handle == kInvalidHandleValue) {
+ m_waitable_handle = WSACreateEvent();
+ assert(m_waitable_handle != WSA_INVALID_EVENT);
+ if (WSAEventSelect(m_socket, m_waitable_handle,
+ FD_ACCEPT | FD_READ | FD_WRITE) != 0)
+ return kInvalidHandleValue;
+ }
+
+ return m_waitable_handle;
+#else
return m_socket;
+#endif
+}
+
+bool Socket::HasReadableData() {
+#ifdef _WIN32
+ if (!IsValid() || m_waitable_handle == kInvalidHandleValue)
+ return false;
+
+ WSANETWORKEVENTS events;
+ if (WSAEnumNetworkEvents(m_socket, m_waitable_handle, &events) != 0)
+ return false;
+
+ return events.lNetworkEvents & FD_CLOSE ||
+ events.lNetworkEvents & FD_ACCEPT || events.lNetworkEvents & FD_READ;
+#else
+ size_t buffer_size = 0;
+ return ioctl(m_socket, FIONREAD, buffer_size) != -1 && buffer_size > 0;
+#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..2fcad7f193e1a 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,12 @@ 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..c8e7e6c2749fa 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 "lldb/Utility/Status.h"
#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/WindowsError.h"
#include <algorithm>
#include <cassert>
#include <cerrno>
@@ -21,16 +24,6 @@
using namespace lldb;
using namespace lldb_private;
-static DWORD ToTimeout(std::optional<MainLoopWindows::TimePoint> point) {
- using namespace std::chrono;
-
- if (!point)
- return WSA_INFINITE;
-
- nanoseconds dur = (std::max)(*point - steady_clock::now(), nanoseconds(0));
- return ceil<milliseconds>(dur).count();
-}
-
MainLoopWindows::MainLoopWindows() {
m_interrupt_event = WSACreateEvent();
assert(m_interrupt_event != WSA_INVALID_EVENT);
@@ -44,36 +37,59 @@ MainLoopWindows::~MainLoopWindows() {
}
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, fd_info] : m_read_fds) {
+ // short circuit waiting if a handle is already ready.
+ if (fd_info.object_sp->HasReadableData())
+ return events.size();
+ events.push_back(fd);
}
events.push_back(m_interrupt_event);
- DWORD result =
- WSAWaitForMultipleEvents(events.size(), events.data(), FALSE,
- ToTimeout(GetNextWakeupTime()), FALSE);
+ while (true) {
+ DWORD timeout = INFINITY;
+ std::optional<lldb_private::MainLoopBase::TimePoint> deadline =
+ GetNextWakeupTime();
+ if (deadline) {
+ std::chrono::milliseconds remaining =
+ std::chrono::duration_cast<std::chrono::milliseconds>(
+ deadline.value() - std::chrono::steady_clock::now());
+ if (remaining.count() <= 0)
+ return events.size() - 1;
+ timeout = remaining.count();
+ }
- for (auto &fd : m_read_fds) {
- int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
- assert(result == 0);
- UNUSED_IF_ASSERT_DISABLED(result);
+ DWORD result =
+ WaitForMultipleObjects(events.size(), events.data(), FALSE, timeout);
+
+ // A timeout is treated as a (premature) signalization of the interrupt
+ // event.
+ if (result == WAIT_TIMEOUT)
+ return events.size() - 1;
+
+ if (result == WAIT_FAILED)
+ return llvm::createStringError(llvm::mapLastWindowsError(),
+ "WaitForMultipleObjects failed");
+
+ // check if interrupt requested.
+ if (result == WAIT_OBJECT_0 + events.size())
+ return result - WAIT_OBJECT_0;
+
+ // An object may be signaled before data is ready for reading, verify it
has
+ // data.
+ if (result >= WAIT_OBJECT_0 &&
+ result < WAIT_OBJECT_0 + (events.size() - 1) &&
+ std::next(m_read_fds.begin(), result - WAIT_OBJECT_0)
+ ->second.object_sp->HasReadableData())
+ return result - WAIT_OBJECT_0;
+
+ // If no handles are actually ready then yeild the thread to allow the CPU
+ // to progress.
+ std::this_thread::yield();
}
- if (result >= WSA_WAIT_EVENT_0 && result < WSA_WAIT_EVENT_0 + events.size())
- return result - WSA_WAIT_EVENT_0;
-
- // A timeout is treated as a (premature) signalization of the interrupt
event.
- if (result == WSA_WAIT_TIMEOUT)
- return events.size() - 1;
-
- return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "WSAWaitForMultipleEvents failed");
+ llvm_unreachable();
}
MainLoopWindows::ReadHandleUP
@@ -83,28 +99,16 @@ 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();
+ assert(waitable_handle != IOObject::kInvalidHandleValue);
const bool inserted =
- m_read_fds
- .try_emplace(object_sp->GetWaitableHandle(), FdInfo{event, callback})
+ m_read_fds.try_emplace(waitable_handle, FdInfo{object_sp, callback})
.second;
if (!inserted) {
- WSACloseEvent(event);
error = Status::FromErrorStringWithFormat(
- "File descriptor %d already monitored.",
- object_sp->GetWaitableHandle());
+ "File descriptor %d already monitored.", waitable_handle);
return nullptr;
}
@@ -114,18 +118,9 @@ 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);
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 +133,7 @@ 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);
+ 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..c0c07cc0b68e3 100644
--- a/lldb/source/Utility/IOObject.cpp
+++ b/lldb/source/Utility/IOObject.cpp
@@ -8,7 +8,16 @@
#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/unittests/Host/FileTest.cpp b/lldb/unittests/Host/FileTest.cpp
index 35c87bb200fad..d973d19430596 100644
--- a/lldb/unittests/Host/FileTest.cpp
+++ b/lldb/unittests/Host/FileTest.cpp
@@ -14,6 +14,10 @@
#include "llvm/Support/Program.h"
#include "gtest/gtest.h"
+#ifdef _WIN32
+#include "lldb/Host/windows/windows.h"
+#endif
+
using namespace lldb;
using namespace lldb_private;
@@ -32,7 +36,11 @@ TEST(File, GetWaitableHandleFileno) {
ASSERT_TRUE(stream);
NativeFile file(stream, true);
- EXPECT_EQ(file.GetWaitableHandle(), fd);
+#ifdef _WIN32
+ EXPECT_EQ(file.GetWaitableHandle(), (HANDLE)_get_osfhandle(fd));
+#else
+ EXPECT_EQ(file.GetWaitableHandle(), (file_t)fd);
+#endif
}
TEST(File, GetStreamFromDescriptor) {
@@ -53,5 +61,9 @@ TEST(File, GetStreamFromDescriptor) {
ASSERT_TRUE(stream != NULL);
EXPECT_EQ(file.GetDescriptor(), fd);
- EXPECT_EQ(file.GetWaitableHandle(), fd);
+#ifdef _WIN32
+ EXPECT_EQ(file.GetWaitableHandle(), (HANDLE)_get_osfhandle(fd));
+#else
+ EXPECT_EQ(file.GetWaitableHandle(), (file_t)fd);
+#endif
}
diff --git a/lldb/unittests/Host/MainLoopTest.cpp
b/lldb/unittests/Host/MainLoopTest.cpp
index e18489978e90c..b8d7f57108017 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -79,6 +79,28 @@ TEST_F(MainLoopTest, ReadObject) {
ASSERT_EQ(1u, callback_count);
}
+TEST_F(MainLoopTest, ReadPipeObject) {
+ Pipe pipe;
+
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+
+ MainLoop loop;
+
+ char X = 'X';
+ size_t len = sizeof(X);
+ ASSERT_THAT_EXPECTED(pipe.Write(&X, len), llvm::Succeeded());
+
+ Status error;
+ auto handle = loop.RegisterReadObject(
+ std::make_shared<NativeFile>(pipe.GetReadFileDescriptor(),
+ File::eOpenOptionReadOnly, false),
+ make_callback(), error);
+ ASSERT_TRUE(error.Success());
+ ASSERT_TRUE(handle);
+ ASSERT_TRUE(loop.Run().Success());
+ ASSERT_EQ(1u, callback_count);
+}
+
TEST_F(MainLoopTest, NoSpuriousReads) {
// Write one byte into the socket.
char X = 'X';
@@ -166,6 +188,10 @@ TEST_F(MainLoopTest, PendingCallbackCalledOnlyOnce) {
if (callback_count == 0) {
loop.AddPendingCallback([&](MainLoopBase &loop) {
callback_count++;
+ char X = 'X';
+ size_t len = sizeof(X);
+ // Write to trigger read object again.
+ ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
});
}
// Terminate the loop on second iteration.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
