[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 06:03:47 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/2] [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/2] 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());
 }



More information about the lldb-commits mailing list