Bug 606719 - Browser stalls with connections left hanging in TCP close wait state. Part 2: fix it. r=dwitte, a=b7+
authorPatrick McManus <mcmanus@ducksong.com>
Thu, 28 Oct 2010 10:10:03 -0700
changeset 56650 16f18e41dcf349bed4dbfbaed141f784eed1fbda
parent 56649 fa02ac41c6504a9103e82f0ff50c90e0380a5254
child 56651 18db490056aad073a978b1079b0e852efa87f41e
push idunknown
push userunknown
push dateunknown
reviewersdwitte, b7
bugs606719
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 606719 - Browser stalls with connections left hanging in TCP close wait state. Part 2: fix it. r=dwitte, a=b7+
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpHandler.cpp
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -201,16 +201,18 @@ nsHttpConnectionMgr::PruneDeadConnection
     }
 }
 
 void
 nsHttpConnectionMgr::StopPruneDeadConnectionsTimer()
 {
     LOG(("nsHttpConnectionMgr::StopPruneDeadConnectionsTimer\n"));
 
+    // Reset mTimeOfNextWakeUp so that we can find a new shortest value.
+    mTimeOfNextWakeUp = LL_MAXUINT;
     if (mTimer) {
         mTimer->Cancel();
         mTimer = NULL;
     }
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpConnectionMgr::nsIObserver
@@ -407,18 +409,18 @@ nsHttpConnectionMgr::PruneDeadConnection
             } else {
                 timeToNextExpire = PR_MIN(timeToNextExpire, conn->TimeToLive());
             }
         }
     }
 
     // If time to next expire found is shorter than time to next wake-up, we need to
     // change the time for next wake-up.
