Reset internal ClientSocketHandle state on Init() synchronous error (failure to do so could lead to a crash). Also delete a CHECK that was asserting on possibly deleted memory.

BUG=http://crbug.com/15207
TEST=See bug for repro.

Review URL: http://codereview.chromium.org/147155

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19343 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 31 insertions, 12 deletions

diff --git a/net/socket/client_socket_handle.cc b/net/socket/client_socket_handle.cc
index 96dec03..15f99a6 100644
--- a/net/socket/client_socket_handle.cc
+++ b/net/socket/client_socket_handle.cc
@@ -16,8 +16,8 @@ ClientSocketHandle::ClientSocketHandle(ClientSocketPool* pool)
     : pool_(pool),
       socket_(NULL),
       is_reused_(false),
-    ALLOW_THIS_IN_INITIALIZER_LIST(
-        callback_(this, &ClientSocketHandle::OnIOComplete)) {}
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          callback_(this, &ClientSocketHandle::OnIOComplete)) {}
 
 ClientSocketHandle::~ClientSocketHandle() {
   Reset();
@@ -29,9 +29,14 @@ int ClientSocketHandle::Init(const std::string& group_name,
                              CompletionCallback* callback) {
   ResetInternal(true);
   group_name_ = group_name;
-  user_callback_ = callback;
-  return pool_->RequestSocket(
+  int rv = pool_->RequestSocket(
       group_name, resolve_info, priority, this, &callback_);
+  if (rv == ERR_IO_PENDING) {
+    user_callback_ = callback;
+  } else {
+    HandleInitCompletion(rv);
+  }
+  return rv;
 }
 
 void ClientSocketHandle::Reset() {
@@ -62,12 +67,16 @@ LoadState ClientSocketHandle::GetLoadState() const {
 }
 
 void ClientSocketHandle::OnIOComplete(int result) {
-  CHECK(ERR_IO_PENDING != result);
   CompletionCallback* callback = user_callback_;
   user_callback_ = NULL;
+  HandleInitCompletion(result);
+  callback->Run(result);
+}
+
+void ClientSocketHandle::HandleInitCompletion(int result) {
+  CHECK(ERR_IO_PENDING != result);
   if (result != OK)
     ResetInternal(false);  // The request failed, so there's nothing to cancel.
-  callback->Run(result);
 }
 
 }  // namespace net
diff --git a/net/socket/client_socket_handle.h b/net/socket/client_socket_handle.h
index 0f80fa0..9235e71 100644
--- a/net/socket/client_socket_handle.h
+++ b/net/socket/client_socket_handle.h
@@ -79,8 +79,13 @@ class ClientSocketHandle {
   bool is_reused() const { return is_reused_; }
 
  private:
+  // Called on asynchronous completion of an Init() request.
   void OnIOComplete(int result);
 
+  // Called on completion (both asynchronous & synchronous) of an Init()
+  // request.
+  void HandleInitCompletion(int result);
+
   // Resets the state of the ClientSocketHandle.  |cancel| indicates whether or
   // not to try to cancel the request with the ClientSocketPool.
   void ResetInternal(bool cancel);
diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index a67e9c9..4a2770f 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -111,10 +111,7 @@ int ClientSocketPoolBase::RequestSocket(
   ConnectJob* connect_job =
       connect_job_factory_->NewConnectJob(group_name, r, this);
   connect_job_map_[handle] = connect_job;
-  int rv = connect_job->Connect();
-
-  CheckSocketCounts(group);
-  return rv;
+  return connect_job->Connect();
 }
 
 void ClientSocketPoolBase::CancelRequest(const std::string& group_name,
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index a76a439..9cd62e8 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -278,10 +278,14 @@ TEST_F(TCPClientSocketPoolTest, InitConnectionFailure) {
   client_socket_factory_.set_client_socket_type(
       MockClientSocketFactory::MOCK_FAILING_CLIENT_SOCKET);
   TestSocketRequest req(pool_.get(), &request_order_);
-  HostResolver::RequestInfo info("unresolvable.host.name", 80);
+  HostResolver::RequestInfo info("a", 80);
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle.Init("a", info, 5, &req));
   EXPECT_EQ(ERR_CONNECTION_FAILED, req.WaitForResult());
+  // HostCache caches it, so MockFailingClientSocket will cause Init() to
+  // synchronously fail.
+  EXPECT_EQ(ERR_CONNECTION_FAILED,
+            req.handle.Init("a", info, 5, &req));
 }
 
 TEST_F(TCPClientSocketPoolTest, PendingRequests) {
diff --git a/net/socket/tcp_client_socket_pool_unittest.cc b/net/socket/tcp_client_socket_pool_unittest.cc
index a76a439..9cd62e8 100644
--- a/net/socket/tcp_client_socket_pool_unittest.cc
+++ b/net/socket/tcp_client_socket_pool_unittest.cc
@@ -278,10 +278,14 @@ TEST_F(TCPClientSocketPoolTest, InitConnectionFailure) {
   client_socket_factory_.set_client_socket_type(
       MockClientSocketFactory::MOCK_FAILING_CLIENT_SOCKET);
   TestSocketRequest req(pool_.get(), &request_order_);
-  HostResolver::RequestInfo info("unresolvable.host.name", 80);
+  HostResolver::RequestInfo info("a", 80);
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle.Init("a", info, 5, &req));
   EXPECT_EQ(ERR_CONNECTION_FAILED, req.WaitForResult());
+  // HostCache caches it, so MockFailingClientSocket will cause Init() to
+  // synchronously fail.
+  EXPECT_EQ(ERR_CONNECTION_FAILED,
+            req.handle.Init("a", info, 5, &req));
 }
 
 TEST_F(TCPClientSocketPoolTest, PendingRequests) {