From 4baaf9d863a0eb967122ba704d8ba36aa77a81bb Mon Sep 17 00:00:00 2001 From: "willchan@chromium.org" Date: Tue, 31 Aug 2010 15:15:44 +0000 Subject: ClientSocketPoolBaseHelper can try to read an invalid iterator. Fix that. If we hit the backup socket timer and there is no pending request, then the code still tries to create a backup socket using pending_requests.begin(). We change the code to cancel the backup socket timer when the pending_requests hit zero. We also check before reading pending_requests.begin(). BUG=53860 TEST=ClientsocketPoolBaseTest.CancelBackupSocketWhenThereAreNoRequests Review URL: http://codereview.chromium.org/3247007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57993 0039d316-1c4b-4281-b951-d872f2087c98 --- net/socket/client_socket_pool_base.cc | 9 +++++++++ net/socket/client_socket_pool_base_unittest.cc | 28 ++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc index 49fa09e..379c041 100644 --- a/net/socket/client_socket_pool_base.cc +++ b/net/socket/client_socket_pool_base.cc @@ -336,6 +336,10 @@ void ClientSocketPoolBaseHelper::CancelRequest( RemoveConnectJob(*group->jobs().begin(), group); CheckForStalledSocketGroups(); } + + // If there are no more requests, we kill the backup timer. + if (group->pending_requests().empty()) + group->CleanupBackupJob(); break; } } @@ -890,6 +894,11 @@ void ClientSocketPoolBaseHelper::Group::OnBackupSocketTimerFired( return; } + if (pending_requests_.empty()) { + LOG(DFATAL) << "No pending request for backup job."; + return; + } + ConnectJob* backup_job = pool->connect_job_factory_->NewConnectJob( group_name, **pending_requests_.begin(), pool); backup_job->net_log().AddEvent(NetLog::TYPE_SOCKET_BACKUP_CREATED, NULL); diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc index ff67e3b..71074a9 100644 --- a/net/socket/client_socket_pool_base_unittest.cc +++ b/net/socket/client_socket_pool_base_unittest.cc @@ -1940,8 +1940,8 @@ TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) { CreatePool(kDefaultMaxSockets, kDefaultMaxSockets); pool_->EnableConnectBackupJobs(); - // Create the first socket and set to ERR_IO_PENDING. This creates a - // backup job. + // Create the first socket and set to ERR_IO_PENDING. This starts the backup + // timer. connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob); ClientSocketHandle handle; TestCompletionCallback callback; @@ -1969,6 +1969,30 @@ TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) { EXPECT_EQ(kDefaultMaxSockets, client_socket_factory_.allocation_count()); } +TEST_F(ClientSocketPoolBaseTest, CancelBackupSocketWhenThereAreNoRequests) { + CreatePool(kDefaultMaxSockets, kDefaultMaxSockets); + pool_->EnableConnectBackupJobs(); + + // Create the first socket and set to ERR_IO_PENDING. This starts the backup + // timer. + connect_job_factory_->set_job_type(TestConnectJob::kMockWaitingJob); + ClientSocketHandle handle; + TestCompletionCallback callback; + EXPECT_EQ(ERR_IO_PENDING, handle.Init("bar", params_, kDefaultPriority, + &callback, pool_, BoundNetLog())); + ASSERT_TRUE(pool_->HasGroup("bar")); + EXPECT_EQ(1, pool_->NumConnectJobsInGroup("bar")); + + // Cancel the socket request. This should cancel the backup timer. Wait for + // the backup time to see if it indeed got canceled. + handle.Reset(); + // Wait for the backup timer to fire (add some slop to ensure it fires) + PlatformThread::Sleep(ClientSocketPool::kMaxConnectRetryIntervalMs / 2 * 3); + MessageLoop::current()->RunAllPending(); + ASSERT_TRUE(pool_->HasGroup("bar")); + EXPECT_EQ(1, pool_->NumConnectJobsInGroup("bar")); +} + // Test delayed socket binding for the case where we have two connects, // and while one is waiting on a connect, the other frees up. // The socket waiting on a connect should switch immediately to the freed -- cgit v1.1