Control the amount of time to leave an unused socket idle before closing it.

This adds constructor arguments for socket idle timeouts.  This allows me to control it for testing, and also makes it possible to run experiments on how long to enable it for.
Currently I've set the timeout for unused sockets to 10 seconds, since that will cover 90% of the TCP RSTs we're seeing.  We can probably increase this, but I'm waiting on histogram data to decide what to change it to.
BUG=http://crbug.com/18192

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

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

4 files changed, 106 insertions, 13 deletions

diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 533d103..18a084b 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -24,9 +24,6 @@ namespace {
 // some conditions.  See http://crbug.com/4606.
 const int kCleanupInterval = 10;  // DO NOT INCREASE THIS TIMEOUT.
 
-// The maximum duration, in seconds, to keep idle persistent sockets alive.
-const int kIdleTimeout = 300;  // 5 minutes.
-
 }  // namespace
 
 namespace net {
@@ -99,12 +96,16 @@ bool ClientSocketPoolBaseHelper::g_late_binding = false;
 ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper(
     int max_sockets,
     int max_sockets_per_group,
+    base::TimeDelta unused_idle_socket_timeout,
+    base::TimeDelta used_idle_socket_timeout,
     ConnectJobFactory* connect_job_factory)
     : idle_socket_count_(0),
       connecting_socket_count_(0),
       handed_out_socket_count_(0),
       max_sockets_(max_sockets),
       max_sockets_per_group_(max_sockets_per_group),
+      unused_idle_socket_timeout_(unused_idle_socket_timeout),
+      used_idle_socket_timeout_(used_idle_socket_timeout),
       may_have_stalled_group_(false),
       connect_job_factory_(connect_job_factory) {
   DCHECK_LE(0, max_sockets_per_group);
@@ -340,9 +341,9 @@ LoadState ClientSocketPoolBaseHelper::GetLoadState(
 }
 
 bool ClientSocketPoolBaseHelper::IdleSocket::ShouldCleanup(
-    base::TimeTicks now) const {
-  bool timed_out = (now - start_time) >=
-      base::TimeDelta::FromSeconds(kIdleTimeout);
+    base::TimeTicks now,
+    base::TimeDelta timeout) const {
+  bool timed_out = (now - start_time) >= timeout;
   return timed_out ||
       !(used ? socket->IsConnectedAndIdle() : socket->IsConnected());
 }
@@ -361,7 +362,9 @@ void ClientSocketPoolBaseHelper::CleanupIdleSockets(bool force) {
 
     std::deque<IdleSocket>::iterator j = group.idle_sockets.begin();
     while (j != group.idle_sockets.end()) {
-      if (force || j->ShouldCleanup(now)) {
+      base::TimeDelta timeout =
+          j->used ? used_idle_socket_timeout_ : unused_idle_socket_timeout_;
+      if (force || j->ShouldCleanup(now, timeout)) {
         delete j->socket;
         j = group.idle_sockets.erase(j);
         DecrementIdleCount();
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index e6368ca..4b5a3b1 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -166,6 +166,8 @@ class ClientSocketPoolBaseHelper
 
   ClientSocketPoolBaseHelper(int max_sockets,
                              int max_sockets_per_group,
+                             base::TimeDelta unused_idle_socket_timeout,
+                             base::TimeDelta used_idle_socket_timeout,
                              ConnectJobFactory* connect_job_factory);
 
   ~ClientSocketPoolBaseHelper();
@@ -217,6 +219,10 @@ class ClientSocketPoolBaseHelper
     return group_map_.find(group_name)->second.jobs.size();
   }
 
+  // Closes all idle sockets if |force| is true.  Else, only closes idle
+  // sockets that timed out or can't be reused.  Made public for testing.
+  void CleanupIdleSockets(bool force);
+
  private:
   // Entry for a persistent socket which became idle at time |start_time|.
   struct IdleSocket {
@@ -227,12 +233,13 @@ class ClientSocketPoolBaseHelper
 
     // An idle socket should be removed if it can't be reused, or has been idle
     // for too long. |now| is the current time value (TimeTicks::Now()).
+    // |timeout| is the length of time to wait before timing out an idle socket.
     //
     // An idle socket can't be reused if it is disconnected or has received
     // data unexpectedly (hence no longer idle).  The unread data would be
     // mistaken for the beginning of the next response if we were to reuse the
     // socket for a new request.
-    bool ShouldCleanup(base::TimeTicks now) const;
+    bool ShouldCleanup(base::TimeTicks now, base::TimeDelta timeout) const;
   };
 
   typedef std::deque<const Request*> RequestQueue;
@@ -274,10 +281,6 @@ class ClientSocketPoolBaseHelper
   static const Request* RemoveRequestFromQueue(RequestQueue::iterator it,
                                                RequestQueue* pending_requests);
 
-  // Closes all idle sockets if |force| is true.  Else, only closes idle
-  // sockets that timed out or can't be reused.
-  void CleanupIdleSockets(bool force);
-
   // Called when the number of idle sockets changes.
   void IncrementIdleCount();
   void DecrementIdleCount();
@@ -359,6 +362,10 @@ class ClientSocketPoolBaseHelper
   // The maximum number of sockets kept per group.
   const int max_sockets_per_group_;
 
+  // The time to wait until closing idle sockets.
+  const base::TimeDelta unused_idle_socket_timeout_;
+  const base::TimeDelta used_idle_socket_timeout_;
+
   // Until the maximum number of sockets limit is reached, a group can only
   // have pending requests if it exceeds the "max sockets per group" limit.
   //
@@ -384,6 +391,14 @@ class ClientSocketPoolBaseHelper
 
 }  // namespace internal
 
+// The maximum duration, in seconds, to keep unused idle persistent sockets
+// alive.
+// TODO(willchan): Change this timeout after getting histogram data on how
+// long it should be.
+static const int kUnusedIdleSocketTimeout = 10;
+// The maximum duration, in seconds, to keep used idle persistent sockets alive.
+static const int kUsedIdleSocketTimeout = 300;  // 5 minutes
+
 template <typename SocketParams>
 class ClientSocketPoolBase {
  public:
@@ -419,11 +434,20 @@ class ClientSocketPoolBase {
     DISALLOW_COPY_AND_ASSIGN(ConnectJobFactory);
   };
 
+  // |max_sockets| is the maximum number of sockets to be maintained by this
+  // ClientSocketPool.  |max_sockets_per_group| specifies the maximum number of
+  // sockets a "group" can have.  |unused_idle_socket_timeout| specifies how
+  // long to leave an unused idle socket open before closing it.
+  // |used_idle_socket_timeout| specifies how long to leave a previously used
+  // idle socket open before closing it.
   ClientSocketPoolBase(int max_sockets,
                        int max_sockets_per_group,
+                       base::TimeDelta unused_idle_socket_timeout,
+                       base::TimeDelta used_idle_socket_timeout,
                        ConnectJobFactory* connect_job_factory)
       : helper_(new internal::ClientSocketPoolBaseHelper(
           max_sockets, max_sockets_per_group,
+          unused_idle_socket_timeout, used_idle_socket_timeout,
           new ConnectJobFactoryAdaptor(connect_job_factory))) {}
 
   ~ClientSocketPoolBase() {}
@@ -485,6 +509,10 @@ class ClientSocketPoolBase {
     return helper_->NumConnectJobsInGroup(group_name);
   }
 
+  void CleanupIdleSockets(bool force) {
+    return helper_->CleanupIdleSockets(force);
+  }
+
  private:
   // This adaptor class exists to bridge the
   // internal::ClientSocketPoolBaseHelper::ConnectJobFactory and
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index fff6869..020bc98 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -250,8 +250,12 @@ class TestClientSocketPool : public ClientSocketPool {
   TestClientSocketPool(
       int max_sockets,
       int max_sockets_per_group,
+      base::TimeDelta unused_idle_socket_timeout,
+      base::TimeDelta used_idle_socket_timeout,
       TestClientSocketPoolBase::ConnectJobFactory* connect_job_factory)
-      : base_(max_sockets, max_sockets_per_group, connect_job_factory) {}
+      : base_(max_sockets, max_sockets_per_group,
+              unused_idle_socket_timeout, used_idle_socket_timeout,
+              connect_job_factory) {}
 
   virtual int RequestSocket(
       const std::string& group_name,
@@ -297,6 +301,8 @@ class TestClientSocketPool : public ClientSocketPool {
     return base_.NumConnectJobsInGroup(group_name);
   }
 
+  void CleanupTimedOutIdleSockets() { base_.CleanupIdleSockets(false); }
+
  private:
   TestClientSocketPoolBase base_;
 
@@ -359,10 +365,23 @@ class ClientSocketPoolBaseTest : public ClientSocketPoolTest {
   ClientSocketPoolBaseTest() {}
 
   void CreatePool(int max_sockets, int max_sockets_per_group) {
+    CreatePoolWithIdleTimeouts(
+        max_sockets,
+        max_sockets_per_group,
+        base::TimeDelta::FromSeconds(kUnusedIdleSocketTimeout),
+        base::TimeDelta::FromSeconds(kUsedIdleSocketTimeout));
+  }
+
+  void CreatePoolWithIdleTimeouts(
+      int max_sockets, int max_sockets_per_group,
+      base::TimeDelta unused_idle_socket_timeout,
+      base::TimeDelta used_idle_socket_timeout) {
     DCHECK(!pool_.get());
     connect_job_factory_ = new TestConnectJobFactory(&client_socket_factory_);
     pool_ = new TestClientSocketPool(max_sockets,
                                      max_sockets_per_group,
+                                     unused_idle_socket_timeout,
+                                     used_idle_socket_timeout,
                                      connect_job_factory_);
   }
 
@@ -1760,6 +1779,47 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding,
   EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
 }
 
+TEST_F(ClientSocketPoolBaseTest_LateBinding, CleanupTimedOutIdleSockets) {
+  CreatePoolWithIdleTimeouts(
+      kDefaultMaxSockets, kDefaultMaxSocketsPerGroup,
+      base::TimeDelta(),  // Time out unused sockets immediately.
+      base::TimeDelta::FromDays(1));  // Don't time out used sockets.
+
+  connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
+
+  // Startup two mock pending connect jobs, which will sit in the MessageLoop.
+
+  TestSocketRequest req(&request_order_, &completion_count_);
+  int rv = InitHandle(req.handle(), "a", 0, &req, pool_.get(), NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(LOAD_STATE_CONNECTING, pool_->GetLoadState("a", req.handle()));
+
+  TestSocketRequest req2(&request_order_, &completion_count_);
+  rv = InitHandle(req2.handle(), "a", 0, &req2, pool_.get(), NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(LOAD_STATE_CONNECTING, pool_->GetLoadState("a", req2.handle()));
+
+  // Cancel one of the requests.  Wait for the other, which will get the first
+  // job.  Release the socket.  Run the loop again to make sure the second
+  // socket is sitting idle and the first one is released (since ReleaseSocket()
+  // just posts a DoReleaseSocket() task).
+
+  req.handle()->Reset();
+  EXPECT_EQ(OK, req2.WaitForResult());
+  req2.handle()->Reset();
+  MessageLoop::current()->RunAllPending();
+
+  ASSERT_EQ(2, pool_->IdleSocketCount());
+  
+  // Invoke the idle socket cleanup check.  Only one socket should be left, the
+  // used socket.  Request it to make sure that it's used.
+
+  pool_->CleanupTimedOutIdleSockets();
+  rv = InitHandle(req.handle(), "a", 0, &req, pool_.get(), NULL);
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(req.handle()->is_reused());
+}
+
 }  // namespace
 
 }  // namespace net
diff --git a/net/socket/tcp_client_socket_pool.cc b/net/socket/tcp_client_socket_pool.cc
index 77a5fad..8a5c4d3 100644
--- a/net/socket/tcp_client_socket_pool.cc
+++ b/net/socket/tcp_client_socket_pool.cc
@@ -146,6 +146,8 @@ TCPClientSocketPool::TCPClientSocketPool(
     HostResolver* host_resolver,
     ClientSocketFactory* client_socket_factory)
     : base_(max_sockets, max_sockets_per_group,
+            base::TimeDelta::FromSeconds(kUnusedIdleSocketTimeout),
+            base::TimeDelta::FromSeconds(kUsedIdleSocketTimeout),
             new TCPConnectJobFactory(client_socket_factory, host_resolver)) {}
 
 TCPClientSocketPool::~TCPClientSocketPool() {}