Fix instability in SSL client/server sockets

net/socket/ssl_server_socket_nss.cc

 // Copyright (c) 2011 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 "net/socket/ssl_server_socket_nss.h"
 
 #if defined(OS_WIN)
 #include <winsock2.h>
 #endif
 
@@ skipping 407 matching lines @@
 
   if (!completed_handshake_)
     return;
 
   if (user_write_buf_) {
     int rv = DoWriteLoop(result);
     if (rv != ERR_IO_PENDING)
       DoWriteCallback(rv);
   } else {
     // Ensure that any queued ciphertext is flushed.
-    DoTransportIO();
+    bool network_moved = false;
+    do {
+      network_moved = DoTransportIO();
+    } while (network_moved);

[Wez, 2011/07/18 21:53:33]

I take it that

while (DoTransportIO()) { }

is bad style?

[Sergey Ulanov, 2011/07/19 19:26:56]

reverted this change

   }
 }
 
 void SSLServerSocketNSS::OnRecvComplete(int result) {
   if (next_handshake_state_ == STATE_HANDSHAKE) {
     // In handshake phase.
     OnHandshakeIOComplete(result);
     return;
   }
 
@@ skipping 89 matching lines @@
   OnRecvComplete(result);
 }
 
 // Do network I/O between the given buffer and the given socket.
 // Return true if some I/O performed, false otherwise (error or ERR_IO_PENDING)
 bool SSLServerSocketNSS::DoTransportIO() {
   bool network_moved = false;
   if (nss_bufs_ != NULL) {
     int nsent = BufferSend();
     int nreceived = BufferRecv();
-    network_moved = (nsent > 0 || nreceived >= 0);
+    network_moved = (nsent > 0 || nreceived > 0);
   }
   return network_moved;
 }
 
 int SSLServerSocketNSS::DoPayloadRead() {
   DCHECK(user_read_buf_);
   DCHECK_GT(user_read_buf_len_, 0);
   int rv = PR_Read(nss_fd_, user_read_buf_->data(), user_read_buf_len_);
   if (rv >= 0)
     return rv;
@@ skipping 67 matching lines @@
                       make_scoped_refptr(new SSLErrorParams(rv, 0)));
     return rv;
   }
 
   bool network_moved;
   int rv;
   do {
     rv = DoPayloadRead();
     network_moved = DoTransportIO();
   } while (rv == ERR_IO_PENDING && network_moved);
+
   return rv;
 }
 
 int SSLServerSocketNSS::DoWriteLoop(int result) {
   DCHECK(completed_handshake_);
   DCHECK(next_handshake_state_ == STATE_NONE);
 
   if (result < 0)
     return result;
 
   if (!nss_bufs_) {
     LOG(DFATAL) << "!nss_bufs_";
     int rv = ERR_UNEXPECTED;
     net_log_.AddEvent(NetLog::TYPE_SSL_WRITE_ERROR,
                       make_scoped_refptr(new SSLErrorParams(rv, 0)));
     return rv;
   }
 
   bool network_moved;
-  int rv;
+  int rv = ERR_IO_PENDING;
   do {
-    rv = DoPayloadWrite();
+    if (rv == ERR_IO_PENDING)
+      rv = DoPayloadWrite();
     network_moved = DoTransportIO();
-  } while (rv == ERR_IO_PENDING && network_moved);
+  } while (network_moved);

[Wez, 2011/07/18 21:53:33]

The loop now won't exit on failure (or success!) of DoPayloadWrite.

[Sergey Ulanov, 2011/07/19 19:26:56]

Same as in client sockets: why can't we continue pushing data to the transport
socket? It won't try calling DoPayloadWrite again after it returned anything
other than ERR_IO_PENDING.

+
   return rv;
 }
 
 int SSLServerSocketNSS::DoHandshake() {
   int net_error = net::OK;
   SECStatus rv = SSL_ForceHandshake(nss_fd_);
 
   if (rv == SECSuccess) {
     completed_handshake_ = true;
   } else {
@@ skipping 81 matching lines @@
   // We must call EnsureOCSPInit() here, on the IO thread, to get the IO loop
   // by MessageLoopForIO::current().
   // X509Certificate::Verify() runs on a worker thread of CertVerifier.
   EnsureOCSPInit();
 #endif
 
   return OK;
 }
 
 }  // namespace net