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 128 matching lines @@
           socket->Close();
           count = 0;
           break;
         } else {
           GetOverlappedResult(file, &ol, &len, TRUE);
         }
       } else {
         return (0 < count) ? count : 0;
       }
     }
+
+    // Quit the operation if we can't write/read anymore.
+    if (len == 0)

[tommi (sloooow) - chröme, 2012/04/13 12:56:47]

I wonder if we should perhaps widen the check a bit:

  if (len != chunk) {
    LOG(ERROR) << "Pipe buffer is full.  Assuming nobody is receiving.";
    break;
  }

Also, out of curiosity, how big is the buffer?

[no longer working on chromium, 2012/04/16 10:26:45]

My test shows that the size of the buffer is 4098 bytes in windows. It looks
like we don't really need the while loop if we add this check :) Let me know if
you want to remove it too.

[tommi (sloooow) - chröme, 2012/04/16 11:24:57]

I think we still need the loop for larger buffers. Think about the case if we
want to e.g. send a 1MB file over the socket.  (remember that SyncSocket is not
just used for audio's int pending_bytes)

+      break;
+
     count += len;
   }
   return count;
 }
 
 }  // namespace
 
 const SyncSocket::Handle SyncSocket::kInvalidHandle = INVALID_HANDLE_VALUE;
 
 SyncSocket::SyncSocket() : handle_(kInvalidHandle) {}
@@ skipping 68 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);
+  DWORD state = PIPE_NOWAIT;
+  // The default mode for the socket is blocking, set it to non-blocking mode
+  // for sending.
+  state = PIPE_NOWAIT;

[tommi (sloooow) - chröme, 2012/04/13 12:56:47]

remove this line. you've already set the value.

[no longer working on chromium, 2012/04/16 10:26:45]

Done.

+  SetNamedPipeHandleState(handle_, &state, NULL, NULL);

[tommi (sloooow) - chröme, 2012/04/13 12:56:47]

instead of calling this directly, what about doing this:

DWORD original_state = 0;
GetNamedPipeHandleState(handle_, &original_state, ...);
if ((original_state & PIPE_NOWAIT) == 0) {
  DWORD state = original_state | PIPE_NOWAIT;
  SetNamedPipeHandleState(handle_, &state, NULL, NULL);
}

size_t len = CancelableFileOperation(...);

if ((original_state & PIPE_NOWAIT) == 0)
  SetNamedPipeHandleState(handle_, &original_state , NULL, NULL);


Otherwise you could be changing states you're not aware of.

[no longer working on chromium, 2012/04/16 10:26:45]

Use WaitForMany.. instead, as we discussed offline.

+
+  size_t len =  CancelableFileOperation(
+      &WriteFile, handle_, reinterpret_cast<const char*>(buffer),
+      length, &file_operation_, &shutdown_event_, this);
+
+  // Set the socket back to blocking mode.
+  state = PIPE_WAIT;
+  SetNamedPipeHandleState(handle_, &state, NULL, NULL);
+
+  return len;
 }
 
 size_t CancelableSyncSocket::Receive(void* buffer, size_t length) {
   return CancelableFileOperation(&ReadFile, handle_,
       reinterpret_cast<char*>(buffer), length, &file_operation_,
       &shutdown_event_, this);
 }
 
 // static
 bool CancelableSyncSocket::CreatePair(CancelableSyncSocket* socket_a,
                                       CancelableSyncSocket* socket_b) {
   return CreatePairImpl(&socket_a->handle_, &socket_b->handle_, true);
 }
 
 
 }  // namespace base