+    PRUint32 now = NowInSeconds();
     if (0 < ent->mIdleConns.Length()) {
-        PRUint32 now = NowInSeconds();
         PRUint64 timeOfNextExpire = now + timeToNextExpire;
         // If pruning of dead connections is not already scheduled to happen
         // or time found for next connection to expire is is before
         // mTimeOfNextWakeUp, we need to schedule the pruning to happen
         // after timeToNextExpire.
         if (!self->mTimer || timeOfNextExpire < self->mTimeOfNextWakeUp) {
             self->PruneDeadConnectionsAfter(timeToNextExpire);
         }
@@ -551,17 +553,18 @@ nsHttpConnectionMgr::ProcessPendingQForE
 PRBool
 nsHttpConnectionMgr::AtActiveConnectionLimit(nsConnectionEntry *ent, PRUint8 caps)
 {
     nsHttpConnectionInfo *ci = ent->mConnInfo;
 
     LOG(("nsHttpConnectionMgr::AtActiveConnectionLimit [ci=%s caps=%x]\n",
         ci->HashKey().get(), caps));
 
-    // use >= just to be safe
+    // If we have more active connections than the limit, then we're done --
+    // purging idle connections won't get us below it.
     if (mNumActiveConns >= mMaxConns) {
         LOG(("  num active conns == max conns\n"));
         return PR_TRUE;
     }
 
     nsHttpConnection *conn;
     PRInt32 i, totalCount, persistCount = 0;
     
@@ -597,21 +600,16 @@ void
 nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps,
                                    nsHttpConnection **result)
 {
     LOG(("nsHttpConnectionMgr::GetConnection [ci=%s caps=%x]\n",
         ent->mConnInfo->HashKey().get(), PRUint32(caps)));
 
     *result = nsnull;
 
-    if (AtActiveConnectionLimit(ent, caps)) {
-        LOG(("  at active connection limit!\n"));
-        return;
-    }
-
     nsHttpConnection *conn = nsnull;
 
     if (caps & NS_HTTP_ALLOW_KEEPALIVE) {
         // search the idle connection list
         while (!conn && (ent->mIdleConns.Length() > 0)) {
             conn = ent->mIdleConns[0];
             // we check if the connection can be reused before even checking if
             // it is a "matching" connection.
@@ -619,45 +617,52 @@ nsHttpConnectionMgr::GetConnection(nsCon
                 LOG(("   dropping stale connection: [conn=%x]\n", conn));
                 conn->Close(NS_ERROR_ABORT);
                 NS_RELEASE(conn);
             }
             else
                 LOG(("   reusing connection [conn=%x]\n", conn));
             ent->mIdleConns.RemoveElementAt(0);
             mNumIdleConns--;
+
+            // If there are no idle connections left at all, we need to make
+            // sure that we are not pruning dead connections anymore.
+            if (0 == mNumIdleConns)
+                StopPruneDeadConnectionsTimer();
         }
     }
 
     if (!conn) {
-        // No reusable idle connection found for this entry. If there are no
-        // idle connections left at all, we need to make sure that we are not
-        // pruning dead connections anymore.
-        if (0 == mNumIdleConns)
-            StopPruneDeadConnectionsTimer();
+        // Check if we need to purge an idle connection. Note that we may have
+        // removed one above; if so, this will be a no-op. We do this before
+        // checking the active connection limit to catch the case where we do
+        // have an idle connection, but the purge timer hasn't fired yet.
+        // XXX this just purges a random idle connection.  we should instead
+        // enumerate the entire hash table to find the eldest idle connection.
+        if (mNumIdleConns && mNumIdleConns + mNumActiveConns + 1 >= mMaxConns)
+            mCT.Enumerate(PurgeOneIdleConnectionCB, this);
+
+        // Need to make a new TCP connection. First, we check if we've hit
+        // either the maximum connection limit globally or for this particular
+        // host or proxy. If we have, we're done.
+        if (AtActiveConnectionLimit(ent, caps)) {
+            LOG(("  at active connection limit!\n"));
+            return;
+        }
 
         conn = new nsHttpConnection();
         if (!conn)
             return;
         NS_ADDREF(conn);
 
         nsresult rv = conn->Init(ent->mConnInfo, mMaxRequestDelay);
         if (NS_FAILED(rv)) {
             NS_RELEASE(conn);
             return;
         }
-        
-        // We created a new connection that will become active, purge the
-        // oldest idle connection if we've reached the upper limit.
-        // This only needs to be done if there is a idle connection.
-        if (0 < mNumIdleConns && mNumIdleConns + mNumActiveConns + 1 > mMaxConns)
-            mCT.Enumerate(PurgeOneIdleConnectionCB, this);
-
-        // XXX this just purges a random idle connection.  we should instead
-        // enumerate the entire hash table to find the eldest idle connection.
     }
 
     *result = conn;
 }
 
 nsresult
 nsHttpConnectionMgr::DispatchTransaction(nsConnectionEntry *ent,
                                          nsAHttpTransaction *trans,
@@ -927,16 +932,18 @@ nsHttpConnectionMgr::OnMsgProcessPending
     NS_RELEASE(ci);
 }
 
 void
 nsHttpConnectionMgr::OnMsgPruneDeadConnections(PRInt32, void *)
 {
     LOG(("nsHttpConnectionMgr::OnMsgPruneDeadConnections\n"));
 
+    // Reset mTimeOfNextWakeUp so that we can find a new shortest value.
+    mTimeOfNextWakeUp = LL_MAXUINT;
     if (mNumIdleConns > 0) 
         mCT.Enumerate(PruneDeadConnectionsCB, this);
 }
 
 void
 nsHttpConnectionMgr::OnMsgReclaimConnection(PRInt32, void *param)
 {
     LOG(("nsHttpConnectionMgr::OnMsgReclaimConnection [conn=%p]\n", param));
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -71,16 +71,17 @@
 #include "nsNetCID.h"
 #include "nsAutoLock.h"
 #include "prprf.h"
 #include "nsReadableUtils.h"
 #include "nsQuickSort.h"
 #include "nsNetUtil.h"
 #include "nsIOService.h"
 #include "nsAsyncRedirectVerifyHelper.h"
+#include "nsSocketTransportService2.h"
 
 #include "nsIXULAppInfo.h"
 
 #ifdef MOZ_IPC
 #include "mozilla/net/NeckoChild.h"
 #endif 
 
 #if defined(XP_UNIX) || defined(XP_BEOS)
@@ -849,17 +850,17 @@ nsHttpHandler::PrefsChanged(nsIPrefBranc
                 mConnMgr->UpdateParam(nsHttpConnectionMgr::MAX_REQUEST_DELAY,
                                       mMaxRequestDelay);
         }
     }
 
     if (PREF_CHANGED(HTTP_PREF("max-connections"))) {
         rv = prefs->GetIntPref(HTTP_PREF("max-connections"), &val);
         if (NS_SUCCEEDED(rv)) {
-            mMaxConnections = (PRUint16) NS_CLAMP(val, 1, 0xffff);
+            mMaxConnections = (PRUint16) NS_CLAMP(val, 1, NS_SOCKET_MAX_COUNT);
             if (mConnMgr)
                 mConnMgr->UpdateParam(nsHttpConnectionMgr::MAX_CONNECTIONS,
                                       mMaxConnections);
         }
     }
 
     if (PREF_CHANGED(HTTP_PREF("max-connections-per-server"))) {
         rv = prefs->GetIntPref(HTTP_PREF("max-connections-per-server"), &val);