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>
@@ skipping 90 matching lines @@
 }
 
 size_t SyncSocket::Receive(void* buffer, size_t length) {
   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) {
+  fd_set rfds;
+  FD_ZERO(&rfds);
+  FD_SET(handle_, &rfds);
+
+  DCHECK(timeout.InMicroseconds() < Time::kMicrosecondsPerSecond);

[awong, 2013/09/17 18:54:12]

nit: DCHECK_LT?

[DaleCurtis, 2013/09/17 19:00:25]

Doesn't work for some reason, ends up with a link time error indicating that
kMicrosecondsPerSecond is not defined... Spent some not insignificant time
trying to resolve, but no luck.  Ideas?

[awong, 2013/09/17 19:57:11]

Oh right.  It's cause there isn't a definition for kMicrosecondsPerSecond...just
a declaration (which happens to have a constant-propagatable value).  We *could*
fix this, but why bother.

+  struct timeval timeout_struct = { 0, timeout.InMicroseconds() };
+
+  int select_result = HANDLE_EINTR(
+      select(handle_ + 1, &rfds, NULL, NULL, &timeout_struct));
+
+  if (select_result > 0) {
+    DCHECK(FD_ISSET(handle_, &rfds));
+    DCHECK_GE(Peek(), length);

[awong, 2013/09/17 18:54:12]

Why do we think Peek() can't be less than length? I thought select could return
with fewer bytes than requested...

[DaleCurtis, 2013/09/17 19:00:25]

Correct, I'll remove the condition.  For its intended use case it seems
practically true since we're only sending 4 bytes around at a time.

[awong, 2013/09/17 19:57:11]

Do you want to read the amount from Peek() or the amount from length?

[DaleCurtis, 2013/09/17 20:13:44]

To be perfect this should be a while loop which runs a select() with a
decreasing timeout and reads only Peek() bytes at a time.  Doing this means
adding Peek() and TimeTicks::Now() calls.  Potentially it also breaks down into
multiple Receive calls unnecessarily if the bytes available change between
Peek() and Receive.

What's written works correctly for our simple use case, but may block longer
than expected for future use cases. I think the current code is sufficient, but
am willing to polish to "perfect" if you prefer.

+    return Receive(buffer, length);
+  }
+
+  return 0;
+}
+
 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;
 }
 
 CancelableSyncSocket::CancelableSyncSocket() {}
@@ skipping 24 matching lines @@
   return len;
 }
 
 // static
 bool CancelableSyncSocket::CreatePair(CancelableSyncSocket* socket_a,
                                       CancelableSyncSocket* socket_b) {
   return SyncSocket::CreatePair(socket_a, socket_b);
 }
 
 }  // namespace base