[Lldb-commits] [lldb] r242767 - Revert "Add Pipe::WriteWithTimeout method"

Pavel Labath labath at google.com
Tue Jul 21 04:04:52 PDT 2015


Author: labath
Date: Tue Jul 21 06:04:52 2015
New Revision: 242767

URL: http://llvm.org/viewvc/llvm-project?rev=242767&view=rev
Log:
Revert "Add Pipe::WriteWithTimeout method"

I have observed an increased flakyness in the buildbots. I suspect something was relying on the
fact that Pipe::Write had an implicit timeout of 1s, which this commit has removed. Reverting
while I investigate.

Modified:
    lldb/trunk/include/lldb/Host/PipeBase.h
    lldb/trunk/include/lldb/Host/posix/PipePosix.h
    lldb/trunk/include/lldb/Host/windows/PipeWindows.h
    lldb/trunk/source/Host/common/PipeBase.cpp
    lldb/trunk/source/Host/posix/PipePosix.cpp
    lldb/trunk/source/Host/windows/PipeWindows.cpp

Modified: lldb/trunk/include/lldb/Host/PipeBase.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/PipeBase.h?rev=242767&r1=242766&r2=242767&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/PipeBase.h (original)
+++ lldb/trunk/include/lldb/Host/PipeBase.h Tue Jul 21 06:04:52 2015
@@ -49,13 +49,9 @@ class PipeBase
     // Delete named pipe.
     virtual Error Delete(llvm::StringRef name) = 0;
 
-    virtual Error WriteWithTimeout(const void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_written) = 0;
+    virtual Error Write(const void *buf, size_t size, size_t &bytes_written) = 0;
     virtual Error ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_read) = 0;
-
     Error Read(void *buf, size_t size, size_t &bytes_read);
-    Error Write(const void *buf, size_t size, size_t &bytes_written);
-
-    static const std::chrono::microseconds kInfiniteTimeout;
 };
 }
 

Modified: lldb/trunk/include/lldb/Host/posix/PipePosix.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/PipePosix.h?rev=242767&r1=242766&r2=242767&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/posix/PipePosix.h (original)
+++ lldb/trunk/include/lldb/Host/posix/PipePosix.h Tue Jul 21 06:04:52 2015
@@ -74,7 +74,7 @@ public:
     Delete(llvm::StringRef name) override;
 
     Error
-    WriteWithTimeout(const void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_written) override;
+    Write(const void *buf, size_t size, size_t &bytes_written) override;
     Error
     ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_read) override;
 

Modified: lldb/trunk/include/lldb/Host/windows/PipeWindows.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/windows/PipeWindows.h?rev=242767&r1=242766&r2=242767&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/windows/PipeWindows.h (original)
+++ lldb/trunk/include/lldb/Host/windows/PipeWindows.h Tue Jul 21 06:04:52 2015
@@ -49,7 +49,7 @@ class PipeWindows : public PipeBase
 
     Error Delete(llvm::StringRef name) override;
 
-    Error WriteWithTimeout(const void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_written) override;
+    Error Write(const void *buf, size_t size, size_t &bytes_written) override;
     Error ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_read) override;
 
     // PipeWindows specific methods.  These allow access to the underlying OS handle.

Modified: lldb/trunk/source/Host/common/PipeBase.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/PipeBase.cpp?rev=242767&r1=242766&r2=242767&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/PipeBase.cpp (original)
+++ lldb/trunk/source/Host/common/PipeBase.cpp Tue Jul 21 06:04:52 2015
@@ -11,24 +11,17 @@
 
 using namespace lldb_private;
 
-const std::chrono::microseconds PipeBase::kInfiniteTimeout(-1);
 
 PipeBase::~PipeBase() = default;
 
 Error
 PipeBase::OpenAsWriter(llvm::StringRef name, bool child_process_inherit)
 {
-    return OpenAsWriterWithTimeout(name, child_process_inherit, kInfiniteTimeout);
+    return OpenAsWriterWithTimeout(name, child_process_inherit, std::chrono::microseconds::zero());
 }
 
 Error
 PipeBase::Read(void *buf, size_t size, size_t &bytes_read)
 {
-    return ReadWithTimeout(buf, size, kInfiniteTimeout, bytes_read);
-}
-
-Error
-PipeBase::Write(const void *buf, size_t size, size_t &bytes_written)
-{
-    return WriteWithTimeout(buf, size, kInfiniteTimeout, bytes_written);
+    return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(), bytes_read);
 }

