diff options
-rw-r--r-- | net/base/net_error_list.h | 4 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base.cc | 20 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base.h | 5 | ||||
-rw-r--r-- | net/socket/client_socket_pool_base_unittest.cc | 75 |
4 files changed, 100 insertions, 4 deletions
diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h index 2998a46..636808c 100644 --- a/net/base/net_error_list.h +++ b/net/base/net_error_list.h @@ -191,7 +191,9 @@ NET_ERROR(SSL_SNAP_START_NPN_MISPREDICTION, -131) // give the user a helpful error message rather than have the connection hang. NET_ERROR(ESET_ANTI_VIRUS_SSL_INTERCEPTION, -132) -// Missing -133. Feel free to reuse in the future. +// We've hit the max socket limit for the socket pool while preconnecting. We +// don't bother trying to preconnect more sockets. +NET_ERROR(PRECONNECT_MAX_SOCKET_LIMIT, -133) // The permission to use the SSL client certificate's private key was denied. NET_ERROR(SSL_CLIENT_AUTH_PRIVATE_KEY_ACCESS_DENIED, -134) diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc index 4f042bb..86ba2dd 100644 --- a/net/socket/client_socket_pool_base.cc +++ b/net/socket/client_socket_pool_base.cc @@ -250,6 +250,8 @@ void ClientSocketPoolBaseHelper::RequestSockets( group->NumActiveSocketSlots() < num_sockets && num_iterations_left > 0 ; num_iterations_left--) { int rv = RequestSocketInternal(group_name, &request); + // TODO(willchan): Possibly check for ERR_PRECONNECT_MAX_SOCKET_LIMIT so we + // can log it into the NetLog. if (rv < 0 && rv != ERR_IO_PENDING) { // We're encountering a synchronous error. Give up. if (!ContainsKey(group_map_, group_name)) @@ -298,7 +300,9 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal( if (ReachedMaxSocketsLimit()) { if (idle_socket_count() > 0) { - CloseOneIdleSocket(); + bool closed = CloseOneIdleSocketExceptInGroup(group); + if (preconnecting && !closed) + return ERR_PRECONNECT_MAX_SOCKET_LIMIT; } else { // We could check if we really have a stalled group here, but it requires // a scan of all groups, so just flip a flag here, and do the check later. @@ -940,10 +944,17 @@ bool ClientSocketPoolBaseHelper::ReachedMaxSocketsLimit() const { } void ClientSocketPoolBaseHelper::CloseOneIdleSocket() { + CloseOneIdleSocketExceptInGroup(NULL); +} + +bool ClientSocketPoolBaseHelper::CloseOneIdleSocketExceptInGroup( + const Group* exception_group) { CHECK_GT(idle_socket_count(), 0); for (GroupMap::iterator i = group_map_.begin(); i != group_map_.end(); ++i) { Group* group = i->second; + if (exception_group == group) + continue; std::list<IdleSocket>* idle_sockets = group->mutable_idle_sockets(); if (!idle_sockets->empty()) { @@ -953,11 +964,14 @@ void ClientSocketPoolBaseHelper::CloseOneIdleSocket() { if (group->IsEmpty()) RemoveGroup(i); - return; + return true; } } - LOG(DFATAL) << "No idle socket found to close!."; + if (!exception_group) + LOG(DFATAL) << "No idle socket found to close!."; + + return false; } void ClientSocketPoolBaseHelper::InvokeUserCallbackLater( diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h index 2e8c618..8e6eb13 100644 --- a/net/socket/client_socket_pool_base.h +++ b/net/socket/client_socket_pool_base.h @@ -480,6 +480,11 @@ class ClientSocketPoolBaseHelper // I'm not sure if we hit this situation often. void CloseOneIdleSocket(); + // Same as CloseOneIdleSocket() except it won't close an idle socket in + // |group|. If |group| is NULL, it is ignored. Returns true if it closed a + // socket. + bool CloseOneIdleSocketExceptInGroup(const Group* group); + // Checks if there are stalled socket groups that should be notified // for possible wakeup. void CheckForStalledSocketGroups(); diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc index fef0a5e..843b6be 100644 --- a/net/socket/client_socket_pool_base_unittest.cc +++ b/net/socket/client_socket_pool_base_unittest.cc @@ -3025,6 +3025,81 @@ TEST_F(ClientSocketPoolBaseTest, PreconnectJobsTakenByNormalRequests) { EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a")); } +// http://crbug.com/64940 regression test. +TEST_F(ClientSocketPoolBaseTest, PreconnectClosesIdleSocketRemovesGroup) { + const int kMaxTotalSockets = 3; + const int kMaxSocketsPerGroup = 2; + CreatePool(kMaxTotalSockets, kMaxSocketsPerGroup); + connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob); + + // Note that group name ordering matters here. "a" comes before "b", so + // CloseOneIdleSocket() will try to close "a"'s idle socket. + + // Set up one idle socket in "a". + ClientSocketHandle handle1; + TestCompletionCallback callback1; + EXPECT_EQ(ERR_IO_PENDING, handle1.Init("a", + params_, + kDefaultPriority, + &callback1, + pool_.get(), + BoundNetLog())); + + ASSERT_EQ(OK, callback1.WaitForResult()); + handle1.Reset(); + EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a")); + + // Set up two active sockets in "b". + ClientSocketHandle handle2; + TestCompletionCallback callback2; + EXPECT_EQ(ERR_IO_PENDING, handle1.Init("b", + params_, + kDefaultPriority, + &callback1, + pool_.get(), + BoundNetLog())); + EXPECT_EQ(ERR_IO_PENDING, handle2.Init("b", + params_, + kDefaultPriority, + &callback2, + pool_.get(), + BoundNetLog())); + + ASSERT_EQ(OK, callback1.WaitForResult()); + ASSERT_EQ(OK, callback2.WaitForResult()); + EXPECT_EQ(0, pool_->IdleSocketCountInGroup("b")); + EXPECT_EQ(2, pool_->NumActiveSocketsInGroup("b")); + + // Now we have 1 idle socket in "a" and 2 active sockets in "b". This means + // we've maxed out on sockets, since we set |kMaxTotalSockets| to 3. + // Requesting 2 preconnected sockets for "a" should fail to allocate any more + // sockets for "a", and "b" should still have 2 active sockets. + + pool_->RequestSockets("a", ¶ms_, 2, BoundNetLog()); + EXPECT_EQ(0, pool_->NumConnectJobsInGroup("a")); + EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a")); + EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("a")); + EXPECT_EQ(0, pool_->NumConnectJobsInGroup("b")); + EXPECT_EQ(0, pool_->IdleSocketCountInGroup("b")); + EXPECT_EQ(2, pool_->NumActiveSocketsInGroup("b")); + + // Now release the 2 active sockets for "b". This will give us 1 idle socket + // in "a" and 2 idle sockets in "b". Requesting 2 preconnected sockets for + // "a" should result in closing 1 for "b". + handle1.Reset(); + handle2.Reset(); + EXPECT_EQ(2, pool_->IdleSocketCountInGroup("b")); + EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("b")); + + pool_->RequestSockets("a", ¶ms_, 2, BoundNetLog()); + EXPECT_EQ(1, pool_->NumConnectJobsInGroup("a")); + EXPECT_EQ(1, pool_->IdleSocketCountInGroup("a")); + EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("a")); + EXPECT_EQ(0, pool_->NumConnectJobsInGroup("b")); + EXPECT_EQ(1, pool_->IdleSocketCountInGroup("b")); + EXPECT_EQ(0, pool_->NumActiveSocketsInGroup("b")); +} + } // namespace } // namespace net |