Bug 592284 - Accelerate TCP connection retries in HTTP. r=honzab a=blocking2.0
authorPatrick McManus <mcmanus@ducksong.com>
Sun, 21 Nov 2010 09:50:36 +0100
changeset 57969 be4b69a4babfb0a62fe0d6de850dc8df9a5ba020
parent 57968 baa51e6d4a1540cfa4139cd4777f413d4f8bf755
child 57970 885574b5c18c0d50a8903cfd880aed4b1afeb39a
push id17104
push userdgottwald@mozilla.com
push dateSun, 21 Nov 2010 08:51:05 +0000
treeherdermozilla-central@be4b69a4babf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, blocking2
bugs592284
milestone2.0b8pre
first release with
nightly linux32
be4b69a4babf / 4.0b8pre / 20101121031805 / files
nightly linux64
be4b69a4babf / 4.0b8pre / 20101121031826 / files
nightly mac
be4b69a4babf / 4.0b8pre / 20101121030816 / files
nightly win32
be4b69a4babf / 4.0b8pre / 20101121044503 / files
nightly win64
be4b69a4babf / 4.0b8pre / 20101121053338 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 592284 - Accelerate TCP connection retries in HTTP. r=honzab a=blocking2.0 Losing a TCP SYN requires a long painful (typically 3 second) delay before being retried. This patch creates a second parallel connection attempt for any nsHttpConnection which has not become writable before a timeout occurs. If you assume .5% packet loss, this converts a full 3 second delay from a 1 in 200 event into a 1 in 40,000 event. Whichever connection establishes itself first is used. If another one has been started and it does connect before the one being used is closed then the extra one is handed to the connection manager for use by a different transaction - essentially a persistent connection with 0 previous transactions on it. (Another way to think about is pre-fetching a 3WHS on a high latency connection). The pref network.http.connection-retry-timeout controls the amount of time in ms to wait for success on the initial connection before beginning the second one. Setting it to 0 disables the parallel connection, the default is 250.
modules/libpref/src/init/all.js
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnection.h
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
--- a/modules/libpref/src/init/all.js
+++ b/modules/libpref/src/init/all.js
@@ -732,16 +732,21 @@ pref("network.http.prompt-temp-redirect"
 // for certain services (i.e. EF for VoIP, AF4x for interactive video,
 // AF3x for broadcast/streaming video, etc)
 
 // default value for HTTP
 // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594,
 // Section 4.8 "High-Throughput Data Service Class"
 pref("network.http.qos", 0);
 
+// The number of milliseconds after sending a SYN for an HTTP connection,
+// to wait before trying a different connection. 0 means do not use a second
+// connection.
+pref("network.http.connection-retry-timeout", 250);
+
 // default values for FTP
 // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594,
 // Section 4.8 "High-Throughput Data Service Class", and 80 (0x50, or AF22)
 // per Section 4.7 "Low-Latency Data Service Class".
 pref("network.ftp.data.qos", 0);
 pref("network.ftp.control.qos", 0);
 
 // </http>
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -55,43 +55,60 @@
 
 #ifdef DEBUG
 // defined by the socket transport service while active
 extern PRThread *gSocketThread;
 #endif
 
 static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID);
 
+
+// Statistics - only update on gSocketThread
+// currently uncollected
+
+static PRUint32 sCreateTransport1 = 0;
+static PRUint32 sCreateTransport2 = 0;
+static PRUint32 sSuccessTransport1 = 0;
+static PRUint32 sSuccessTransport2 = 0;
+static PRUint32 sUnNecessaryTransport2 = 0;
+static PRUint32 sWastedReuseCount = 0;
+
 //-----------------------------------------------------------------------------
 // nsHttpConnection <public>
 //-----------------------------------------------------------------------------
 
 nsHttpConnection::nsHttpConnection()
     : mTransaction(nsnull)
     , mConnInfo(nsnull)
     , mLock(nsnull)
     , mLastReadTime(0)
     , mIdleTimeout(0)
     , mKeepAlive(PR_TRUE) // assume to keep-alive by default
     , mKeepAliveMask(PR_TRUE)
     , mSupportsPipelining(PR_FALSE) // assume low-grade server
     , mIsReused(PR_FALSE)
     , mCompletedSSLConnect(PR_FALSE)
