Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1242)

Unified Diff: net/base/tcp_client_socket_pool.cc

Issue 128001: Revert "Make TCPClientSocketPool own the ConnectingSockets." (Closed)
Patch Set: Created 11 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/base/tcp_client_socket_pool.h ('k') | net/url_request/url_request_unittest.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/base/tcp_client_socket_pool.cc
diff --git a/net/base/tcp_client_socket_pool.cc b/net/base/tcp_client_socket_pool.cc
index d35c71a997bfa7ba1e73e88a87f5080ad387af4c..7dadd4cd4261761727b8fa9e3a0c33a6deabcd0c 100644
--- a/net/base/tcp_client_socket_pool.cc
+++ b/net/base/tcp_client_socket_pool.cc
@@ -45,15 +45,20 @@ TCPClientSocketPool::ConnectingSocket::ConnectingSocket(
callback_(this,
&TCPClientSocketPool::ConnectingSocket::OnIOComplete)),
pool_(pool),
- resolver_(pool->GetHostResolver()) {}
+ resolver_(pool->GetHostResolver()),
+ canceled_(false) {
+ CHECK(!ContainsKey(pool_->connecting_socket_map_, handle));
+ pool_->connecting_socket_map_[handle] = this;
+}
TCPClientSocketPool::ConnectingSocket::~ConnectingSocket() {
- // We don't worry about cancelling the host resolution and TCP connect, since
- // ~SingleRequestHostResolver and ~ClientSocket will take care of it.
+ if (!canceled_)
+ pool_->connecting_socket_map_.erase(handle_);
}
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 */);
@@ -68,14 +73,30 @@ 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_);
- CHECK(group_it != pool_->group_map_.end());
+ if (group_it == pool_->group_map_.end()) {
+ // The request corresponding to this ConnectingSocket has been canceled.
+ // Stop bothering with it.
+ delete this;
+ return result;
+ }
Group& group = group_it->second;
RequestMap* request_map = &group.connecting_requests;
RequestMap::iterator it = request_map->find(handle_);
- CHECK(it != request_map->end());
+ if (it == request_map->end()) {
+ // The request corresponding to this ConnectingSocket has been canceled.
+ // Stop bothering with it.
+ delete this;
+ return result;
+ }
if (result == OK && it->second.load_state == LOAD_STATE_RESOLVING_HOST) {
it->second.load_state = LOAD_STATE_CONNECTING;
@@ -87,7 +108,6 @@ int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
}
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_;
@@ -121,10 +141,17 @@ int TCPClientSocketPool::ConnectingSocket::OnIOCompleteInternal(
if (!synchronous)
request.callback->Run(result);
- pool_->RemoveConnectingSocket(handle_); // will delete |this|.
+ 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,
@@ -141,7 +168,6 @@ TCPClientSocketPool::~TCPClientSocketPool() {
// to the manager being destroyed.
CloseIdleSockets();
DCHECK(group_map_.empty());
- DCHECK(connecting_socket_map_.empty());
}
// InsertRequestIntoQueue inserts the request into the queue based on
@@ -193,16 +219,20 @@ int TCPClientSocketPool::RequestSocket(
// 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;
- CHECK(!ContainsKey(connecting_socket_map_, handle));
-
+ // connecting_socket will delete itself.
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;
}
@@ -227,8 +257,6 @@ void TCPClientSocketPool::CancelRequest(const std::string& group_name,
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--;
@@ -391,12 +419,4 @@ void TCPClientSocketPool::DoReleaseSocket(const std::string& group_name,
}
}
-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
« no previous file with comments | « net/base/tcp_client_socket_pool.h ('k') | net/url_request/url_request_unittest.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698