https://github.com/ashgti updated
https://github.com/llvm/llvm-project/pull/145621
>From 954f107c85f766ee75370b349eb039819f456db5 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Wed, 25 Jun 2025 18:03:39 -0700
Subject: [PATCH 1/3] [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 | 49 +++++++-
.../posix/ConnectionFileDescriptorPosix.cpp | 14 ++-
lldb/source/Host/windows/MainLoopWindows.cpp | 116 +++++++++---------
lldb/source/Utility/IOObject.cpp | 9 ++
lldb/unittests/Host/FileTest.cpp | 16 ++-
lldb/unittests/Host/MainLoopTest.cpp | 26 ++++
11 files changed, 190 insertions(+), 73 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..47ccbd94ce510 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) {
@@ -380,7 +416,14 @@ Status Socket::Close() {
Log *log = GetLog(LLDBLog::Connection);
LLDB_LOGF(log, "%p Socket::Close (fd = %" PRIu64 ")",
static_cast<void *>(this), static_cast<uint64_t>(m_socket));
-
+#ifdef _WIN32
+ if (m_waitable_handle != kInvalidHandleValue) {
+ if (WSACloseEvent(m_waitable_handle) == 0)
+ m_waitable_handle = kInvalidHandleValue;
+ else
+ error = GetLastError();
+ }
+#endif
bool success = CloseSocket(m_socket) == 0;
// A reference to a FD was passed in, set it to an invalid value
m_socket = kInvalidSocketValue;
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..33cc06266ce03 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,61 @@ 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) {
+ // Check how much time is remaining, we may have woken up early for an
+ // unrelated reason on a file descriptor (e.g. a stat was triggered).
+ 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 yield 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 +101,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 +120,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 +135,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.
>From caa1a9c5547fec0bfbcdc33b183aa0d428d5dd22 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Thu, 26 Jun 2025 20:10:23 -0700
Subject: [PATCH 2/3] Adopting a different strategy for monitoring pipes and
files.
For pipes, we use a monitor thread that performs a zero byte read to detect
changes before signaling.
For files, WaitFor{SingleObject,MultipleObjects}() is signaled if the file
changes on disk.
---
lldb/include/lldb/Host/File.h | 2 -
lldb/include/lldb/Host/Socket.h | 2 -
.../lldb/Host/windows/MainLoopWindows.h | 18 +-
lldb/include/lldb/Utility/IOObject.h | 1 -
lldb/source/Host/common/File.cpp | 14 --
lldb/source/Host/common/JSONTransport.cpp | 2 +-
lldb/source/Host/common/Socket.cpp | 43 +---
lldb/source/Host/windows/MainLoopWindows.cpp | 224 ++++++++++++++----
lldb/unittests/Host/MainLoopTest.cpp | 24 +-
9 files changed, 207 insertions(+), 123 deletions(-)
diff --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index 36cb192281289..9e2d0abe0b1af 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -127,7 +127,6 @@ 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.
///
@@ -401,7 +400,6 @@ 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 6569e9e6ea818..89953ee7fd5b6 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -158,7 +158,6 @@ 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);
@@ -186,7 +185,6 @@ 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 43b7d13a0e445..c7cb92c344008 100644
--- a/lldb/include/lldb/Host/windows/MainLoopWindows.h
+++ b/lldb/include/lldb/Host/windows/MainLoopWindows.h
@@ -31,6 +31,18 @@ class MainLoopWindows : public MainLoopBase {
Status Run() override;
+ struct FdInfo {
+ FdInfo(intptr_t event, Callback callback)
+ : event(event), callback(callback) {}
+ virtual ~FdInfo() {}
+ virtual void WillPoll() {}
+ virtual void DidPoll() {}
+ virtual void Disarm() {}
+ intptr_t event;
+ Callback callback;
+ };
+ using FdInfoUP = std::unique_ptr<FdInfo>;
+
protected:
void UnregisterReadObject(IOObject::WaitableHandle handle) override;
@@ -39,11 +51,7 @@ class MainLoopWindows : public MainLoopBase {
private:
llvm::Expected<size_t> Poll();
- struct FdInfo {
- lldb::IOObjectSP object_sp;
- Callback callback;
- };
- llvm::DenseMap<IOObject::WaitableHandle, FdInfo> m_read_fds;
+ llvm::DenseMap<IOObject::WaitableHandle, FdInfoUP> m_read_fds;
void *m_interrupt_event;
};
diff --git a/lldb/include/lldb/Utility/IOObject.h
b/lldb/include/lldb/Utility/IOObject.h
index 48a8a2076581f..de6532a637083 100644
--- a/lldb/include/lldb/Utility/IOObject.h
+++ b/lldb/include/lldb/Utility/IOObject.h
@@ -41,7 +41,6 @@ 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 2d33f9e2028c4..23b6dc9fe850d 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -118,8 +118,6 @@ 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());
@@ -283,18 +281,6 @@ IOObject::WaitableHandle NativeFile::GetWaitableHandle() {
#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() {
ValueGuard stream_guard = StreamIsValid();
if (!stream_guard) {
diff --git a/lldb/source/Host/common/JSONTransport.cpp
b/lldb/source/Host/common/JSONTransport.cpp
index 1a0851d5c4365..bf269ffa45966 100644
--- a/lldb/source/Host/common/JSONTransport.cpp
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -42,7 +42,7 @@ ReadFull(IOObject &descriptor, size_t length,
if (timeout && timeout_supported) {
SelectHelper sh;
sh.SetTimeout(*timeout);
- sh.FDSetRead(descriptor.GetWaitableHandle());
+ sh.FDSetRead((lldb::socket_t)descriptor.GetWaitableHandle());
Status status = sh.Select();
if (status.Fail()) {
// Convert timeouts into a specific error.
diff --git a/lldb/source/Host/common/Socket.cpp
b/lldb/source/Host/common/Socket.cpp
index 47ccbd94ce510..b6678eedbc938 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -173,7 +173,6 @@ bool Socket::FindProtocolByScheme(const char *scheme,
Socket::Socket(SocketProtocol protocol, bool should_close)
: IOObject(eFDTypeSocket), m_protocol(protocol),
m_socket(kInvalidSocketValue),
- m_waitable_handle(IOObject::kInvalidHandleValue),
m_should_close_fd(should_close) {}
Socket::~Socket() { Close(); }
@@ -318,39 +317,7 @@ Socket::DecodeHostAndPort(llvm::StringRef host_and_port) {
}
IOObject::WaitableHandle Socket::GetWaitableHandle() {
-#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
+ return (IOObject::WaitableHandle)m_socket;
}
Status Socket::Read(void *buf, size_t &num_bytes) {
@@ -416,14 +383,6 @@ Status Socket::Close() {
Log *log = GetLog(LLDBLog::Connection);
LLDB_LOGF(log, "%p Socket::Close (fd = %" PRIu64 ")",
static_cast<void *>(this), static_cast<uint64_t>(m_socket));
-#ifdef _WIN32
- if (m_waitable_handle != kInvalidHandleValue) {
- if (WSACloseEvent(m_waitable_handle) == 0)
- m_waitable_handle = kInvalidHandleValue;
- else
- error = GetLastError();
- }
-#endif
bool success = CloseSocket(m_socket) == 0;
// A reference to a FD was passed in, set it to an invalid value
m_socket = kInvalidSocketValue;
diff --git a/lldb/source/Host/windows/MainLoopWindows.cpp
b/lldb/source/Host/windows/MainLoopWindows.cpp
index 33cc06266ce03..f9ec2bd307a96 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -24,6 +24,139 @@
using namespace lldb;
using namespace lldb_private;
+namespace {
+
+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();
+}
+
+class PipeFdInfo : public MainLoopWindows::FdInfo {
+public:
+ explicit PipeFdInfo(HANDLE handle, MainLoopBase::Callback callback)
+ : FdInfo((intptr_t)CreateEventW(NULL, /*bManualReset=*/FALSE,
+ /*bInitialState=*/FALSE, NULL),
+ callback),
+ handle(handle), ready(CreateEventW(NULL, /*bManualReset=*/FALSE,
+ /*bInitialState=*/FALSE, NULL)) {
+ assert(event && ready);
+ }
+
+ ~PipeFdInfo() override {
+ if (monitor_thread.joinable()) {
+ stopped = true;
+ SetEvent(ready);
+ // Keep trying to cancel ReadFile() until the thread exits.
+ do {
+ CancelIoEx((HANDLE)handle, /*lpOverlapped=*/NULL);
+ } while (WaitForSingleObject(monitor_thread.native_handle(), 1) ==
+ WAIT_TIMEOUT);
+ monitor_thread.join();
+ }
+ CloseHandle((HANDLE)event);
+ CloseHandle(ready);
+ }
+
+ void WillPoll() override {
+ if (!monitor_thread.joinable())
+ monitor_thread = std::thread(&PipeFdInfo::Monitor, this);
+ }
+
+ void Disarm() override {
+ SetEvent(ready);
+ }
+
+ /// Monitors the handle performing a zero byte read to determine when data is
+ /// avaiable.
+ void Monitor() {
+ do {
+ char buf[1];
+ DWORD bytes_read = 0;
+ OVERLAPPED ov = {0};
+ // Block on a 0-byte read; this will only resume when data is
+ // available in the pipe. The pipe must be PIPE_WAIT or this thread
+ // will spin.
+ BOOL success =
+ ReadFile(handle, buf, /*nNumberOfBytesToRead=*/0, &bytes_read, &ov);
+ DWORD bytes_available = 0;
+ DWORD err = GetLastError();
+ if (!success && err == ERROR_IO_PENDING) {
+ success = GetOverlappedResult(handle, &ov, &bytes_read,
+ /*bWait=*/TRUE);
+ err = GetLastError();
+ }
+ if (success) {
+ success = PeekNamedPipe(handle, NULL, 0, NULL, &bytes_available, NULL);
+ err = GetLastError();
+ }
+ if (success) {
+ if (bytes_available == 0) {
+ // This can happen with a zero-byte write. Try again.
+ continue;
+ }
+ } else if (err == ERROR_NO_DATA) {
+ // The pipe is nonblocking. Try again.
+ Sleep(0);
+ continue;
+ } else if (err == ERROR_OPERATION_ABORTED) {
+ // Read may have been cancelled, try again.
+ continue;
+ }
+
+ SetEvent((HANDLE)event);
+
+ // Wait until the current read is consumed before doing the next read.
+ WaitForSingleObject(ready, INFINITE);
+ } while (!stopped);
+ }
+
+ HANDLE handle;
+ HANDLE ready;
+ std::thread monitor_thread;
+ std::atomic<bool> stopped = false;
+};
+
+class SocketFdInfo : public MainLoopWindows::FdInfo {
+public:
+ explicit SocketFdInfo(SOCKET socket, MainLoopBase::Callback callback)
+ : FdInfo((intptr_t)WSACreateEvent(), callback), socket(socket) {
+ assert(event != WSA_INVALID_EVENT);
+ }
+
+ ~SocketFdInfo() override { WSACloseEvent((HANDLE)event); }
+
+ void WillPoll() {
+ int result = WSAEventSelect(socket, (HANDLE)event, FD_READ | FD_ACCEPT |
FD_CLOSE);
+ assert(result == 0);
+ UNUSED_IF_ASSERT_DISABLED(result);
+ }
+
+ void DidPoll() {
+ int result = WSAEventSelect(socket, WSA_INVALID_EVENT, 0);
+ assert(result == 0);
+ UNUSED_IF_ASSERT_DISABLED(result);
+ }
+
+ void Disarm() override {
+ WSAResetEvent((HANDLE)event);
+ }
+
+ SOCKET socket;
+};
+
+class FileFdInfo : public MainLoopWindows::FdInfo {
+public:
+ explicit FileFdInfo(HANDLE handle, MainLoopBase::Callback callback)
+ : FdInfo((intptr_t)handle, callback) {}
+};
+
+} // namespace
+
MainLoopWindows::MainLoopWindows() {
m_interrupt_event = WSACreateEvent();
assert(m_interrupt_event != WSA_INVALID_EVENT);
@@ -39,59 +172,29 @@ MainLoopWindows::~MainLoopWindows() {
llvm::Expected<size_t> MainLoopWindows::Poll() {
std::vector<HANDLE> events;
events.reserve(m_read_fds.size() + 1);
- 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);
+ for (auto &[_, fd_info] : m_read_fds) {
+ fd_info->WillPoll();
+ events.push_back((HANDLE)fd_info->event);
}
events.push_back(m_interrupt_event);
- while (true) {
- DWORD timeout = INFINITY;
- std::optional<lldb_private::MainLoopBase::TimePoint> deadline =
- GetNextWakeupTime();
- if (deadline) {
- // Check how much time is remaining, we may have woken up early for an
- // unrelated reason on a file descriptor (e.g. a stat was triggered).
- 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();
- }
+ DWORD result =
+ WSAWaitForMultipleEvents(events.size(), events.data(), FALSE,
+ ToTimeout(GetNextWakeupTime()), FALSE);
- 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 yield the thread to allow the CPU
- // to progress.
- std::this_thread::yield();
+ for (auto &[_, fd_info] : m_read_fds) {
+ fd_info->DidPoll();
}
- llvm_unreachable();
+ 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");
}
MainLoopWindows::ReadHandleUP
@@ -105,15 +208,31 @@ MainLoopWindows::RegisterReadObject(const IOObjectSP
&object_sp,
IOObject::WaitableHandle waitable_handle = object_sp->GetWaitableHandle();
assert(waitable_handle != IOObject::kInvalidHandleValue);
- const bool inserted =
- m_read_fds.try_emplace(waitable_handle, FdInfo{object_sp, callback})
- .second;
- if (!inserted) {
+ if (m_read_fds.find(waitable_handle) != m_read_fds.end()) {
error = Status::FromErrorStringWithFormat(
"File descriptor %d already monitored.", waitable_handle);
return nullptr;
}
+ if (object_sp->GetFdType() == IOObject::eFDTypeSocket)
+ m_read_fds[waitable_handle] =
+ std::make_unique<SocketFdInfo>((SOCKET)waitable_handle, callback);
+ else
+ switch (GetFileType(waitable_handle)) {
+ case FILE_TYPE_PIPE:
+ m_read_fds[waitable_handle] =
+ std::make_unique<PipeFdInfo>((HANDLE)waitable_handle, callback);
+ break;
+ case FILE_TYPE_DISK:
+ m_read_fds[waitable_handle] =
+ std::make_unique<FileFdInfo>((HANDLE)waitable_handle, callback);
+ break;
+ default:
+ error = Status::FromErrorStringWithFormat("Unsupported file type %d",
+ GetFileType(waitable_handle));
+ return nullptr;
+ }
+
return CreateReadHandle(object_sp);
}
@@ -135,7 +254,8 @@ Status MainLoopWindows::Run() {
if (*signaled_event < m_read_fds.size()) {
auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
- KV.second.callback(*this); // Do the work.
+ KV.second->Disarm();
+ KV.second->callback(*this); // Do the work.
} else {
assert(*signaled_event == m_read_fds.size());
WSAResetEvent(m_interrupt_event);
diff --git a/lldb/unittests/Host/MainLoopTest.cpp
b/lldb/unittests/Host/MainLoopTest.cpp
index b8d7f57108017..a5a5df02eee04 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -101,6 +101,26 @@ TEST_F(MainLoopTest, ReadPipeObject) {
ASSERT_EQ(1u, callback_count);
}
+TEST_F(MainLoopTest, ReadFileObject) {
+ llvm::SmallString<1024> path;
+ int fd;
+ ASSERT_FALSE(llvm::sys::fs::createUniqueFile("test-%%%%%%%%%%", fd, path));
+ std::shared_ptr<NativeFile> file = std::make_shared<NativeFile>(fd,
File::eOpenOptionReadWrite, false);
+
+ MainLoop loop;
+
+ char X = 'X';
+ size_t len = sizeof(X);
+ ASSERT_TRUE(file->Write(&X, len).Success());
+
+ Status error;
+ auto handle = loop.RegisterReadObject(file, 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';
@@ -188,10 +208,6 @@ 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.
>From ba35656894eb7cb9135f662efd31be89398d31b4 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Fri, 27 Jun 2025 10:18:18 -0700
Subject: [PATCH 3/3] Removing disk file support and reverting unrelated
changes.
---
.../lldb/Host/windows/MainLoopWindows.h | 22 +--
lldb/source/Host/common/Socket.cpp | 7 +-
.../posix/ConnectionFileDescriptorPosix.cpp | 11 +-
lldb/source/Host/windows/MainLoopWindows.cpp | 131 ++++++++----------
lldb/unittests/Host/MainLoopTest.cpp | 52 ++++---
5 files changed, 111 insertions(+), 112 deletions(-)
diff --git a/lldb/include/lldb/Host/windows/MainLoopWindows.h
b/lldb/include/lldb/Host/windows/MainLoopWindows.h
index c7cb92c344008..53df815255c3d 100644
--- a/lldb/include/lldb/Host/windows/MainLoopWindows.h
+++ b/lldb/include/lldb/Host/windows/MainLoopWindows.h
@@ -31,17 +31,19 @@ class MainLoopWindows : public MainLoopBase {
Status Run() override;
- struct FdInfo {
- FdInfo(intptr_t event, Callback callback)
- : event(event), callback(callback) {}
- virtual ~FdInfo() {}
+ class IOEvent {
+ public:
+ IOEvent(IOObject::WaitableHandle event) : m_event(event) {}
+ virtual ~IOEvent() {}
virtual void WillPoll() {}
virtual void DidPoll() {}
virtual void Disarm() {}
- intptr_t event;
- Callback callback;
+ IOObject::WaitableHandle GetHandle() { return m_event; }
+
+ protected:
+ IOObject::WaitableHandle m_event;
};
- using FdInfoUP = std::unique_ptr<FdInfo>;
+ using IOEventUP = std::unique_ptr<IOEvent>;
protected:
void UnregisterReadObject(IOObject::WaitableHandle handle) override;
@@ -51,7 +53,11 @@ class MainLoopWindows : public MainLoopBase {
private:
llvm::Expected<size_t> Poll();
- llvm::DenseMap<IOObject::WaitableHandle, FdInfoUP> m_read_fds;
+ struct FdInfo {
+ IOEventUP event;
+ Callback callback;
+ };
+ llvm::DenseMap<IOObject::WaitableHandle, FdInfo> m_read_fds;
void *m_interrupt_event;
};
diff --git a/lldb/source/Host/common/Socket.cpp
b/lldb/source/Host/common/Socket.cpp
index b6678eedbc938..bd21bd673cfb1 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -31,11 +31,8 @@
#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
@@ -172,8 +169,7 @@ 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_should_close_fd(should_close) {}
Socket::~Socket() { Close(); }
@@ -383,6 +379,7 @@ Status Socket::Close() {
Log *log = GetLog(LLDBLog::Connection);
LLDB_LOGF(log, "%p Socket::Close (fd = %" PRIu64 ")",
static_cast<void *>(this), static_cast<uint64_t>(m_socket));
+
bool success = CloseSocket(m_socket) == 0;
// A reference to a FD was passed in, set it to an invalid value
m_socket = kInvalidSocketValue;
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 2fcad7f193e1a..9e2d506dd2af0 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -452,16 +452,14 @@ ConnectionFileDescriptor::BytesAvailable(const
Timeout<std::micro> &timeout,
select_helper.SetTimeout(*timeout);
// FIXME: Migrate to MainLoop.
+ select_helper.FDSetRead((lldb::socket_t)handle);
#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)
@@ -496,12 +494,7 @@ 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
+ if (select_helper.FDIsSetRead((lldb::socket_t)handle))
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 f9ec2bd307a96..eb889ac02e835 100644
--- a/lldb/source/Host/windows/MainLoopWindows.cpp
+++ b/lldb/source/Host/windows/MainLoopWindows.cpp
@@ -24,9 +24,7 @@
using namespace lldb;
using namespace lldb_private;
-namespace {
-
-DWORD ToTimeout(std::optional<MainLoopWindows::TimePoint> point) {
+static DWORD ToTimeout(std::optional<MainLoopWindows::TimePoint> point) {
using namespace std::chrono;
if (!point)
@@ -36,40 +34,39 @@ DWORD ToTimeout(std::optional<MainLoopWindows::TimePoint>
point) {
return ceil<milliseconds>(dur).count();
}
-class PipeFdInfo : public MainLoopWindows::FdInfo {
+namespace {
+
+class PipeEvent : public MainLoopWindows::IOEvent {
public:
- explicit PipeFdInfo(HANDLE handle, MainLoopBase::Callback callback)
- : FdInfo((intptr_t)CreateEventW(NULL, /*bManualReset=*/FALSE,
- /*bInitialState=*/FALSE, NULL),
- callback),
- handle(handle), ready(CreateEventW(NULL, /*bManualReset=*/FALSE,
- /*bInitialState=*/FALSE, NULL)) {
+ explicit PipeEvent(HANDLE handle)
+ : IOEvent((IOObject::WaitableHandle)CreateEventW(NULL,
/*bManualReset=*/FALSE,
+ /*bInitialState=*/FALSE, NULL)),
+ m_handle(handle), m_ready(CreateEventW(NULL, /*bManualReset=*/FALSE,
+ /*bInitialState=*/FALSE, NULL))
{
assert(event && ready);
}
- ~PipeFdInfo() override {
- if (monitor_thread.joinable()) {
- stopped = true;
- SetEvent(ready);
+ ~PipeEvent() override {
+ if (m_monitor_thread.joinable()) {
+ m_stopped = true;
+ SetEvent(m_ready);
// Keep trying to cancel ReadFile() until the thread exits.
do {
- CancelIoEx((HANDLE)handle, /*lpOverlapped=*/NULL);
- } while (WaitForSingleObject(monitor_thread.native_handle(), 1) ==
+ CancelIoEx((HANDLE)m_handle, /*lpOverlapped=*/NULL);
+ } while (WaitForSingleObject(m_monitor_thread.native_handle(), 1) ==
WAIT_TIMEOUT);
- monitor_thread.join();
+ m_monitor_thread.join();
}
- CloseHandle((HANDLE)event);
- CloseHandle(ready);
+ CloseHandle((HANDLE)m_event);
+ CloseHandle(m_ready);
}
- void WillPoll() override {
- if (!monitor_thread.joinable())
- monitor_thread = std::thread(&PipeFdInfo::Monitor, this);
+ void WillPoll() override {
+ if (!m_monitor_thread.joinable())
+ m_monitor_thread = std::thread(&PipeEvent::Monitor, this);
}
- void Disarm() override {
- SetEvent(ready);
- }
+ void Disarm() override { SetEvent(m_ready); }
/// Monitors the handle performing a zero byte read to determine when data is
/// avaiable.
@@ -82,16 +79,17 @@ class PipeFdInfo : public MainLoopWindows::FdInfo {
// available in the pipe. The pipe must be PIPE_WAIT or this thread
// will spin.
BOOL success =
- ReadFile(handle, buf, /*nNumberOfBytesToRead=*/0, &bytes_read, &ov);
+ ReadFile(m_handle, buf, /*nNumberOfBytesToRead=*/0, &bytes_read,
&ov);
DWORD bytes_available = 0;
DWORD err = GetLastError();
if (!success && err == ERROR_IO_PENDING) {
- success = GetOverlappedResult(handle, &ov, &bytes_read,
+ success = GetOverlappedResult(m_handle, &ov, &bytes_read,
/*bWait=*/TRUE);
err = GetLastError();
}
if (success) {
- success = PeekNamedPipe(handle, NULL, 0, NULL, &bytes_available, NULL);
+ success =
+ PeekNamedPipe(m_handle, NULL, 0, NULL, &bytes_available, NULL);
err = GetLastError();
}
if (success) {
@@ -108,51 +106,45 @@ class PipeFdInfo : public MainLoopWindows::FdInfo {
continue;
}
- SetEvent((HANDLE)event);
+ SetEvent((HANDLE)m_event);
// Wait until the current read is consumed before doing the next read.
- WaitForSingleObject(ready, INFINITE);
- } while (!stopped);
+ WaitForSingleObject(m_ready, INFINITE);
+ } while (!m_stopped);
}
- HANDLE handle;
- HANDLE ready;
- std::thread monitor_thread;
- std::atomic<bool> stopped = false;
+private:
+ HANDLE m_handle;
+ HANDLE m_ready;
+ std::thread m_monitor_thread;
+ std::atomic<bool> m_stopped = false;
};
-class SocketFdInfo : public MainLoopWindows::FdInfo {
+class SocketEvent : public MainLoopWindows::IOEvent {
public:
- explicit SocketFdInfo(SOCKET socket, MainLoopBase::Callback callback)
- : FdInfo((intptr_t)WSACreateEvent(), callback), socket(socket) {
+ explicit SocketEvent(SOCKET socket)
+ : IOEvent((IOObject::WaitableHandle)WSACreateEvent()), m_socket(socket) {
assert(event != WSA_INVALID_EVENT);
}
- ~SocketFdInfo() override { WSACloseEvent((HANDLE)event); }
+ ~SocketEvent() override { WSACloseEvent((HANDLE)m_event); }
void WillPoll() {
- int result = WSAEventSelect(socket, (HANDLE)event, FD_READ | FD_ACCEPT |
FD_CLOSE);
+ int result =
+ WSAEventSelect(m_socket, (HANDLE)m_event, FD_READ | FD_ACCEPT |
FD_CLOSE);
assert(result == 0);
UNUSED_IF_ASSERT_DISABLED(result);
}
void DidPoll() {
- int result = WSAEventSelect(socket, WSA_INVALID_EVENT, 0);
+ int result = WSAEventSelect(m_socket, WSA_INVALID_EVENT, 0);
assert(result == 0);
UNUSED_IF_ASSERT_DISABLED(result);
}
- void Disarm() override {
- WSAResetEvent((HANDLE)event);
- }
-
- SOCKET socket;
-};
+ void Disarm() override { WSAResetEvent((HANDLE)m_event); }
-class FileFdInfo : public MainLoopWindows::FdInfo {
-public:
- explicit FileFdInfo(HANDLE handle, MainLoopBase::Callback callback)
- : FdInfo((intptr_t)handle, callback) {}
+ SOCKET m_socket;
};
} // namespace
@@ -173,8 +165,8 @@ llvm::Expected<size_t> MainLoopWindows::Poll() {
std::vector<HANDLE> events;
events.reserve(m_read_fds.size() + 1);
for (auto &[_, fd_info] : m_read_fds) {
- fd_info->WillPoll();
- events.push_back((HANDLE)fd_info->event);
+ fd_info.event->WillPoll();
+ events.push_back((HANDLE)fd_info.event->GetHandle());
}
events.push_back(m_interrupt_event);
@@ -183,7 +175,7 @@ llvm::Expected<size_t> MainLoopWindows::Poll() {
ToTimeout(GetNextWakeupTime()), FALSE);
for (auto &[_, fd_info] : m_read_fds) {
- fd_info->DidPoll();
+ fd_info.event->DidPoll();
}
if (result >= WSA_WAIT_EVENT_0 && result < WSA_WAIT_EVENT_0 + events.size())
@@ -215,23 +207,16 @@ MainLoopWindows::RegisterReadObject(const IOObjectSP
&object_sp,
}
if (object_sp->GetFdType() == IOObject::eFDTypeSocket)
- m_read_fds[waitable_handle] =
- std::make_unique<SocketFdInfo>((SOCKET)waitable_handle, callback);
- else
- switch (GetFileType(waitable_handle)) {
- case FILE_TYPE_PIPE:
- m_read_fds[waitable_handle] =
- std::make_unique<PipeFdInfo>((HANDLE)waitable_handle, callback);
- break;
- case FILE_TYPE_DISK:
- m_read_fds[waitable_handle] =
- std::make_unique<FileFdInfo>((HANDLE)waitable_handle, callback);
- break;
- default:
- error = Status::FromErrorStringWithFormat("Unsupported file type %d",
- GetFileType(waitable_handle));
- return nullptr;
- }
+ m_read_fds[waitable_handle] = {
+ std::make_unique<SocketEvent>((SOCKET)waitable_handle), callback};
+ else if (GetFileType(waitable_handle) == FILE_TYPE_PIPE)
+ m_read_fds[waitable_handle] = {
+ std::make_unique<PipeEvent>((HANDLE)waitable_handle), callback};
+ else {
+ error = Status::FromErrorStringWithFormat("Unsupported file type %d",
+ GetFileType(waitable_handle));
+ return nullptr;
+ }
return CreateReadHandle(object_sp);
}
@@ -254,8 +239,8 @@ Status MainLoopWindows::Run() {
if (*signaled_event < m_read_fds.size()) {
auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
- KV.second->Disarm();
- KV.second->callback(*this); // Do the work.
+ KV.second.event->Disarm();
+ KV.second.callback(*this); // Do the work.
} else {
assert(*signaled_event == m_read_fds.size());
WSAResetEvent(m_interrupt_event);
diff --git a/lldb/unittests/Host/MainLoopTest.cpp
b/lldb/unittests/Host/MainLoopTest.cpp
index a5a5df02eee04..83a4643e05d84 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -82,7 +82,7 @@ TEST_F(MainLoopTest, ReadObject) {
TEST_F(MainLoopTest, ReadPipeObject) {
Pipe pipe;
- ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ ASSERT_TRUE(pipe.CreateNew(false).Success());
MainLoop loop;
@@ -101,27 +101,46 @@ TEST_F(MainLoopTest, ReadPipeObject) {
ASSERT_EQ(1u, callback_count);
}
-TEST_F(MainLoopTest, ReadFileObject) {
- llvm::SmallString<1024> path;
- int fd;
- ASSERT_FALSE(llvm::sys::fs::createUniqueFile("test-%%%%%%%%%%", fd, path));
- std::shared_ptr<NativeFile> file = std::make_shared<NativeFile>(fd,
File::eOpenOptionReadWrite, false);
+TEST_F(MainLoopTest, NoSpuriousPipeReads) {
+ Pipe pipe;
- MainLoop loop;
+ ASSERT_TRUE(pipe.CreateNew(false).Success());
char X = 'X';
size_t len = sizeof(X);
- ASSERT_TRUE(file->Write(&X, len).Success());
+ ASSERT_THAT_EXPECTED(pipe.Write(&X, len), llvm::Succeeded());
+
+ lldb::IOObjectSP r =
std::make_shared<NativeFile>(pipe.GetReadFileDescriptor(),
+ File::eOpenOptionReadOnly,
false);
+
+ MainLoop loop;
Status error;
- auto handle = loop.RegisterReadObject(file, make_callback(), error);
- ASSERT_TRUE(error.Success());
- ASSERT_TRUE(handle);
- ASSERT_TRUE(loop.Run().Success());
+ auto handle = loop.RegisterReadObject(
+ r,
+ [&](MainLoopBase &) {
+ if (callback_count == 0) {
+ // Read the byte back the first time we're called. After that, the
+ // pipe is empty, and we should not be called anymore.
+ char X;
+ size_t len = sizeof(X);
+ EXPECT_THAT_ERROR(r->Read(&X, len).ToError(), llvm::Succeeded());
+ EXPECT_EQ(len, sizeof(X));
+ }
+ ++callback_count;
+ },
+ error);
+ ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+ // Terminate the loop after one second.
+ loop.AddCallback([](MainLoopBase &loop) { loop.RequestTermination(); },
+ std::chrono::seconds(1));
+ ASSERT_THAT_ERROR(loop.Run().ToError(), llvm::Succeeded());
+
+ // Make sure the callback was called only once.
ASSERT_EQ(1u, callback_count);
}
-TEST_F(MainLoopTest, NoSpuriousReads) {
+TEST_F(MainLoopTest, NoSpuriousSocketReads) {
// Write one byte into the socket.
char X = 'X';
size_t len = sizeof(X);
@@ -206,9 +225,8 @@ TEST_F(MainLoopTest, PendingCallbackCalledOnlyOnce) {
[&](MainLoopBase &loop) {
// Add one pending callback on the first iteration.
if (callback_count == 0) {
- loop.AddPendingCallback([&](MainLoopBase &loop) {
- callback_count++;
- });
+ loop.AddPendingCallback(
+ [&](MainLoopBase &loop) { callback_count++; });
}
// Terminate the loop on second iteration.
if (callback_count++ >= 1)
@@ -363,7 +381,7 @@ TEST_F(MainLoopTest, UnmonitoredSignal) {
MainLoop loop;
Status error;
struct sigaction sa;
- sa.sa_sigaction = [](int, siginfo_t *, void *) { };
+ sa.sa_sigaction = [](int, siginfo_t *, void *) {};
sa.sa_flags = SA_SIGINFO; // important: no SA_RESTART
sigemptyset(&sa.sa_mask);
ASSERT_EQ(0, sigaction(SIGUSR2, &sa, nullptr));
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
