Add SyncSocket::ReceiveWithTimeout() and SyncSocket unit tests.

base/sync_socket_posix.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/sync_socket.h"
 
 #include <errno.h>
 #include <limits.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 
 #if defined(OS_SOLARIS)
 #include <sys/filio.h>
 #endif
 
 #include "base/file_util.h"
 #include "base/logging.h"
 
-
 namespace base {
 
 namespace {
 // To avoid users sending negative message lengths to Send/Receive
 // we clamp message lengths, which are size_t, to no more than INT_MAX.
 const size_t kMaxMessageLength = static_cast<size_t>(INT_MAX);
 
 }  // namespace
 
 const SyncSocket::Handle SyncSocket::kInvalidHandle = -1;
@@ skipping 53 matching lines @@
     return false;
   }
   int retval = HANDLE_EINTR(close(handle_));
   if (retval < 0)
     DPLOG(ERROR) << "close";
   handle_ = kInvalidHandle;
   return (retval == 0);
 }
 
 size_t SyncSocket::Send(const void* buffer, size_t length) {
+  DCHECK_GT(length, 0u);
   DCHECK_LE(length, kMaxMessageLength);
   const char* charbuffer = static_cast<const char*>(buffer);
   int len = file_util::WriteFileDescriptor(handle_, charbuffer, length);
 
   return (len == -1) ? 0 : static_cast<size_t>(len);
 }
 
 size_t SyncSocket::Receive(void* buffer, size_t length) {
+  DCHECK_GT(length, 0u);
   DCHECK_LE(length, kMaxMessageLength);
   char* charbuffer = static_cast<char*>(buffer);
   if (file_util::ReadFromFD(handle_, charbuffer, length))
     return length;
   return 0;
 }
 
+size_t SyncSocket::ReceiveWithTimeout(void* buffer,
+                                      size_t length,
+                                      TimeDelta timeout) {
+  DCHECK_GT(length, 0u);
+  DCHECK_LE(length, kMaxMessageLength);
+
+  // Only timeouts greater than zero and less than one second are allowed.
+  DCHECK_GT(timeout.InMicroseconds(), 0);
+  DCHECK(timeout.InMicroseconds() < Time::kMicrosecondsPerSecond);
+
+  // Track the start time so we can reduce the timeout as data is read.
+  TimeTicks start_time = base::TimeTicks::Now();

[DaleCurtis, 2013/09/25 22:48:25]

Is this worth doing? TimeTicks::Now() has a resolution of 1 to 15ms, yet the
timeouts we'll be using are  1 to 20ms.

+
+  fd_set rfds;
+  size_t bytes_remaining = length;
+  do {
+    FD_ZERO(&rfds);
+    FD_SET(handle_, &rfds);
+
+    // Wait for data to become available.
+    struct timeval timeout_struct = { 0, timeout.InMicroseconds() };
+    const int select_result = HANDLE_EINTR(
+        select(handle_ + 1, &rfds, NULL, NULL, &timeout_struct));
+    if (select_result <= 0)
+      return length - bytes_remaining;
+
+    // select() only tells us that data is ready for reading, not how much.  We
+    // must Peek() for the amount ready for reading to avoid blocking.
+    DCHECK(FD_ISSET(handle_, &rfds));
+    const size_t bytes_to_read = std::min(Peek(), length);
+    const size_t bytes_received = Receive(buffer, bytes_to_read);
+    if (bytes_received != bytes_to_read)
+      return length - bytes_remaining;
+
+    // Since TimeTicks::Now() is expensive, only bother updating the tracking
+    // variables if we have more work to do.
+    if (bytes_remaining -= bytes_received) {
+      buffer = static_cast<uint8_t*>(buffer) + bytes_received;
+      timeout -= base::TimeTicks::Now() - start_time;
+    }
+  } while (bytes_remaining > 0 && timeout > base::TimeDelta());
+  return length;
+}
+
 size_t SyncSocket::Peek() {
   int number_chars;
   if (-1 == ioctl(handle_, FIONREAD, &number_chars)) {
     // If there is an error in ioctl, signal that the channel would block.
     return 0;
   }
-  return (size_t) number_chars;
+  DCHECK_GE(number_chars, 0);
+  return number_chars;
 }
 
 CancelableSyncSocket::CancelableSyncSocket() {}
 CancelableSyncSocket::CancelableSyncSocket(Handle handle)
     : SyncSocket(handle) {
 }
 
 bool CancelableSyncSocket::Shutdown() {
   return HANDLE_EINTR(shutdown(handle(), SHUT_RDWR)) >= 0;
 }
 
 size_t CancelableSyncSocket::Send(const void* buffer, size_t length) {
   long flags = 0;

[DaleCurtis, 2013/09/25 22:48:25]

While coding I began to wonder if this is necessary to do for every Send() call.
 Or should it be a part of the constructor and CreatePair function.  tommi:
WDYT?

[tommi (sloooow) - chröme, 2013/09/29 09:15:54]

you mean set the socket to non-blocking inside CreatePair?  Sgtm... wonder why
that wasn't done - do we ever accept sockets created outside of this class?

[DaleCurtis, 2013/09/30 18:00:51]

Not that I know of or can find.  After this lands I'll create a follow up CL to
move the async send settings to CreatePair().

   flags = fcntl(handle_, F_GETFL, NULL);
   if (flags != -1 && (flags & O_NONBLOCK) == 0) {
     // Set the socket to non-blocking mode for sending if its original mode
     // is blocking.
     fcntl(handle_, F_SETFL, flags | O_NONBLOCK);
   }
 
   size_t len = SyncSocket::Send(buffer, length);
 
   if (flags != -1 && (flags & O_NONBLOCK) == 0) {
     // Restore the original flags.
     fcntl(handle_, F_SETFL, flags);
   }
 
   return len;
 }
 
 // static
 bool CancelableSyncSocket::CreatePair(CancelableSyncSocket* socket_a,
                                       CancelableSyncSocket* socket_b) {
   return SyncSocket::CreatePair(socket_a, socket_b);
 }
 
 }  // namespace base