Bug 613977 - Intermittent invalid certificate error prompt (partial) r=honzab a=blocking-beta9

Bug 614677 - Connection is reset message appears intermittently
Bug 614950 - Connections stall occasionally after 592284 landed

A couple of follow-on changes to 592284 rolled together to prevent
diff conflicts

1] Set the securitycallback information for unused speculative
connections in the connection manager to be the new cloned connection
rather than the one they originated on. (613977)

2] When adding unused speculative connections to the connection
manager, due so with a short timeout (<= 5 seconds) as some servers
get grumpy if they haven't seen a request in that time. Most will
close the connection, but some will just sit there quietly and RST
things when the connection is used - so if you don't use the
connection quickly don't use it at all. This is probably a L4 load
balancer issue, actually. Mozillazine illustrates the
problem. Connections are made in bursts anyhow, so the reuse optimization is
likely still quite useful. (614677 and 614950)

3] mark every connection in the connection manager persistent
conneciton pool as "reused". This allows the transaction to be
restarted if a RST is recvd upon sending the requests (see #2) - with
the conservative timeout this is now a rare event, but still possible
so recovery is the right thing to do. (614677 and 614950)

4] obtain an nshttpconnection object from the connection manager,
subject to the max connection constraints, at the same time as
starting the backup conneciton. If we defer that until recycling time
the exceeded limits of the SocketService can cause problems for other
connections.

also re-enables the syn retry feature by default.

r+ honzab

modules/libpref/src/init/all.js

@@ -740,8 +740,7 @@ 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.
-// Temporarily Disabled for 4.0 Beta 8 - bug 614677
-pref("network.http.connection-retry-timeout", 0);
+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,

netwerk/protocol/http/nsHttpConnection.cpp

@@ -104,6 +104,11 @@ nsHttpConnection::~nsHttpConnection()
         mIdleSynTimer = nsnull;
     }
 
+    if (mBackupConnection) {
+        gHttpHandler->ReclaimConnection(mBackupConnection);
+        mBackupConnection = nsnull;
+    }
+    
     NS_IF_RELEASE(mConnInfo);
     NS_IF_RELEASE(mTransaction);
 
@@ -154,16 +159,22 @@ nsHttpConnection::IdleSynTimeout(nsITimer *timer, void *closure)
 
         LOG(("SocketTransport hit idle timer - starting backup socket"));
 
-        sCreateTransport2++;
+        gHttpHandler->ConnMgr()->GetConnection(self->mConnInfo,
+                                               self->mSocketCaps,
+                                               getter_AddRefs(
+                                                   self->mBackupConnection));
+        if (!self->mBackupConnection)
+            return;
         nsresult rv =
             self->CreateTransport(self->mSocketCaps,
                                   getter_AddRefs(self->mSocketTransport2),
                                   getter_AddRefs(self->mSocketIn2),
                                   getter_AddRefs(self->mSocketOut2));
-        
-        if (NS_SUCCEEDED(rv))
+        if (NS_SUCCEEDED(rv)) {
+            sCreateTransport2++;
             self->mSocketOut2->AsyncWait(self, 0, 0, nsnull);
         }
+    }
 
     return;
 }
@@ -632,6 +643,21 @@ nsHttpConnection::CreateTransport(PRUint8 caps,
     return NS_OK;
 }
 
+nsresult
+nsHttpConnection::AssignTransport(nsISocketTransport *sock,
+                                  nsIAsyncOutputStream *outs,
+                                  nsIAsyncInputStream *ins)
+{
+    nsresult rv = sock->SetEventSink(this, nsnull);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv =sock->SetSecurityCallbacks(this);
+    NS_ENSURE_SUCCESS(rv, rv);
+    mSocketTransport = sock;
+    mSocketOut = outs;
+    mSocketIn = ins;
+    return NS_OK;
+}
+
 void
 nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason)
 {
@@ -882,30 +908,28 @@ nsHttpConnection::SetupSSLProxyConnect()
     return NS_NewCStringInputStream(getter_AddRefs(mSSLProxyConnectStream), buf);
 }
 