+    , mActivationCount(0)
 {
     LOG(("Creating nsHttpConnection @%x\n", this));
 
     // grab a reference to the handler to ensure that it doesn't go away.
     nsHttpHandler *handler = gHttpHandler;
     NS_ADDREF(handler);
 }
 
 nsHttpConnection::~nsHttpConnection()
 {
     LOG(("Destroying nsHttpConnection @%x\n", this));
  
+    if (mIdleSynTimer) {
+        mIdleSynTimer->Cancel();
+        mIdleSynTimer = nsnull;
+    }
+    
     NS_IF_RELEASE(mConnInfo);
     NS_IF_RELEASE(mTransaction);
 
     if (mLock) {
         PR_DestroyLock(mLock);
         mLock = nsnull;
     }
 
@@ -115,72 +132,124 @@ nsHttpConnection::Init(nsHttpConnectionI
     mConnInfo = info;
     NS_ADDREF(mConnInfo);
 
     mMaxHangTime = maxHangTime;
     mLastReadTime = NowInSeconds();
     return NS_OK;
 }
 
+void
+nsHttpConnection::IdleSynTimeout(nsITimer *timer, void *closure)
+{
+    // nsITimer is guaranteed to execute timer on same thread it
+    // was initialized on
+    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
+
+    nsHttpConnection *self = (nsHttpConnection *)closure;
+    NS_ABORT_IF_FALSE(timer == self->mIdleSynTimer, "wrong timer");
+    self->mIdleSynTimer = nsnull;
+
+    if (!self->mSocketTransport) {
+        NS_ABORT_IF_FALSE(self->mSocketTransport1 && !self->mSocketTransport2,
+                          "establishing backup tranport");
+
+        LOG(("SocketTransport hit idle timer - starting backup socket"));
+
+        sCreateTransport2++;
+        nsresult rv =
+            self->CreateTransport(self->mSocketCaps,
+                                  getter_AddRefs(self->mSocketTransport2),
+                                  getter_AddRefs(self->mSocketIn2),
+                                  getter_AddRefs(self->mSocketOut2));
+        
+        if (NS_SUCCEEDED(rv))
+            self->mSocketOut2->AsyncWait(self, 0, 0, nsnull);
+    }
+
+    return;
+}
+
 // called on the socket thread
 nsresult
 nsHttpConnection::Activate(nsAHttpTransaction *trans, PRUint8 caps)
 {
     nsresult rv;
 
+    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     LOG(("nsHttpConnection::Activate [this=%x trans=%x caps=%x]\n",
          this, trans, caps));
 
     NS_ENSURE_ARG_POINTER(trans);
     NS_ENSURE_TRUE(!mTransaction, NS_ERROR_IN_PROGRESS);
 
     // take ownership of the transaction
     mTransaction = trans;
     NS_ADDREF(mTransaction);
+    mActivationCount++;
 
     // set mKeepAlive according to what will be requested
     mKeepAliveMask = mKeepAlive = (caps & NS_HTTP_ALLOW_KEEPALIVE);
 
-    // if we don't have a socket transport then create a new one
-    if (!mSocketTransport) {
-        rv = CreateTransport(caps);
-        if (NS_FAILED(rv))
-            goto loser;
-    }
-
     // need to handle SSL proxy CONNECT if this is the first time.
     if (mConnInfo->UsingSSL() && mConnInfo->UsingHttpProxy() && !mCompletedSSLConnect) {
         rv = SetupSSLProxyConnect();
         if (NS_FAILED(rv))
-            goto loser;
+            goto failed_activation;
     }
 
-    // wait for the output stream to be readable
-    rv = mSocketOut->AsyncWait(this, 0, 0, nsnull);
-    if (NS_SUCCEEDED(rv))
-        return rv;
+    // if we don't have a socket transport then create a new one
+    if (!mSocketTransport) {
+        rv = CreateTransport(caps);
+    }
+    else {
+        NS_ABORT_IF_FALSE(mSocketOut && mSocketIn,
+                          "Socket Transport and SocketOut mismatch");
+        
+        // If this is the first transaction on this connection, but
+        // we already have a socket that means the socket was created
+        // speculatively in the past, not used at that time, and
+        // given to the connection manager.
+        if (mActivationCount == 1)
+            sWastedReuseCount++;
 
-loser:
-    NS_RELEASE(mTransaction);
+        rv = mSocketOut->AsyncWait(this, 0, 0, nsnull);
+    }
+    
+failed_activation:
+    if (NS_FAILED(rv))
+        NS_RELEASE(mTransaction);
     return rv;
 }
 
 void
 nsHttpConnection::Close(nsresult reason)
 {
     LOG(("nsHttpConnection::Close [this=%x reason=%x]\n", this, reason));
 
-    NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread");
+    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
 
     if (NS_FAILED(reason)) {
         if (mSocketTransport) {
             mSocketTransport->SetSecurityCallbacks(nsnull);
             mSocketTransport->SetEventSink(nsnull, nsnull);
             mSocketTransport->Close(reason);
         }
+        
+        if (mSocketTransport1) {
+            mSocketTransport1->SetSecurityCallbacks(nsnull);
+            mSocketTransport1->SetEventSink(nsnull, nsnull);
+            mSocketTransport1->Close(reason);
+        }
+        
+        if (mSocketTransport2) {
+            mSocketTransport2->SetSecurityCallbacks(nsnull);
+            mSocketTransport2->SetEventSink(nsnull, nsnull);
+            mSocketTransport2->Close(reason);
+        }
         mKeepAlive = PR_FALSE;
     }
 }
 
 // called on the socket thread
 nsresult
 nsHttpConnection::ProxyStartSSL()
 {
@@ -445,19 +514,61 @@ nsHttpConnection::ResumeRecv()
 
 //-----------------------------------------------------------------------------
 // nsHttpConnection <private>
 //-----------------------------------------------------------------------------
 
 nsresult
 nsHttpConnection::CreateTransport(PRUint8 caps)
 {
+    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
+    NS_ABORT_IF_FALSE(!mSocketTransport, "unexpected");
+
     nsresult rv;
+    mSocketCaps = caps;
+    sCreateTransport1++;
+    
+    PRUint16 timeout = gHttpHandler->GetIdleSynTimeout();
+    if (timeout) {
+
+        // Setup the timer that will establish a backup socket
+        // if we do not get a writable event on the main one.
+        // We do this because a lost SYN takes a very long time
+        // to repair at the TCP level.
+        //
+        // Failure to setup the timer is something we can live with,
+        // so don't return an error in that case.
 
-    NS_PRECONDITION(!mSocketTransport, "unexpected");
+        mIdleSynTimer = do_CreateInstance(NS_TIMER_CONTRACTID, &rv);
+        if (NS_SUCCEEDED(rv))
+            mIdleSynTimer->InitWithFuncCallback(IdleSynTimeout, this,
+                                                timeout,
+                                                nsITimer::TYPE_ONE_SHOT);
+    }
+    
+    rv = CreateTransport(mSocketCaps,
+                         getter_AddRefs(mSocketTransport1),
+                         getter_AddRefs(mSocketIn1),
+                         getter_AddRefs(mSocketOut1));
+    if (NS_FAILED(rv))
+        return rv;
+
+    // wait for the output stream to be readable or timeout to occur
+    return mSocketOut1->AsyncWait(this, 0, 0, nsnull);
+}
+
+nsresult
+nsHttpConnection::CreateTransport(PRUint8 caps,
+                                  nsISocketTransport **sock,
+                                  nsIAsyncInputStream **instream,
+                                  nsIAsyncOutputStream **outstream)
+{
+    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
+
+    nsresult rv;
 
     nsCOMPtr<nsISocketTransportService> sts =
             do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &rv);
     if (NS_FAILED(rv)) return rv;
 
     // configure the socket type based on the connection type requested.
     const char* types[1];
 
@@ -499,19 +610,20 @@ nsHttpConnection::CreateTransport(PRUint
     rv = strans->OpenOutputStream(nsITransport::OPEN_UNBUFFERED, 0, 0,
                                   getter_AddRefs(sout));
     if (NS_FAILED(rv)) return rv;
     nsCOMPtr<nsIInputStream> sin;
     rv = strans->OpenInputStream(nsITransport::OPEN_UNBUFFERED, 0, 0,
                                  getter_AddRefs(sin));
     if (NS_FAILED(rv)) return rv;
 
-    mSocketTransport = strans;
-    mSocketIn = do_QueryInterface(sin);
-    mSocketOut = do_QueryInterface(sout);
+    strans.forget(sock);
+    CallQueryInterface(sin, instream);
+    CallQueryInterface(sout, outstream);
+
     return NS_OK;
 }
 
 void
 nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason)
 {
     LOG(("nsHttpConnection::CloseTransaction[this=%x trans=%x reason=%x]\n",
         this, trans, reason));
@@ -755,16 +867,99 @@ nsHttpConnection::SetupSSLProxyConnect()
 
     buf.Truncate();
     request.Flatten(buf, PR_FALSE);
     buf.AppendLiteral("\r\n");
 
     return NS_NewCStringInputStream(getter_AddRefs(mSSLProxyConnectStream), buf);
 }
 
+nsresult
+nsHttpConnection::ReleaseBackupTransport(nsISocketTransport *sock,
+                                         nsIAsyncOutputStream *outs,
+                                         nsIAsyncInputStream *ins)
+{
+    nsRefPtr<nsHttpConnection> clone = new nsHttpConnection();
+    nsresult rv = clone->Init(mConnInfo, mMaxHangTime);
+    if (NS_SUCCEEDED(rv)) {
+        // We need to establish a non-zero idle timeout so the connection mgr
+        // perceives this socket as suitable for persistent connection reuse
+        clone->mIdleTimeout = gHttpHandler->IdleTimeout();
+        clone->mSocketTransport = sock;
+        clone->mSocketOut = outs;
+        clone->mSocketIn = ins;
+        gHttpHandler->ReclaimConnection(clone);
+    }
+    return rv;
+}
+
+nsresult
+nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out)
+{
+    nsresult rv = NS_OK;
+    
+    if (!mSocketOut) {
+        // Setup the Main Socket
+        
+        if (mIdleSynTimer) {
+            mIdleSynTimer->Cancel();
+            mIdleSynTimer = nsnull;
+        }
+        
+        if (out == mSocketOut1) {
+            sSuccessTransport1++;
+            mSocketTransport.swap(mSocketTransport1);
+            mSocketOut.swap(mSocketOut1);
+            mSocketIn.swap(mSocketIn1);
+
+            if (mSocketTransport2)
+                sUnNecessaryTransport2++;
+        }
+        else if (out == mSocketOut2) {
+            NS_ABORT_IF_FALSE(mSocketOut1,
+                              "backup socket without primary being tested");
+
+            sSuccessTransport2++;
+            mSocketTransport.swap(mSocketTransport2);
+            mSocketOut.swap(mSocketOut2);
+            mSocketIn.swap(mSocketIn2);
+        }
+        else {
+            NS_ABORT_IF_FALSE(0, "setup on unexpected socket");
+            return NS_ERROR_UNEXPECTED;
+        }
+    }
+    else if (out == mSocketOut1) {
+        // Socket2 became the primary socket but Socket1 is now valid - give it
+        // to the connection manager
+
+        rv = ReleaseBackupTransport(mSocketTransport1,
+                                    mSocketOut1,
+                                    mSocketIn1);
+        sSuccessTransport1++;
+        mSocketTransport1 = nsnull;
+        mSocketOut1 = nsnull;
+        mSocketIn1 = nsnull;
+    }
+    else if (out == mSocketOut2) {
+        // Socket1 became the primary socket but Socket2 is now valid - give it
+        // to the connectionmanager
+
+        rv = ReleaseBackupTransport(mSocketTransport2,
+                                    mSocketOut2,
+                                    mSocketIn2);
+        sSuccessTransport2++;
+        mSocketTransport2 = nsnull;
+        mSocketOut2 = nsnull;
+        mSocketIn2 = nsnull;
+    }
+
+    return rv;
+}
+
 //-----------------------------------------------------------------------------
 // nsHttpConnection::nsISupports
 //-----------------------------------------------------------------------------
 
 NS_IMPL_THREADSAFE_ISUPPORTS4(nsHttpConnection,
                               nsIInputStreamCallback,
                               nsIOutputStreamCallback,
                               nsITransportEventSink,
@@ -796,26 +991,38 @@ nsHttpConnection::OnInputStreamReady(nsI
 
 //-----------------------------------------------------------------------------
 // nsHttpConnection::nsIOutputStreamCallback
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpConnection::OnOutputStreamReady(nsIAsyncOutputStream *out)
 {
-    NS_ASSERTION(out == mSocketOut, "unexpected stream");
-    NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread");
+    NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
 
     // if the transaction was dropped...
     if (!mTransaction) {
         LOG(("  no transaction; ignoring event\n"));
         return NS_OK;
     }
 
-    nsresult rv = OnSocketWritable();
+    NS_ABORT_IF_FALSE(out == mSocketOut  ||
+                      out == mSocketOut1 ||
+                      out == mSocketOut2    , "unexpected socket");
+    nsresult rv;
+    if (out != mSocketOut)
+        rv = SelectPrimaryTransport(out);
+    else
+        rv = NS_OK;
+
+    if (NS_SUCCEEDED(rv) && (mSocketOut == out)) {
+        NS_ABORT_IF_FALSE(!mIdleSynTimer,"IdleSynTimer should not be set");
+        rv = OnSocketWritable();
+    }
+
     if (NS_FAILED(rv))
         CloseTransaction(mTransaction, rv);
 
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpConnection::nsITransportEventSink
--- a/netwerk/protocol/http/nsHttpConnection.h
+++ b/netwerk/protocol/http/nsHttpConnection.h
@@ -47,16 +47,17 @@
 #include "nsCOMPtr.h"
 #include "prlock.h"
 
 #include "nsIStreamListener.h"
 #include "nsISocketTransport.h"
 #include "nsIAsyncInputStream.h"
 #include "nsIAsyncOutputStream.h"
 #include "nsIInterfaceRequestor.h"
+#include "nsITimer.h"
 
 //-----------------------------------------------------------------------------
 // nsHttpConnection - represents a connection to a HTTP server (or proxy)
 //
 // NOTE: this objects lives on the socket thread only.  it should not be
 // accessed from any other thread.
 //-----------------------------------------------------------------------------
 
@@ -125,25 +126,35 @@ public:
     static NS_METHOD ReadFromStream(nsIInputStream *, void *, const char *,
                                     PRUint32, PRUint32, PRUint32 *);
 
 private:
     // called to cause the underlying socket to start speaking SSL
     nsresult ProxyStartSSL();
 
     nsresult CreateTransport(PRUint8 caps);
+    nsresult CreateTransport(PRUint8 caps,
+                             nsISocketTransport **sock,
+                             nsIAsyncInputStream **instream,
+                             nsIAsyncOutputStream **outstream);
+
     nsresult OnTransactionDone(nsresult reason);
     nsresult OnSocketWritable();
     nsresult OnSocketReadable();
 
     nsresult SetupSSLProxyConnect();
 
     PRBool   IsAlive();
     PRBool   SupportsPipelining(nsHttpResponseHead *);
     
+    static void  IdleSynTimeout(nsITimer *, void *);
+    nsresult     SelectPrimaryTransport(nsIAsyncOutputStream *out);
+    nsresult     ReleaseBackupTransport(nsISocketTransport *sock,
+                                        nsIAsyncOutputStream *outs,
+                                        nsIAsyncInputStream *ins);
 private:
     nsCOMPtr<nsISocketTransport>    mSocketTransport;
     nsCOMPtr<nsIAsyncInputStream>   mSocketIn;
     nsCOMPtr<nsIAsyncOutputStream>  mSocketOut;
 
     nsresult                        mSocketInCondition;
     nsresult                        mSocketOutCondition;
 
@@ -159,11 +170,26 @@ private:
     PRUint16                        mMaxHangTime;    // max download time before dropping keep-alive status
     PRUint16                        mIdleTimeout;    // value of keep-alive: timeout=
 
     PRPackedBool                    mKeepAlive;
     PRPackedBool                    mKeepAliveMask;
     PRPackedBool                    mSupportsPipelining;
     PRPackedBool                    mIsReused;
     PRPackedBool                    mCompletedSSLConnect;
+
+    PRUint32                        mActivationCount;
+
+    // These items are used to implement a parallel connection opening
+    // attempt when network.http.connection-retry-timeout has expired
+    PRUint8                         mSocketCaps;
+    nsCOMPtr<nsITimer>              mIdleSynTimer;
+
+    nsCOMPtr<nsISocketTransport>    mSocketTransport1;
+    nsCOMPtr<nsIAsyncInputStream>   mSocketIn1;
+    nsCOMPtr<nsIAsyncOutputStream>  mSocketOut1;
+
+    nsCOMPtr<nsISocketTransport>    mSocketTransport2;
+    nsCOMPtr<nsIAsyncInputStream>   mSocketIn2;
+    nsCOMPtr<nsIAsyncOutputStream>  mSocketOut2;
 };
 
 #endif // nsHttpConnection_h__
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -361,34 +361,40 @@ nsHttpConnectionMgr::ProcessOneTransacti
     nsConnectionEntry *ent = (nsConnectionEntry *) data;
 
     if (self->ProcessPendingQForEntry(ent))
         return kHashEnumerateStop;
 
     return kHashEnumerateNext;
 }
 
+// If the global number of idle connections is preventing the opening of
+// new connections to a host without idle connections, then
+// close them regardless of their TTL
 PRIntn
-nsHttpConnectionMgr::PurgeOneIdleConnectionCB(nsHashKey *key, void *data, void *closure)
+nsHttpConnectionMgr::PurgeExcessIdleConnectionsCB(nsHashKey *key,
+                                                  void *data, void *closure)
 {
     nsHttpConnectionMgr *self = (nsHttpConnectionMgr *) closure;
     nsConnectionEntry *ent = (nsConnectionEntry *) data;
 
-    if (ent->mIdleConns.Length() > 0) {
+    while (self->mNumIdleConns + self->mNumActiveConns + 1 >= self->mMaxConns) {
+        if (!ent->mIdleConns.Length()) {
+            // There are no idle conns left in this connection entry
+            return kHashEnumerateNext;
+        }
         nsHttpConnection *conn = ent->mIdleConns[0];
         ent->mIdleConns.RemoveElementAt(0);
         conn->Close(NS_ERROR_ABORT);
         NS_RELEASE(conn);
         self->mNumIdleConns--;
         if (0 == self->mNumIdleConns)
             self->StopPruneDeadConnectionsTimer();
-        return kHashEnumerateStop;
     }
-
-    return kHashEnumerateNext;
+    return kHashEnumerateStop;
 }
 
 PRIntn
 nsHttpConnectionMgr::PruneDeadConnectionsCB(nsHashKey *key, void *data, void *closure)
 {
     nsHttpConnectionMgr *self = (nsHttpConnectionMgr *) closure;
     nsConnectionEntry *ent = (nsConnectionEntry *) data;
 
@@ -598,22 +604,29 @@ nsHttpConnectionMgr::AtActiveConnectionL
 
 void
 nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps,
                                    nsHttpConnection **result)
 {
     LOG(("nsHttpConnectionMgr::GetConnection [ci=%s caps=%x]\n",
         ent->mConnInfo->HashKey().get(), PRUint32(caps)));
 
+    // First, see if an idle persistent connection may be reused instead of
+    // establishing a new socket. We do not need to check the connection limits
+    // yet as they govern the maximum number of open connections and reusing
+    // an old connection never increases that.
+
     *result = nsnull;
 
     nsHttpConnection *conn = nsnull;
 
     if (caps & NS_HTTP_ALLOW_KEEPALIVE) {
-        // search the idle connection list
+        // search the idle connection list. Each element in the list
+        // has a reference, so if we remove it from the list into a local
+        // ptr, that ptr now owns the reference
         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.
             if (!conn->CanReuse()) {
                 LOG(("   dropping stale connection: [conn=%x]\n", conn));
                 conn->Close(NS_ERROR_ABORT);
                 NS_RELEASE(conn);
@@ -633,31 +646,28 @@ nsHttpConnectionMgr::GetConnection(nsCon
     if (!conn) {
         // 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);
+            mCT.Enumerate(PurgeExcessIdleConnectionsCB, 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;
         }
     }
 
     *result = conn;
@@ -959,38 +969,45 @@ nsHttpConnectionMgr::OnMsgReclaimConnect
     nsHttpConnectionInfo *ci = conn->ConnectionInfo();
     NS_ADDREF(ci);
 
     nsCStringKey key(ci->HashKey());
     nsConnectionEntry *ent = (nsConnectionEntry *) mCT.Get(&key);
 
     NS_ASSERTION(ent, "no connection entry");
     if (ent) {
-        ent->mActiveConns.RemoveElement(conn);
-        mNumActiveConns--;
+        // If the connection is in the active list, remove that entry
+        // and the reference held by the mActiveConns list.
+        // This is never the final reference on conn as the event context
+        // is also holding one that is released at the end of this function.
+        if (ent->mActiveConns.RemoveElement(conn)) {
+            nsHttpConnection *temp = conn;
+            NS_RELEASE(temp);
+            mNumActiveConns--;
+        }
+
         if (conn->CanReuse()) {
             LOG(("  adding connection to idle list\n"));
             // hold onto this connection in the idle list.  we push it to
             // the end of the list so as to ensure that we'll visit older
             // connections first before getting to this one.
+            NS_ADDREF(conn);
             ent->mIdleConns.AppendElement(conn);
             mNumIdleConns++;
             // If the added connection was first idle connection or has shortest
             // time to live among the idle connections, pruning dead
             // connections needs to be done when it can't be reused anymore.
             PRUint32 timeToLive = conn->TimeToLive();
             if(!mTimer || NowInSeconds() + timeToLive < mTimeOfNextWakeUp)
                 PruneDeadConnectionsAfter(timeToLive);
         }
         else {
             LOG(("  connection cannot be reused; closing connection\n"));
             // make sure the connection is closed and release our reference.
             conn->Close(NS_ERROR_ABORT);
-            nsHttpConnection *temp = conn;
-            NS_RELEASE(temp);
         }
     }
  
     OnMsgProcessPendingQ(NS_OK, ci); // releases |ci|
     NS_RELEASE(conn);
 }
 
 void
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -199,20 +199,20 @@ private:
     PRUint16 mMaxRequestDelay; // in seconds
     PRUint16 mMaxPipelinedRequests;
 
     //-------------------------------------------------------------------------
     // NOTE: these members are only accessed on the socket transport thread
     //-------------------------------------------------------------------------
 
     static PRIntn ProcessOneTransactionCB(nsHashKey *, void *, void *);
-    static PRIntn PurgeOneIdleConnectionCB(nsHashKey *, void *, void *);
+
     static PRIntn PruneDeadConnectionsCB(nsHashKey *, void *, void *);
     static PRIntn ShutdownPassCB(nsHashKey *, void *, void *);
-
+    static PRIntn PurgeExcessIdleConnectionsCB(nsHashKey *, void *, void *);
     PRBool   ProcessPendingQForEntry(nsConnectionEntry *);
     PRBool   AtActiveConnectionLimit(nsConnectionEntry *, PRUint8 caps);
     void     GetConnection(nsConnectionEntry *, PRUint8 caps, nsHttpConnection **);
     nsresult DispatchTransaction(nsConnectionEntry *, nsAHttpTransaction *,
                                  PRUint8 caps, nsHttpConnection *);
     PRBool   BuildPipeline(nsConnectionEntry *, nsAHttpTransaction *, nsHttpPipeline **);
     nsresult ProcessNewTransaction(nsHttpTransaction *);
 
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -173,16 +173,17 @@ nsHttpHandler::nsHttpHandler()
     , mHttpVersion(NS_HTTP_VERSION_1_1)
     , mProxyHttpVersion(NS_HTTP_VERSION_1_1)
     , mCapabilities(NS_HTTP_ALLOW_KEEPALIVE)
     , mProxyCapabilities(NS_HTTP_ALLOW_KEEPALIVE)
     , mReferrerLevel(0xff) // by default we always send a referrer
     , mIdleTimeout(10)
     , mMaxRequestAttempts(10)
     , mMaxRequestDelay(10)
+    , mIdleSynTimeout(250)
     , mMaxConnections(24)
     , mMaxConnectionsPerServer(8)
     , mMaxPersistentConnectionsPerServer(2)
     , mMaxPersistentConnectionsPerProxy(4)
     , mMaxPipelinedRequests(2)
     , mRedirectionLimit(10)
     , mInPrivateBrowsingMode(PR_FALSE)
     , mPhishyUserPassLength(1)
@@ -902,16 +903,22 @@ nsHttpHandler::PrefsChanged(nsIPrefBranc
     }
 
     if (PREF_CHANGED(HTTP_PREF("redirection-limit"))) {
         rv = prefs->GetIntPref(HTTP_PREF("redirection-limit"), &val);
         if (NS_SUCCEEDED(rv))
             mRedirectionLimit = (PRUint8) NS_CLAMP(val, 0, 0xff);
     }
 
+    if (PREF_CHANGED(HTTP_PREF("connection-retry-timeout"))) {
+        rv = prefs->GetIntPref(HTTP_PREF("connection-retry-timeout"), &val);
+        if (NS_SUCCEEDED(rv))
+            mIdleSynTimeout = (PRUint16) NS_CLAMP(val, 0, 3000);
+    }
+
     if (PREF_CHANGED(HTTP_PREF("version"))) {
         nsXPIDLCString httpVersion;
         prefs->GetCharPref(HTTP_PREF("version"), getter_Copies(httpVersion));
         if (httpVersion) {
             if (!PL_strcmp(httpVersion, "1.1"))
                 mHttpVersion = NS_HTTP_VERSION_1_1;
             else if (!PL_strcmp(httpVersion, "0.9"))
                 mHttpVersion = NS_HTTP_VERSION_0_9;
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -102,16 +102,17 @@ public:
     PRBool         SendSecureXSiteReferrer() { return mSendSecureXSiteReferrer; }
     PRUint8        RedirectionLimit()        { return mRedirectionLimit; }
     PRUint16       IdleTimeout()             { return mIdleTimeout; }
     PRUint16       MaxRequestAttempts()      { return mMaxRequestAttempts; }
     const char    *DefaultSocketType()       { return mDefaultSocketType.get(); /* ok to return null */ }
     nsIIDNService *IDNConverter()            { return mIDNConverter; }
     PRUint32       PhishyUserPassLength()    { return mPhishyUserPassLength; }
     PRUint8        GetQoSBits()              { return mQoSBits; }
+    PRUint16       GetIdleSynTimeout()       { return mIdleSynTimeout; }
     
     PRBool         IsPersistentHttpsCachingEnabled() { return mEnablePersistentHttpsCaching; }
 
     PRBool         PromptTempRedirect()      { return mPromptTempRedirect; }
 
     nsHttpAuthCache     *AuthCache() { return &mAuthCache; }
     nsHttpConnectionMgr *ConnMgr()   { return mConnMgr; }
 
@@ -261,16 +262,17 @@ private:
     PRUint8  mProxyHttpVersion;
     PRUint8  mCapabilities;
     PRUint8  mProxyCapabilities;
     PRUint8  mReferrerLevel;
 
     PRUint16 mIdleTimeout;
     PRUint16 mMaxRequestAttempts;
     PRUint16 mMaxRequestDelay;
+    PRUint16 mIdleSynTimeout;
 
     PRUint16 mMaxConnections;
     PRUint8  mMaxConnectionsPerServer;
     PRUint8  mMaxPersistentConnectionsPerServer;
     PRUint8  mMaxPersistentConnectionsPerProxy;
     PRUint8  mMaxPipelinedRequests;
 
     PRUint8  mRedirectionLimit;