Take 2. Make TCPClientSocketPool own the ConnectingSockets.

net/base/tcp_client_socket_pool.cc

 // Copyright (c) 2006-2008 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/base/tcp_client_socket_pool.h"
 
 #include "base/compiler_specific.h"
 #include "base/field_trial.h"
 #include "base/message_loop.h"
 #include "base/time.h"
@@ skipping 27 matching lines @@
     const ClientSocketHandle* handle,
     ClientSocketFactory* client_socket_factory,
     TCPClientSocketPool* pool)
     : group_name_(group_name),
       handle_(handle),
       client_socket_factory_(client_socket_factory),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           callback_(this,
                     &TCPClientSocketPool::ConnectingSocket::OnIOComplete)),
       pool_(pool),
-      resolver_(pool->GetHostResolver()),
-      canceled_(false) {
-  CHECK(!ContainsKey(pool_->connecting_socket_map_, handle));
-  pool_->connecting_socket_map_[handle] = this;
-}
+      resolver_(pool->GetHostResolver()) {}
 
 TCPClientSocketPool::ConnectingSocket::~ConnectingSocket() {
-  if (!canceled_)
-    pool_->connecting_socket_map_.erase(handle_);
+  // We don't worry about cancelling the host resolution and TCP connect, since
+  // ~SingleRequestHostResolver and ~ClientSocket will take care of it.
 }
 
 int TCPClientSocketPool::ConnectingSocket::Connect(
     const HostResolver::RequestInfo& resolve_info) {
-  CHECK(!canceled_);
   int rv = resolver_.Resolve(resolve_info, &addresses_, &callback_);
   if (rv != ERR_IO_PENDING)
     rv = OnIOCompleteInternal(rv, true /* synchronous */);
   return rv;
 }
 
 void TCPClientSocketPool::ConnectingSocket::OnIOComplete(int result) {
   OnIOCompleteInternal(result, false /* asynchronous */);
 }
 
 int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
     int result, bool synchronous) {
   CHECK(result != ERR_IO_PENDING);
 
-  if (canceled_) {
-    // We got canceled, so bail out.
-    delete this;
-    return result;
-  }
-
   GroupMap::iterator group_it = pool_->group_map_.find(group_name_);
-  if (group_it == pool_->group_map_.end()) {
-    // The request corresponding to this ConnectingSocket has been canceled.
-    // Stop bothering with it.
-    delete this;
-    return result;
-  }
+  CHECK(group_it != pool_->group_map_.end());
 
   Group& group = group_it->second;
 
   RequestMap* request_map = &group.connecting_requests;
   RequestMap::iterator it = request_map->find(handle_);
-  if (it == request_map->end()) {
-    // The request corresponding to this ConnectingSocket has been canceled.
-    // Stop bothering with it.
-    delete this;
-    return result;
-  }
+  CHECK(it != request_map->end());
 
   if (result == OK && it->second.load_state == LOAD_STATE_RESOLVING_HOST) {
     it->second.load_state = LOAD_STATE_CONNECTING;
     socket_.reset(client_socket_factory_->CreateTCPClientSocket(addresses_));
     connect_start_time_ = base::Time::Now();
     result = socket_->Connect(&callback_);
     if (result == ERR_IO_PENDING)
       return result;
   }
 
   if (result == OK) {
+    CHECK(it->second.load_state == LOAD_STATE_CONNECTING);
     CHECK(connect_start_time_ != base::Time());
     base::TimeDelta connect_duration =
         base::Time::Now() - connect_start_time_;
 
     UMA_HISTOGRAM_CLIPPED_TIMES("Net.TCP_Connection_Latency",
         connect_duration,
         base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMinutes(10),
         100);
   }
@@ skipping 11 matching lines @@
     group.active_socket_count--;
 
     // Delete group if no longer needed.
     if (group.active_socket_count == 0 && group.idle_sockets.empty()) {
       CHECK(group.pending_requests.empty());
       CHECK(group.connecting_requests.empty());
       pool_->group_map_.erase(group_it);
     }
   }
 
+  pool_->RemoveConnectingSocket(handle_);  // will delete |this|.
+
   if (!synchronous)
     request.callback->Run(result);
-  delete this;
   return result;
 }
 