-nsresult
+void
 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;
+    // We need to establish a small non-zero idle timeout so the connection
+    // mgr perceives this socket as suitable for persistent connection reuse
+    NS_ABORT_IF_FALSE(sock && outs && ins, "release Backup precond");
+    mBackupConnection->mIdleTimeout = NS_MIN((PRUint16) 5,
+                                             gHttpHandler->IdleTimeout());
+    mBackupConnection->mIsReused = PR_TRUE;
+    nsresult rv = mBackupConnection->AssignTransport(sock, outs, ins);
+    if (NS_SUCCEEDED(rv))
+        rv = gHttpHandler->ReclaimConnection(mBackupConnection);
+    if (NS_FAILED(rv))
+        NS_WARNING("Backup nsHttpConnection could not be reclaimed");
+    mBackupConnection = nsnull;
 }
 
-nsresult
+void
 nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out)
 {
-    nsresult rv = NS_OK;
-    
     if (!mSocketOut) {
         // Setup the Main Socket
         
@@ -934,14 +958,14 @@ nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out)
         }
         else {
             NS_ABORT_IF_FALSE(0, "setup on unexpected socket");
-            return NS_ERROR_UNEXPECTED;
+            return;
         }
     }
     else if (out == mSocketOut1) {
         // Socket2 became the primary socket but Socket1 is now valid - give it
         // to the connection manager
 
-        rv = ReleaseBackupTransport(mSocketTransport1,
+        ReleaseBackupTransport(mSocketTransport1,
                                mSocketOut1,
                                mSocketIn1);
         sSuccessTransport1++;
@@ -953,7 +977,7 @@ nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out)
         // Socket1 became the primary socket but Socket2 is now valid - give it
         // to the connectionmanager
 
-        rv = ReleaseBackupTransport(mSocketTransport2,
+        ReleaseBackupTransport(mSocketTransport2,
                                mSocketOut2,
                                mSocketIn2);
         sSuccessTransport2++;
@@ -961,8 +985,6 @@ nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out)
         mSocketOut2 = nsnull;
         mSocketIn2 = nsnull;
     }
-
-    return rv;
 }
 
 //-----------------------------------------------------------------------------
@@ -1017,19 +1039,15 @@ nsHttpConnection::OnOutputStreamReady(nsIAsyncOutputStream *out)
     NS_ABORT_IF_FALSE(out == mSocketOut  ||
                       out == mSocketOut1 ||
                       out == mSocketOut2    , "unexpected socket");
-    nsresult rv;
     if (out != mSocketOut)
-        rv = SelectPrimaryTransport(out);
-    else
-        rv = NS_OK;
+        SelectPrimaryTransport(out);
 
-    if (NS_SUCCEEDED(rv) && (mSocketOut == out)) {
+    if (mSocketOut == out) {
         NS_ABORT_IF_FALSE(!mIdleSynTimer,"IdleSynTimer should not be set");
-        rv = OnSocketWritable();
-    }
-
+        nsresult rv = OnSocketWritable();
         if (NS_FAILED(rv))
             CloseTransaction(mTransaction, rv);
+    }
 
     return NS_OK;
 }

netwerk/protocol/http/nsHttpConnection.h

@@ -135,6 +135,9 @@ private:
                              nsISocketTransport **sock,
                              nsIAsyncInputStream **instream,
                              nsIAsyncOutputStream **outstream);
+    nsresult AssignTransport(nsISocketTransport *sock,
+                             nsIAsyncOutputStream *outs,
+                             nsIAsyncInputStream *ins);
 
     nsresult OnTransactionDone(nsresult reason);
     nsresult OnSocketWritable();
@@ -146,8 +149,8 @@ private:
     PRBool   SupportsPipelining(nsHttpResponseHead *);
     
     static void  IdleSynTimeout(nsITimer *, void *);
