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

base/sync_socket_win.cc

Index: base/sync_socket_win.cc
diff --git a/base/sync_socket_win.cc b/base/sync_socket_win.cc
index c6fb1ce789c4b7fe596f8cccd99e704ec1d0713c..e4ff732d386f44dac5b707a61839bf384abed442 100644
--- a/base/sync_socket_win.cc
+++ b/base/sync_socket_win.cc
@@ -146,6 +146,11 @@ size_t CancelableFileOperation(Function operation, HANDLE file,
         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;
@@ -234,9 +239,21 @@ bool CancelableSyncSocket::Close() {
 }
 
 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) {