Limits only 16 new QUIC connections can be opened per socket event for QuicSimpleServer. Fix test f…

net/tools/quic/quic_simple_server.cc

Index: net/tools/quic/quic_simple_server.cc
diff --git a/net/tools/quic/quic_simple_server.cc b/net/tools/quic/quic_simple_server.cc
index 91a000b98e7722b8181b0a12be9da2751bc6af35..9ce920b4894181b0d374018cc8ca0f1fabac93dc 100644
--- a/net/tools/quic/quic_simple_server.cc
+++ b/net/tools/quic/quic_simple_server.cc
@@ -27,6 +27,7 @@ namespace net {
 namespace {
 
 const char kSourceAddressTokenSecret[] = "secret";
+const size_t kNumSessionsToCreatePerSocketEvent = 16;
 
 // Allocate some extra space so we can send an error if the client goes over
 // the limit.
@@ -53,6 +54,7 @@ QuicSimpleServer::QuicSimpleServer(
       read_pending_(false),
       synchronous_read_count_(0),
       read_buffer_(new IOBufferWithSize(kReadBufferSize)),
+      check_packet_buffer_(true),
       weak_factory_(this) {
   Initialize();
 }
@@ -152,12 +154,26 @@ void QuicSimpleServer::StartReading() {
   }
   read_pending_ = true;
 
+  if (check_packet_buffer_) {
+    dispatcher_->ProcessBufferedChlos(kNumSessionsToCreatePerSocketEvent);
+  }
+
+  check_packet_buffer_ = false;
   int result = socket_->RecvFrom(
       read_buffer_.get(), read_buffer_->size(), &client_address_,
       base::Bind(&QuicSimpleServer::OnReadComplete, base::Unretained(this)));
 
   if (result == ERR_IO_PENDING) {
     synchronous_read_count_ = 0;
+    if (dispatcher_->HasChlosBuffered()) {
+      // No more packets to read, but still keep processing buffered packets if
+      // there is any.
+      read_pending_ = false;

[Ryan Hamilton, 2016/09/06 21:18:33]

This looks like a bug. read_pending_ should be true until OnReadComplete() is
called. Otherwise we can end up with multiple reads happening at once.

+      check_packet_buffer_ = true;
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::Bind(&QuicSimpleServer::StartReading,
+                                weak_factory_.GetWeakPtr()));

[Ryan Hamilton, 2016/09/06 21:18:33]

I don't think this is the right method to call. This will end up issuing another
read to the socket, which will crash. 

I think you're going to need to split reading into two phases:

1) ProcessBufferedPackets
2) ReadPacketsFrom the network

When we're done processing buffered packets, if there are still more buffered
packets, we can post a task to process more buffered packets. When we've hit
ERR_IO_PENDING, we can post a task to process more buffered packets.

But we can not read more packets from the network until the last read completed
because of how chrome sockets work.

+    }
     return;
   }
 
@@ -165,10 +181,12 @@ void QuicSimpleServer::StartReading() {
     synchronous_read_count_ = 0;
     // Schedule the processing through the message loop to 1) prevent infinite
     // recursion and 2) avoid blocking the thread for too long.
+    check_packet_buffer_ = true;
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&QuicSimpleServer::OnReadComplete,
                               weak_factory_.GetWeakPtr(), result));
   } else {
+    // No need to check buffer befor following read.

[Ryan Hamilton, 2016/09/06 21:18:33]

nit: "before" not "befor"

That being said, I'm sure sure that this comment add value. We've just read a
packet and need to process it, so calling OnReadComplete seems like the right
choice. It's not obvious to me why we might consider checking the buffer here.
(If we needed to do that, I would think we would have done that before calling
read)

     OnReadComplete(result);
   }
 }