Modified: lldb/trunk/source/Host/posix/PipePosix.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/PipePosix.cpp?rev=242767&r1=242766&r2=242767&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/PipePosix.cpp (original)
+++ lldb/trunk/source/Host/posix/PipePosix.cpp Tue Jul 21 06:04:52 2015
@@ -73,8 +73,7 @@ SelectIO(int handle, bool is_read, const
     while (!done)
     {
         struct timeval tv = {0, 0};
-        struct timeval *tv_p = &tv;
-        if (timeout != PipeBase::kInfiniteTimeout)
+        if (timeout != microseconds::zero())
         {
             const auto remaining_dur = duration_cast<microseconds>(finish_time - Now());
             if (remaining_dur.count() <= 0)
@@ -89,7 +88,7 @@ SelectIO(int handle, bool is_read, const
             tv.tv_usec = dur_usecs.count();
         }
         else
-            tv_p = nullptr;
+            tv.tv_sec = 1;
 
         FD_ZERO(&fds);
         FD_SET(handle, &fds);
@@ -97,7 +96,7 @@ SelectIO(int handle, bool is_read, const
         const auto retval = ::select(handle + 1,
                                      (is_read) ? &fds : nullptr,
                                      (is_read) ? nullptr : &fds,
-                                     nullptr, tv_p);
+                                     nullptr, &tv);
         if (retval == -1)
         {
             if (errno == EINTR)
@@ -271,7 +270,7 @@ PipePosix::OpenAsWriterWithTimeout(llvm:
 
     while (!CanWrite())
     {
-        if (timeout != kInfiniteTimeout)
+        if (timeout != microseconds::zero())
         {
             const auto dur = duration_cast<microseconds>(finish_time - Now()).count();
             if (dur <= 0)
@@ -402,10 +401,7 @@ PipePosix::ReadWithTimeout(void *buf, si
 }
 
 Error
-PipePosix::WriteWithTimeout(const void *buf,
-                            size_t size,
-                            const std::chrono::microseconds &timeout,
-                            size_t &bytes_written)
+PipePosix::Write(const void *buf, size_t size, size_t &bytes_written)
 {
     bytes_written = 0;
     if (!CanWrite())
@@ -431,5 +427,5 @@ PipePosix::WriteWithTimeout(const void *
 
                         return error;
                     },
-                    timeout);
+                    std::chrono::microseconds::zero());
 }

Modified: lldb/trunk/source/Host/windows/PipeWindows.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/PipeWindows.cpp?rev=242767&r1=242766&r2=242767&view=diff
==============================================================================
--- lldb/trunk/source/Host/windows/PipeWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/PipeWindows.cpp Tue Jul 21 06:04:52 2015
@@ -289,7 +289,7 @@ PipeWindows::ReadWithTimeout(void *buf,
     if (!result && GetLastError() != ERROR_IO_PENDING)
         return Error(::GetLastError(), eErrorTypeWin32);
 
-    DWORD timeout = (duration == kInfiniteTimeout) ? INFINITE : duration.count() * 1000;
+    DWORD timeout = (duration == std::chrono::microseconds::zero()) ? INFINITE : duration.count() * 1000;
     DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
     if (wait_result != WAIT_OBJECT_0)
     {
@@ -319,10 +319,7 @@ PipeWindows::ReadWithTimeout(void *buf,
 }
 
 Error
-PipeWindows::WriteWithTimeout(const void *buf,
-                              size_t num_bytes,
-                              const std::chrono::microseconds &duration,
-                              size_t &bytes_written)
+PipeWindows::Write(const void *buf, size_t num_bytes, size_t &bytes_written)
 {
     if (!CanWrite())
         return Error(ERROR_INVALID_HANDLE, eErrorTypeWin32);
@@ -332,32 +329,8 @@ PipeWindows::WriteWithTimeout(const void
     if (!write_result && GetLastError() != ERROR_IO_PENDING)
         return Error(::GetLastError(), eErrorTypeWin32);
 
-    DWORD timeout = (duration == kInfiniteTimeout) ? 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;
-        DWORD failure_error = ::GetLastError();
-        if (wait_result == WAIT_TIMEOUT)
-        {
-            BOOL cancel_result = CancelIoEx(m_read, &m_write_overlapped);
-            if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
-                failed = false;
-        }
-        if (failed)
-            return Error(failure_error, eErrorTypeWin32);
-    }
-
-    // Now we call GetOverlappedResult setting bWait to false, since we've already waited
-    // as long as we're willing to.
-    BOOL result = GetOverlappedResult(m_write, &m_write_overlapped, &sys_bytes_written, FALSE);
+    BOOL result = GetOverlappedResult(m_write, &m_write_overlapped, &sys_bytes_written, TRUE);
     if (!result)
         return Error(::GetLastError(), eErrorTypeWin32);
-
-    bytes_written = sys_bytes_written;
     return Error();
 }





More information about the lldb-commits mailing list