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 <dvassil...@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 <dvassil...@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 <dvassil...@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, _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits