Make the CancellableSyncSocket non-blocking on Send, and blocking on Receive

base/sync_socket_win.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 "base/logging.h"
 #include "base/win/scoped_handle.h"
 
 namespace base {
@@ skipping 96 matching lines @@
 
 // Template function that supports calling ReadFile or WriteFile in an
 // overlapped fashion and waits for IO completion.  The function also waits
 // on an event that can be used to cancel the operation.  If the operation
 // is cancelled, the function returns and closes the relevant socket object.
 template <typename BufferType, typename Function>
 size_t CancelableFileOperation(Function operation, HANDLE file,
                                BufferType* buffer, size_t length,
                                base::WaitableEvent* io_event,
                                base::WaitableEvent* cancel_event,
-                               CancelableSyncSocket* socket) {
+                               CancelableSyncSocket* socket,
+                               const unsigned int timeout_in_ms) {

[tommi (sloooow) - chröme, 2012/04/16 13:06:36]

s/const unsigned int/DWORD
No need for const on integral arguments like these (uncommon in chrome) and
since this is a Windows specific file that already uses DWORD we should keep it
consistent with the API that uses the argument.

[no longer working on chromium, 2012/04/16 15:45:30]

Done.

   // The buffer must be byte size or the length check won't make much sense.
   COMPILE_ASSERT(sizeof(buffer[0]) == sizeof(char), incorrect_buffer_type);
   DCHECK_LE(length, kMaxMessageLength);
 
   OVERLAPPED ol = {0};
   ol.hEvent = io_event->handle();
   size_t count = 0;
   while (count < length) {
     DWORD chunk = GetNextChunkSize(count, length);
     // This is either the ReadFile or WriteFile call depending on whether
     // we're receiving or sending data.
     DWORD len = 0;
     BOOL ok = operation(file, static_cast<BufferType*>(buffer) + count, chunk,
                         &len, &ol);
     if (!ok) {
       if (::GetLastError() == ERROR_IO_PENDING) {
-        base::WaitableEvent* events[] = { io_event, cancel_event };
-        size_t signaled = WaitableEvent::WaitMany(events, arraysize(events));
-        if (signaled == 1) {
+        HANDLE events[] = { io_event->handle(), cancel_event->handle() };
+        DWORD wait_result = WaitForMultipleObjects(
+            arraysize(events), events, false, timeout_in_ms);
+        if (wait_result == (WAIT_OBJECT_0 + 0)) {
+          GetOverlappedResult(file, &ol, &len, TRUE);
+        } else if (wait_result == (WAIT_OBJECT_0 + 1)) {
           VLOG(1) << "Shutdown was signaled. Closing socket.";
           CancelIo(file);
           socket->Close();
           count = 0;
           break;
         } else {
-          GetOverlappedResult(file, &ol, &len, TRUE);
+          // Timeout happened.

[tommi (sloooow) - chröme, 2012/04/16 13:06:36]

DCHECK_EQ(WAIT_TIMEOUT, wait_result)?

[no longer working on chromium, 2012/04/16 15:45:30]

Done.

+          if (!CancelIo(file)){
+            DLOG(WARNING) << "CancelIo() failed";

[tommi (sloooow) - chröme, 2012/04/16 13:06:36]

Perhaps you should also add to the documentation of Send() that if Send() failed
due to out-of-buffer, that the socket will not be closed in that case?

[no longer working on chromium, 2012/04/16 15:45:30]

Done.

+          }
+          break;
         }
       } else {
-        return (0 < count) ? count : 0;
+        break;
       }
     }
+
+    // Quit the operation if we can't write/read anymore.
+    if (len != chunk)

[tommi (sloooow) - chröme, 2012/04/16 13:06:36]

should we move this check below |count += len;|?
The reason is that if we can partially send a chunk of data, but not all, then
we want to break out of the loop but |count| should still reflect how much was
actually sent.

[no longer working on chromium, 2012/04/16 15:45:30]

Done.

+      break;
+
     count += len;
   }
-  return count;
+
+  return (count > 0) ? count : 0;
 }
 
 }  // namespace
 
 const SyncSocket::Handle SyncSocket::kInvalidHandle = INVALID_HANDLE_VALUE;
 
 SyncSocket::SyncSocket() : handle_(kInvalidHandle) {}
 
 SyncSocket::~SyncSocket() {
   Close();
@@ skipping 65 matching lines @@
   return true;
 }
 
 bool CancelableSyncSocket::Close() {
   bool ret = SyncSocket::Close();
   shutdown_event_.Reset();
   return ret;
 }
 
 size_t CancelableSyncSocket::Send(const void* buffer, size_t length) {
-  return CancelableFileOperation(&WriteFile, handle_,
-      reinterpret_cast<const char*>(buffer), length, &file_operation_,
-      &shutdown_event_, this);
+  static const unsigned int kWaitTimeOutInMs = 500;

[tommi (sloooow) - chröme, 2012/04/16 13:06:36]

s/unsigned int/DWORD
(ok in this file since it is Win specific)

[no longer working on chromium, 2012/04/16 15:45:30]

Done.

+  return CancelableFileOperation(
+      &WriteFile, handle_, reinterpret_cast<const char*>(buffer),
+      length, &file_operation_, &shutdown_event_, this, kWaitTimeOutInMs);
 }
 
 size_t CancelableSyncSocket::Receive(void* buffer, size_t length) {
   return CancelableFileOperation(&ReadFile, handle_,
       reinterpret_cast<char*>(buffer), length, &file_operation_,
-      &shutdown_event_, this);
+      &shutdown_event_, this, INFINITE);
 }
 
 // static
 bool CancelableSyncSocket::CreatePair(CancelableSyncSocket* socket_a,
                                       CancelableSyncSocket* socket_b) {
   return CreatePairImpl(&socket_a->handle_, &socket_b->handle_, true);
 }
 
 
 }  // namespace base