-void TCPClientSocketPool::ConnectingSocket::Cancel() {
-  CHECK(!canceled_);
-  CHECK(ContainsKey(pool_->connecting_socket_map_, handle_));
-  pool_->connecting_socket_map_.erase(handle_);
-  canceled_ = true;
-}
-
 TCPClientSocketPool::TCPClientSocketPool(
     int max_sockets_per_group,
     HostResolver* host_resolver,
     ClientSocketFactory* client_socket_factory)
     : client_socket_factory_(client_socket_factory),
       idle_socket_count_(0),
       max_sockets_per_group_(max_sockets_per_group),
       host_resolver_(host_resolver) {
 }
 
 TCPClientSocketPool::~TCPClientSocketPool() {
   // Clean up any idle sockets.  Assert that we have no remaining active
   // sockets or pending requests.  They should have all been cleaned up prior
   // to the manager being destroyed.
   CloseIdleSockets();
   DCHECK(group_map_.empty());
+  DCHECK(connecting_socket_map_.empty());
 }
 
 // InsertRequestIntoQueue inserts the request into the queue based on
 // priority.  Highest priorities are closest to the front.  Older requests are
 // prioritized over requests of equal priority.
 //
 // static
 void TCPClientSocketPool::InsertRequestIntoQueue(
     const Request& r, RequestQueue* pending_requests) {
   RequestQueue::iterator it = pending_requests->begin();
@@ skipping 31 matching lines @@
       // We found one we can reuse!
       handle->set_socket(idle_socket.socket);
       handle->set_is_reused(true);
       return OK;
     }
     delete idle_socket.socket;
   }
 
   // We couldn't find a socket to reuse, so allocate and connect a new one.
 
-  // First, we need to make sure we aren't already servicing a request for this
-  // handle (which could happen if we requested, canceled, and then requested
-  // with the same handle).
-  if (ContainsKey(connecting_socket_map_, handle))
-    connecting_socket_map_[handle]->Cancel();
-
   CHECK(callback);
   Request r(handle, callback, priority, resolve_info,
             LOAD_STATE_RESOLVING_HOST);
   group_map_[group_name].connecting_requests[handle] = r;
 
-  // connecting_socket will delete itself.
+  CHECK(!ContainsKey(connecting_socket_map_, handle));
+
   ConnectingSocket* connecting_socket =
       new ConnectingSocket(group_name, handle, client_socket_factory_, this);
+  connecting_socket_map_[handle] = connecting_socket;
   int rv = connecting_socket->Connect(resolve_info);
   return rv;
 }
 
 void TCPClientSocketPool::CancelRequest(const std::string& group_name,
                                         const ClientSocketHandle* handle) {
   CHECK(ContainsKey(group_map_, group_name));
 
   Group& group = group_map_[group_name];
 
   // Search pending_requests for matching handle.
   RequestQueue::iterator it = group.pending_requests.begin();
   for (; it != group.pending_requests.end(); ++it) {
     if (it->handle == handle) {
       group.pending_requests.erase(it);
       return;
     }
   }
 
   // It's invalid to cancel a non-existent request.
   CHECK(ContainsKey(group.connecting_requests, handle));
 
   RequestMap::iterator map_it = group.connecting_requests.find(handle);
   if (map_it != group.connecting_requests.end()) {
+    RemoveConnectingSocket(handle);
+
     group.connecting_requests.erase(map_it);
     group.active_socket_count--;
 
     // Delete group if no longer needed.
     if (group.active_socket_count == 0 && group.idle_sockets.empty()) {
       CHECK(group.pending_requests.empty());
       CHECK(group.connecting_requests.empty());
       group_map_.erase(group_name);
     }
   }
@@ skipping 142 matching lines @@
   }
 
   // Delete group if no longer needed.
   if (group.active_socket_count == 0 && group.idle_sockets.empty()) {
     CHECK(group.pending_requests.empty());
     CHECK(group.connecting_requests.empty());
     group_map_.erase(i);
   }
 }
 
+void TCPClientSocketPool::RemoveConnectingSocket(
+    const ClientSocketHandle* handle) {
+  ConnectingSocketMap::iterator it = connecting_socket_map_.find(handle);
+  CHECK(it != connecting_socket_map_.end());
+  delete it->second;
+  connecting_socket_map_.erase(it);
+}
+
 }  // namespace net