-    nsresult     SelectPrimaryTransport(nsIAsyncOutputStream *out);
-    nsresult     ReleaseBackupTransport(nsISocketTransport *sock,
+    void         SelectPrimaryTransport(nsIAsyncOutputStream *out);
+    void         ReleaseBackupTransport(nsISocketTransport *sock,
                                         nsIAsyncOutputStream *outs,
                                         nsIAsyncInputStream *ins);
 private:
@@ -182,6 +185,7 @@ private:
     // attempt when network.http.connection-retry-timeout has expired
     PRUint8                         mSocketCaps;
     nsCOMPtr<nsITimer>              mIdleSynTimer;
+    nsRefPtr<nsHttpConnection>      mBackupConnection;
 
     nsCOMPtr<nsISocketTransport>    mSocketTransport1;
     nsCOMPtr<nsIAsyncInputStream>   mSocketIn1;

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -598,8 +598,8 @@ nsHttpConnectionMgr::AtActiveConnectionLimit(nsConnectionEntry *ent, PRUint8 cap
     LOG(("nsHttpConnectionMgr::AtActiveConnectionLimit [ci=%s caps=%x]\n",
         ci->HashKey().get(), caps));
 
-    // If we have more active connections than the limit, then we're done --
-    // purging idle connections won't get us below it.
+    // If there are more active connections than the global 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;
@@ -636,6 +636,18 @@ nsHttpConnectionMgr::AtActiveConnectionLimit(nsConnectionEntry *ent, PRUint8 cap
                                          (persistCount >= maxPersistConns) );
 }
 
+void
+nsHttpConnectionMgr::GetConnection(nsHttpConnectionInfo *ci,
+                                   PRUint8 caps,
+                                   nsHttpConnection **result)
+{
+    NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread");
+    nsCStringKey key(ci->HashKey());
+    nsConnectionEntry *ent = (nsConnectionEntry *) mCT.Get(&key);
+    if (!ent) return;
+    GetConnection(ent, caps, result);
+}
+
 void
 nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps,
                                    nsHttpConnection **result)
@@ -704,6 +716,11 @@ nsHttpConnectionMgr::GetConnection(nsConnectionEntry *ent, PRUint8 caps,
         }
     }
 
+    // hold an owning ref to this connection
+    ent->mActiveConns.AppendElement(conn);
+    mNumActiveConns++;
+    NS_ADDREF(conn);
+
     *result = conn;
 }
 
@@ -728,11 +745,6 @@ nsHttpConnectionMgr::DispatchTransaction(nsConnectionEntry *ent,
             trans = pipeline;
     }
 
-    // hold an owning ref to this connection
-    ent->mActiveConns.AppendElement(conn);
-    mNumActiveConns++;
-    NS_ADDREF(conn);
-
     // give the transaction the indirect reference to the connection.
     trans->SetConnection(handle);
 
@@ -847,15 +859,6 @@ nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction *trans)
 
         // destroy connection handle.
         trans->SetConnection(nsnull);
-
-        // remove sticky connection from active connection list; we'll add it
-        // right back in DispatchTransaction.
-        if (ent->mActiveConns.RemoveElement(conn))
-            mNumActiveConns--;
-        else {
-            NS_ERROR("sticky connection not found in active list");
-            return NS_ERROR_UNEXPECTED;
-        }
     }
     else
         GetConnection(ent, caps, &conn);

netwerk/protocol/http/nsHttpConnectionMgr.h

@@ -137,6 +137,9 @@ public:
     // preference to the specified connection.
     nsresult ProcessPendingQ(nsHttpConnectionInfo *);
 
+    // called to reserve a nshttpconnection object with the manager
+    void GetConnection(nsHttpConnectionInfo *, PRUint8 caps,
+                       nsHttpConnection **);
 private:
     virtual ~nsHttpConnectionMgr();