[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)
Dmitry Vasilyev via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 5 09:19:00 PDT 2024
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101383
>From 14a653c244ea36233de288ebe67a9f42adaacfc5 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Wed, 31 Jul 2024 22:02:53 +0400
Subject: [PATCH 1/3] [lldb] Added Pipe::WriteWithTimeout()
Fixed few bugs in PipeWindows. Added the test for async read/write.
---
lldb/include/lldb/Host/PipeBase.h | 5 +-
lldb/include/lldb/Host/posix/PipePosix.h | 4 +-
lldb/include/lldb/Host/windows/PipeWindows.h | 5 +-
lldb/source/Host/common/PipeBase.cpp | 5 +
lldb/source/Host/posix/PipePosix.cpp | 6 +-
lldb/source/Host/windows/PipeWindows.cpp | 128 ++++++++++++-------
lldb/unittests/Host/PipeTest.cpp | 66 +++++++++-
7 files changed, 165 insertions(+), 54 deletions(-)
diff --git a/lldb/include/lldb/Host/PipeBase.h b/lldb/include/lldb/Host/PipeBase.h
index 48c19b899cef6..d51d0cd54e036 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -56,7 +56,10 @@ class PipeBase {
// Delete named pipe.
virtual Status Delete(llvm::StringRef name) = 0;
- virtual Status Write(const void *buf, size_t size, size_t &bytes_written) = 0;
+ virtual Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) = 0;
+ Status Write(const void *buf, size_t size, size_t &bytes_written);
virtual Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h b/lldb/include/lldb/Host/posix/PipePosix.h
index ec4c752a24e94..2e291160817c4 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -64,7 +64,9 @@ class PipePosix : public PipeBase {
Status Delete(llvm::StringRef name) override;
- Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) override;
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..e28d104cc60ec 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -32,7 +32,6 @@ class PipeWindows : public PipeBase {
Status CreateNew(bool child_process_inherit) override;
// Create a named pipe.
- Status CreateNewNamed(bool child_process_inherit);
Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
Status CreateWithUniqueName(llvm::StringRef prefix,
bool child_process_inherit,
@@ -60,7 +59,9 @@ class PipeWindows : public PipeBase {
Status Delete(llvm::StringRef name) override;
- Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) override;
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/source/Host/common/PipeBase.cpp b/lldb/source/Host/common/PipeBase.cpp
index b3e0ab34a58df..904a2df12392d 100644
--- a/lldb/source/Host/common/PipeBase.cpp
+++ b/lldb/source/Host/common/PipeBase.cpp
@@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name,
std::chrono::microseconds::zero());
}
+Status PipeBase::Write(const void *buf, size_t size, size_t &bytes_written) {
+ return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(),
+ bytes_written);
+}
+
Status PipeBase::Read(void *buf, size_t size, size_t &bytes_read) {
return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(),
bytes_read);
diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..00c6242f3f2e8 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
return error;
}
-Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
+Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written) {
std::lock_guard<std::mutex> guard(m_write_mutex);
bytes_written = 0;
if (!CanWriteUnlocked())
@@ -343,7 +345,7 @@ Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
const int fd = GetWriteFileDescriptorUnlocked();
SelectHelper select_helper;
- select_helper.SetTimeout(std::chrono::seconds(0));
+ select_helper.SetTimeout(timeout);
select_helper.FDSetWrite(fd);
Status error;
diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp
index c82c919607b5b..93d21b6cf9a05 100644
--- a/lldb/source/Host/windows/PipeWindows.cpp
+++ b/lldb/source/Host/windows/PipeWindows.cpp
@@ -58,30 +58,15 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write)
}
ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+ m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
+
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
PipeWindows::~PipeWindows() { Close(); }
Status PipeWindows::CreateNew(bool child_process_inherit) {
- // Create an anonymous pipe with the specified inheritance.
- SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0,
- child_process_inherit ? TRUE : FALSE};
- BOOL result = ::CreatePipe(&m_read, &m_write, &sa, 1024);
- if (result == FALSE)
- return Status(::GetLastError(), eErrorTypeWin32);
-
- m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
- ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
- m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
-
- m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
- ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
-
- return Status();
-}
-
-Status PipeWindows::CreateNewNamed(bool child_process_inherit) {
// Even for anonymous pipes, we open a named pipe. This is because you
// cannot get overlapped i/o on Windows without using a named pipe. So we
// synthesize a unique name.
@@ -105,12 +90,19 @@ Status PipeWindows::CreateNew(llvm::StringRef name,
std::string pipe_path = g_pipe_name_prefix.str();
pipe_path.append(name.str());
+ SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), 0,
+ child_process_inherit ? TRUE : FALSE};
+
// Always open for overlapped i/o. We implement blocking manually in Read
// and Write.
DWORD read_mode = FILE_FLAG_OVERLAPPED;
- m_read = ::CreateNamedPipeA(
- pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode,
- PIPE_TYPE_BYTE | PIPE_WAIT, 1, 1024, 1024, 120 * 1000, NULL);
+ m_read =
+ ::CreateNamedPipeA(pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode,
+ PIPE_TYPE_BYTE | PIPE_WAIT, 1,
+ 1024, // Out buffer size
+ 1024, // In buffer size
+ 0, // Default timeout in ms, 0 means 50ms
+ &sa);
if (INVALID_HANDLE_VALUE == m_read)
return Status(::GetLastError(), eErrorTypeWin32);
m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
@@ -155,7 +147,7 @@ Status PipeWindows::CreateWithUniqueName(llvm::StringRef prefix,
Status PipeWindows::OpenAsReader(llvm::StringRef name,
bool child_process_inherit) {
if (CanRead())
- return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
+ return Status(); // Note the name is ignored.
return OpenNamedPipe(name, child_process_inherit, true);
}
@@ -165,7 +157,7 @@ PipeWindows::OpenAsWriterWithTimeout(llvm::StringRef name,
bool child_process_inherit,
const std::chrono::microseconds &timeout) {
if (CanWrite())
- return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
+ return Status(); // Note the name is ignored.
return OpenNamedPipe(name, child_process_inherit, false);
}
@@ -177,8 +169,8 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name,
assert(is_read ? !CanRead() : !CanWrite());
- SECURITY_ATTRIBUTES attributes = {};
- attributes.bInheritHandle = child_process_inherit;
+ SECURITY_ATTRIBUTES attributes{sizeof(SECURITY_ATTRIBUTES), 0,
+ child_process_inherit ? TRUE : FALSE};
std::string pipe_path = g_pipe_name_prefix.str();
pipe_path.append(name.str());
@@ -202,6 +194,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name,
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
return Status();
@@ -228,6 +221,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() {
return PipeWindows::kInvalidDescriptor;
int result = m_write_fd;
m_write_fd = PipeWindows::kInvalidDescriptor;
+ if (m_write_overlapped.hEvent)
+ ::CloseHandle(m_write_overlapped.hEvent);
m_write = INVALID_HANDLE_VALUE;
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
return result;
@@ -250,6 +245,9 @@ void PipeWindows::CloseWriteFileDescriptor() {
if (!CanWrite())
return;
+ if (m_write_overlapped.hEvent)
+ ::CloseHandle(m_write_overlapped.hEvent);
+
_close(m_write_fd);
m_write = INVALID_HANDLE_VALUE;
m_write_fd = PipeWindows::kInvalidDescriptor;
@@ -280,15 +278,21 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32);
bytes_read = 0;
- DWORD sys_bytes_read = size;
- BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
- &m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
- return Status(::GetLastError(), eErrorTypeWin32);
+ DWORD sys_bytes_read = 0;
+ BOOL result =
+ ::ReadFile(m_read, buf, size, &sys_bytes_read, &m_read_overlapped);
+ if (result) {
+ bytes_read = sys_bytes_read;
+ return Status();
+ }
+
+ DWORD failure_error = ::GetLastError();
+ if (failure_error != ERROR_IO_PENDING)
+ return Status(failure_error, eErrorTypeWin32);
DWORD timeout = (duration == std::chrono::microseconds::zero())
? INFINITE
- : duration.count() * 1000;
+ : duration.count() / 1000;
DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
if (wait_result != WAIT_OBJECT_0) {
// The operation probably failed. However, if it timed out, we need to
@@ -298,10 +302,10 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
// happens, the original operation should be considered to have been
// successful.
bool failed = true;
- DWORD failure_error = ::GetLastError();
+ failure_error = ::GetLastError();
if (wait_result == WAIT_TIMEOUT) {
- BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped);
- if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
+ BOOL cancel_result = ::CancelIoEx(m_read, &m_read_overlapped);
+ if (!cancel_result && ::GetLastError() == ERROR_NOT_FOUND)
failed = false;
}
if (failed)
@@ -310,27 +314,61 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
// Now we call GetOverlappedResult setting bWait to false, since we've
// already waited as long as we're willing to.
- if (!GetOverlappedResult(m_read, &m_read_overlapped, &sys_bytes_read, FALSE))
+ if (!::GetOverlappedResult(m_read, &m_read_overlapped, &sys_bytes_read,
+ FALSE))
return Status(::GetLastError(), eErrorTypeWin32);
bytes_read = sys_bytes_read;
return Status();
}
-Status PipeWindows::Write(const void *buf, size_t num_bytes,
- size_t &bytes_written) {
+Status PipeWindows::WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &duration,
+ size_t &bytes_written) {
if (!CanWrite())
return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32);
- DWORD sys_bytes_written = 0;
- BOOL write_result = ::WriteFile(m_write, buf, num_bytes, &sys_bytes_written,
- &m_write_overlapped);
- if (!write_result && GetLastError() != ERROR_IO_PENDING)
- return Status(::GetLastError(), eErrorTypeWin32);
+ bytes_written = 0;
+ DWORD sys_bytes_write = 0;
+ BOOL result =
+ ::WriteFile(m_write, buf, size, &sys_bytes_write, &m_write_overlapped);
+ if (result) {
+ bytes_written = sys_bytes_write;
+ return Status();
+ }
+
+ DWORD failure_error = ::GetLastError();
+ if (failure_error != ERROR_IO_PENDING)
+ return Status(failure_error, eErrorTypeWin32);
- BOOL result = GetOverlappedResult(m_write, &m_write_overlapped,
- &sys_bytes_written, TRUE);
- if (!result)
+ DWORD timeout = (duration == std::chrono::microseconds::zero())
+ ? INFINITE
+ : duration.count() / 1000;
+ DWORD wait_result = ::WaitForSingleObject(m_write_overlapped.hEvent, timeout);
+ if (wait_result != WAIT_OBJECT_0) {
+ // The operation probably failed. However, if it timed out, we need to
+ // cancel the I/O. Between the time we returned from WaitForSingleObject
+ // and the time we call CancelIoEx, the operation may complete. If that
+ // hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that
+ // happens, the original operation should be considered to have been
+ // successful.
+ bool failed = true;
+ failure_error = ::GetLastError();
+ if (wait_result == WAIT_TIMEOUT) {
+ BOOL cancel_result = ::CancelIoEx(m_write, &m_write_overlapped);
+ if (!cancel_result && ::GetLastError() == ERROR_NOT_FOUND)
+ failed = false;
+ }
+ if (failed)
+ return Status(failure_error, eErrorTypeWin32);
+ }
+
+ // Now we call GetOverlappedResult setting bWait to false, since we've
+ // already waited as long as we're willing to.
+ if (!::GetOverlappedResult(m_write, &m_write_overlapped, &sys_bytes_write,
+ FALSE))
return Status(::GetLastError(), eErrorTypeWin32);
+
+ bytes_written = sys_bytes_write;
return Status();
}
diff --git a/lldb/unittests/Host/PipeTest.cpp b/lldb/unittests/Host/PipeTest.cpp
index 35a44ccf03733..cfd4cddbe86c3 100644
--- a/lldb/unittests/Host/PipeTest.cpp
+++ b/lldb/unittests/Host/PipeTest.cpp
@@ -29,8 +29,6 @@ TEST_F(PipeTest, CreateWithUniqueName) {
llvm::Succeeded());
}
-// Test broken
-#ifndef _WIN32
TEST_F(PipeTest, OpenAsReader) {
Pipe pipe;
llvm::SmallString<0> name;
@@ -44,8 +42,70 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
+ // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
+ // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
+
+ ASSERT_TRUE(pipe.CanRead());
+}
+
+TEST_F(PipeTest, WriteWithTimeout) {
+ Pipe pipe;
+ ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
+ // Note write_chunk_size must be less than the pipe buffer.
+ // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ const size_t buf_size = 8192;
+ const size_t write_chunk_size = 256;
+ const size_t read_chunk_size = 300;
+ std::unique_ptr<int32_t[]> write_buf_ptr(
+ new int32_t[buf_size / sizeof(int32_t)]);
+ int32_t *write_buf = write_buf_ptr.get();
+ std::unique_ptr<int32_t[]> read_buf_ptr(
+ new int32_t[(buf_size + 100) / sizeof(int32_t)]);
+ int32_t *read_buf = read_buf_ptr.get();
+ for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
+ write_buf[i] = i;
+ read_buf[i] = -i;
+ }
+
+ char *write_ptr = (char *)write_buf;
+ size_t write_bytes = 0;
+ char *read_ptr = (char *)read_buf;
+ size_t read_bytes = 0;
+ size_t num_bytes = 0;
+ Status error;
+ while (write_bytes < buf_size) {
+ error = pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail()) {
+ ASSERT_TRUE(read_bytes < buf_size);
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, read_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail())
+ FAIL();
+ else
+ read_bytes += num_bytes;
+ } else
+ write_bytes += num_bytes;
+ }
+ // Read the rest data.
+ while (read_bytes < buf_size) {
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, buf_size - read_bytes,
+ std::chrono::milliseconds(10), num_bytes);
+ if (error.Fail())
+ FAIL();
+ else
+ read_bytes += num_bytes;
+ }
+
+ // Be sure the pipe is empty.
+ error = pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
+ std::chrono::milliseconds(10), num_bytes);
+ ASSERT_TRUE(error.Fail());
+
+ // Compare the data.
+ ASSERT_EQ(write_bytes, read_bytes);
+ ASSERT_EQ(memcmp(write_buf, read_buf, buf_size), 0);
}
-#endif
>From b0a38a101345a09d199165b51318777af3ff27ba Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Mon, 5 Aug 2024 17:03:27 +0400
Subject: [PATCH 2/3] Updated PipeTest.WriteWithTimeout
---
lldb/source/Host/windows/PipeWindows.cpp | 9 +-
lldb/unittests/Host/PipeTest.cpp | 116 ++++++++++++++---------
2 files changed, 77 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp
index 93d21b6cf9a05..17c8087982e53 100644
--- a/lldb/source/Host/windows/PipeWindows.cpp
+++ b/lldb/source/Host/windows/PipeWindows.cpp
@@ -98,11 +98,10 @@ Status PipeWindows::CreateNew(llvm::StringRef name,
DWORD read_mode = FILE_FLAG_OVERLAPPED;
m_read =
::CreateNamedPipeA(pipe_path.c_str(), PIPE_ACCESS_INBOUND | read_mode,
- PIPE_TYPE_BYTE | PIPE_WAIT, 1,
- 1024, // Out buffer size
- 1024, // In buffer size
- 0, // Default timeout in ms, 0 means 50ms
- &sa);
+ PIPE_TYPE_BYTE | PIPE_WAIT, /*nMaxInstances=*/1,
+ /*nOutBufferSize=*/1024,
+ /*nInBufferSize=*/1024,
+ /*nDefaultTimeOut=*/0, &sa);
if (INVALID_HANDLE_VALUE == m_read)
return Status(::GetLastError(), eErrorTypeWin32);
m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
diff --git a/lldb/unittests/Host/PipeTest.cpp b/lldb/unittests/Host/PipeTest.cpp
index cfd4cddbe86c3..9d80f760e4524 100644
--- a/lldb/unittests/Host/PipeTest.cpp
+++ b/lldb/unittests/Host/PipeTest.cpp
@@ -12,6 +12,9 @@
#include "lldb/Host/HostInfo.h"
#include "gtest/gtest.h"
+#include <numeric>
+#include <vector>
+
using namespace lldb_private;
class PipeTest : public testing::Test {
@@ -29,6 +32,8 @@ TEST_F(PipeTest, CreateWithUniqueName) {
llvm::Succeeded());
}
+// Test broken
+#ifndef _WIN32
TEST_F(PipeTest, OpenAsReader) {
Pipe pipe;
llvm::SmallString<0> name;
@@ -42,70 +47,95 @@ TEST_F(PipeTest, OpenAsReader) {
size_t name_len = name.size();
name += "foobar";
llvm::StringRef name_ref(name.data(), name_len);
- // Note OpenAsReader() do nothing on Windows, the pipe is already opened for
- // read and write.
ASSERT_THAT_ERROR(
pipe.OpenAsReader(name_ref, /*child_process_inherit=*/false).ToError(),
llvm::Succeeded());
ASSERT_TRUE(pipe.CanRead());
}
+#endif
TEST_F(PipeTest, WriteWithTimeout) {
Pipe pipe;
ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
// Note write_chunk_size must be less than the pipe buffer.
- // The pipe buffer is 1024 for PipeWindows and 4096 for PipePosix.
+ // The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
const size_t buf_size = 8192;
- const size_t write_chunk_size = 256;
- const size_t read_chunk_size = 300;
- std::unique_ptr<int32_t[]> write_buf_ptr(
- new int32_t[buf_size / sizeof(int32_t)]);
- int32_t *write_buf = write_buf_ptr.get();
- std::unique_ptr<int32_t[]> read_buf_ptr(
- new int32_t[(buf_size + 100) / sizeof(int32_t)]);
- int32_t *read_buf = read_buf_ptr.get();
- for (int i = 0; i < buf_size / sizeof(int32_t); ++i) {
- write_buf[i] = i;
- read_buf[i] = -i;
- }
+ const size_t write_chunk_size = 234;
- char *write_ptr = (char *)write_buf;
+ std::vector<int32_t> write_buf(buf_size / sizeof(int32_t));
+ std::iota(write_buf.begin(), write_buf.end(), 0);
+ std::vector<int32_t> read_buf(write_buf.size() + 100, -1);
+
+ char *write_ptr = (char *)&write_buf.front();
+ char *read_ptr = (char *)&read_buf.front();
size_t write_bytes = 0;
- char *read_ptr = (char *)read_buf;
size_t read_bytes = 0;
size_t num_bytes = 0;
- Status error;
+
+ // Write to the pipe until it is full.
while (write_bytes < buf_size) {
- error = pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
- std::chrono::milliseconds(10), num_bytes);
- if (error.Fail()) {
- ASSERT_TRUE(read_bytes < buf_size);
- error = pipe.ReadWithTimeout(read_ptr + read_bytes, read_chunk_size,
- std::chrono::milliseconds(10), num_bytes);
- if (error.Fail())
- FAIL();
- else
- read_bytes += num_bytes;
- } else
- write_bytes += num_bytes;
- }
- // Read the rest data.
- while (read_bytes < buf_size) {
- error = pipe.ReadWithTimeout(read_ptr + read_bytes, buf_size - read_bytes,
- std::chrono::milliseconds(10), num_bytes);
+ Status error =
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(10), num_bytes);
if (error.Fail())
- FAIL();
- else
- read_bytes += num_bytes;
+ break; // The write buffer is full
+ write_bytes += num_bytes;
+ }
+ ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
+
+ // Attempt a write with a long timeout.
+ auto start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(2000), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(
+ std::chrono::steady_clock::now() - start_time)
+ .count();
+ ASSERT_GE(dur, 2000);
+
+ // Attempt a write with a short timeout
+ start_time = std::chrono::steady_clock::now();
+ ASSERT_THAT_ERROR(
+ pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
+ std::chrono::milliseconds(200), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ dur = std::chrono::duration_cast<std::chrono::milliseconds>(
+ std::chrono::steady_clock::now() - start_time)
+ .count();
+ ASSERT_GE(dur, 200);
+ ASSERT_LT(dur, 300);
+
+ // Drain the pipe
+ while (read_bytes < write_bytes) {
+ ASSERT_THAT_ERROR(
+ pipe.ReadWithTimeout(read_ptr + read_bytes, write_bytes - read_bytes,
+ std::chrono::milliseconds(10), num_bytes)
+ .ToError(),
+ llvm::Succeeded());
+ read_bytes += num_bytes;
}
// Be sure the pipe is empty.
- error = pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
- std::chrono::milliseconds(10), num_bytes);
- ASSERT_TRUE(error.Fail());
+ ASSERT_THAT_ERROR(pipe.ReadWithTimeout(read_ptr + read_bytes, 100,
+ std::chrono::milliseconds(10),
+ num_bytes)
+ .ToError(),
+ llvm::Failed());
- // Compare the data.
+ // Check that we got what we wrote.
ASSERT_EQ(write_bytes, read_bytes);
- ASSERT_EQ(memcmp(write_buf, read_buf, buf_size), 0);
+ ASSERT_TRUE(std::equal(write_buf.begin(),
+ write_buf.begin() + write_bytes / sizeof(uint32_t),
+ read_buf.begin()));
+
+ // Write to the pipe again and check that it succeeds.
+ ASSERT_THAT_ERROR(pipe.WriteWithTimeout(write_ptr, write_chunk_size,
+ std::chrono::milliseconds(10),
+ num_bytes)
+ .ToError(),
+ llvm::Succeeded());
}
>From b06518abdf65ccb4359b6a9fb68c46d00670dccb Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Mon, 5 Aug 2024 18:24:28 +0400
Subject: [PATCH 3/3] Updated the test. Increased the buffer size.
---
lldb/unittests/Host/PipeTest.cpp | 59 +++++++++++++++++++-------------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/lldb/unittests/Host/PipeTest.cpp b/lldb/unittests/Host/PipeTest.cpp
index 9d80f760e4524..506f3d225a21e 100644
--- a/lldb/unittests/Host/PipeTest.cpp
+++ b/lldb/unittests/Host/PipeTest.cpp
@@ -11,7 +11,7 @@
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
#include "gtest/gtest.h"
-
+#include <fcntl.h>
#include <numeric>
#include <vector>
@@ -58,58 +58,69 @@ TEST_F(PipeTest, OpenAsReader) {
TEST_F(PipeTest, WriteWithTimeout) {
Pipe pipe;
ASSERT_THAT_ERROR(pipe.CreateNew(false).ToError(), llvm::Succeeded());
- // Note write_chunk_size must be less than the pipe buffer.
+
// The pipe buffer is 1024 for PipeWindows and at least 512 on Darwin.
- const size_t buf_size = 8192;
+ // In Linux versions before 2.6.11, the capacity of a pipe was the same as the
+ // system page size (e.g., 4096 bytes on i386).
+ // Since Linux 2.6.11, the pipe capacity is 16 pages (i.e., 65,536 bytes in a
+ // system with a page size of 4096 bytes).
+ // Since Linux 2.6.35, the default pipe capacity is 16 pages, but the capacity
+ // can be queried and set using the fcntl(2) F_GETPIPE_SZ and F_SETPIPE_SZ
+ // operations:
+
+#if !defined(_WIN32) && defined(F_SETPIPE_SZ)
+ ::fcntl(pipe.GetWriteFileDescriptor(), F_SETPIPE_SZ, 4096);
+#endif
+
+ const size_t buf_size = 66000;
+
+ // Note write_chunk_size must be less than the pipe buffer.
const size_t write_chunk_size = 234;
std::vector<int32_t> write_buf(buf_size / sizeof(int32_t));
std::iota(write_buf.begin(), write_buf.end(), 0);
std::vector<int32_t> read_buf(write_buf.size() + 100, -1);
- char *write_ptr = (char *)&write_buf.front();
- char *read_ptr = (char *)&read_buf.front();
+ char *write_ptr = reinterpret_cast<char *>(write_buf.data());
+ char *read_ptr = reinterpret_cast<char *>(read_buf.data());
size_t write_bytes = 0;
size_t read_bytes = 0;
size_t num_bytes = 0;
// Write to the pipe until it is full.
- while (write_bytes < buf_size) {
+ while (write_bytes + write_chunk_size <= buf_size) {
Status error =
pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
std::chrono::milliseconds(10), num_bytes);
if (error.Fail())
- break; // The write buffer is full
+ break; // The write buffer is full.
write_bytes += num_bytes;
}
- ASSERT_TRUE(write_bytes + write_chunk_size <= buf_size);
+ ASSERT_LE(write_bytes + write_chunk_size, buf_size)
+ << "Pipe buffer larger than expected";
// Attempt a write with a long timeout.
auto start_time = std::chrono::steady_clock::now();
- ASSERT_THAT_ERROR(
- pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
- std::chrono::milliseconds(2000), num_bytes)
- .ToError(),
- llvm::Failed());
- auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(
- std::chrono::steady_clock::now() - start_time)
- .count();
- ASSERT_GE(dur, 2000);
+ ASSERT_THAT_ERROR(pipe.WriteWithTimeout(write_ptr + write_bytes,
+ write_chunk_size,
+ std::chrono::seconds(2), num_bytes)
+ .ToError(),
+ llvm::Failed());
+ auto dur = std::chrono::steady_clock::now() - start_time;
+ ASSERT_GE(dur, std::chrono::seconds(2));
- // Attempt a write with a short timeout
+ // Attempt a write with a short timeout.
start_time = std::chrono::steady_clock::now();
ASSERT_THAT_ERROR(
pipe.WriteWithTimeout(write_ptr + write_bytes, write_chunk_size,
std::chrono::milliseconds(200), num_bytes)
.ToError(),
llvm::Failed());
- dur = std::chrono::duration_cast<std::chrono::milliseconds>(
- std::chrono::steady_clock::now() - start_time)
- .count();
- ASSERT_GE(dur, 200);
- ASSERT_LT(dur, 300);
+ dur = std::chrono::steady_clock::now() - start_time;
+ ASSERT_GE(dur, std::chrono::milliseconds(200));
+ ASSERT_LT(dur, std::chrono::seconds(2));
- // Drain the pipe
+ // Drain the pipe.
while (read_bytes < write_bytes) {
ASSERT_THAT_ERROR(
pipe.ReadWithTimeout(read_ptr + read_bytes, write_bytes - read_bytes,
More information about the lldb-commits
mailing list