summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--net/base/net_error_list.h4
-rw-r--r--net/socket/client_socket_pool_base.cc20
-rw-r--r--net/socket/client_socket_pool_base.h5
-rw-r--r--net/socket/client_socket_pool_base_unittest.cc75
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", &params_, 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", &params_, 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