bug 528288 - spdy. Cleanups involving host coalescing r=honzab
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 02 Dec 2011 10:28:57 -0500
changeset 81189 9aed66c3a561683acbe305a6cb0e8d74fb5fa24e
parent 81188 0c7ba908f2fd9ce258f3adf169f708126a9319dc
child 81190 2cb0358aa68b1ad502a6f6c4efe88fe9308259ae
push id21564
push usermak77@bonardo.net
push dateSat, 03 Dec 2011 11:10:17 +0000
treeherdermozilla-central@a68c96c1d8e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs528288, 666028
milestone11.0a1
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 528288 - spdy. Cleanups involving host coalescing r=honzab patch 14 fixes 666028
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -395,28 +395,72 @@ nsHttpConnectionMgr::ProcessPendingQ(nsH
 
     NS_ADDREF(ci);
     nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgProcessPendingQ, 0, ci);
     if (NS_FAILED(rv))
         NS_RELEASE(ci);
     return rv;
 }
 
+// Given a nsHttpConnectionInfo find the connection entry object that
+// contains either the nshttpconnection or nshttptransaction parameter.
+// Normally this is done by the hashkey lookup of connectioninfo,
+// but if spdy coalescing is in play it might be found in a redirected
+// entry
+nsHttpConnectionMgr::nsConnectionEntry *
+nsHttpConnectionMgr::LookupConnectionEntry(nsHttpConnectionInfo *ci,
+                                           nsHttpConnection *conn,
+                                           nsHttpTransaction *trans)
+{
+    if (!ci)
+        return nsnull;
+
+    nsConnectionEntry *ent = mCT.Get(ci->HashKey());
+    
+    // If there is no sign of coalescing (or it is disabled) then just
+    // return the primary hash lookup
+    if (!gHttpHandler->IsSpdyEnabled() || !gHttpHandler->CoalesceSpdy() ||
+        !ent || !ent->mUsingSpdy || ent->mCoalescingKey.IsEmpty())
+        return ent;
+
+    // If there is no preferred coalescing entry for this host (or the
+    // preferred entry is the one that matched the mCT hash lookup) then
+    // there is only option
+    nsConnectionEntry *preferred = mSpdyPreferredHash.Get(ent->mCoalescingKey);
+    if (!preferred || (preferred == ent))
+        return ent;
+
+    if (conn) {
+        // The connection could be either in preferred or ent. It is most
+        // likely the only active connection in preferred - so start with that.
+        if (preferred->mActiveConns.Contains(conn))
+            return preferred;
+        if (preferred->mIdleConns.Contains(conn))
+            return preferred;
+    }
+    
+    if (trans && preferred->mPendingQ.Contains(trans))
+        return preferred;
+    
+    // Neither conn nor trans found in preferred, use the default entry
+    return ent;
+}
+
 nsresult
 nsHttpConnectionMgr::CloseIdleConnection(nsHttpConnection *conn)
 {
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     LOG(("nsHttpConnectionMgr::CloseIdleConnection %p conn=%p",
          this, conn));
 
-    nsHttpConnectionInfo *ci = conn->ConnectionInfo();
-    if (!ci)
+    if (!conn->ConnectionInfo())
         return NS_ERROR_UNEXPECTED;
 
-    nsConnectionEntry *ent = mCT.Get(ci->HashKey());
+    nsConnectionEntry *ent = LookupConnectionEntry(conn->ConnectionInfo(),
+                                                   conn, nsnull);
 
     if (!ent || !ent->mIdleConns.RemoveElement(conn))
         return NS_ERROR_UNEXPECTED;
 
     conn->Close(NS_ERROR_ABORT);
     NS_RELEASE(conn);
     mNumIdleConns--;
     ConditionallyStopPruneDeadConnectionsTimer();
@@ -424,17 +468,19 @@ nsHttpConnectionMgr::CloseIdleConnection
 }
 
 void
 nsHttpConnectionMgr::ReportSpdyConnection(nsHttpConnection *conn,
                                           bool usingSpdy)
 {
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     
-    nsConnectionEntry *ent = mCT.Get(conn->ConnectionInfo()->HashKey());
+    nsConnectionEntry *ent = LookupConnectionEntry(conn->ConnectionInfo(),
+                                                   conn, nsnull);
+
     NS_ABORT_IF_FALSE(ent, "no connection entry");
     if (!ent)
         return;
 
     ent->mTestedSpdy = true;
 
     if (!usingSpdy) {
         if (ent->mUsingSpdy)
@@ -459,54 +505,56 @@ nsHttpConnectionMgr::ReportSpdyConnectio
 
     LOG(("ReportSpdyConnection %s %s ent=%p ispreferred=%d\n",
          ent->mConnInfo->Host(), ent->mCoalescingKey.get(),
          ent, preferred));
     
     if (!preferred) {
         ent->mSpdyPreferred = true;
         SetSpdyPreferred(ent);
-        ent->mSpdyRedir = false;
+        preferred = ent;
     }
     else if (preferred != ent) {
         // A different hostname is the preferred spdy host for this
         // IP address.
-        ent->mSpdyRedir = true;
+        ent->mUsingSpdy = true;
         conn->DontReuse();
+        ent->mCert = nsnull;
     }
 
     // If this is a preferred host for coalescing (aka ip pooling) then
     // keep a reference to the server SSL cert. This will be used as an
     // extra level of verification when deciding that
     // connections from other hostnames are redirected to the preferred host.
     //
-
-    // Even if mCert is already set update the reference in case the
-    // reference is changing.
-    ent->mCert = nsnull;
+    if (preferred == ent) {
+        // Even if mCert is already set update the reference in case the
+        // reference is changing.
+        ent->mCert = nsnull;
 
-    nsCOMPtr<nsISupports> securityInfo;
-    nsCOMPtr<nsISSLStatusProvider> sslStatusProvider;
-    nsCOMPtr<nsISSLStatus> sslStatus;
-    nsCOMPtr<nsIX509Cert> cert;
+        nsCOMPtr<nsISupports> securityInfo;
+        nsCOMPtr<nsISSLStatusProvider> sslStatusProvider;
+        nsCOMPtr<nsISSLStatus> sslStatus;
+        nsCOMPtr<nsIX509Cert> cert;
 
-    conn->GetSecurityInfo(getter_AddRefs(securityInfo));
-    if (securityInfo)
-        sslStatusProvider = do_QueryInterface(securityInfo);
+        conn->GetSecurityInfo(getter_AddRefs(securityInfo));
+        if (securityInfo)
+            sslStatusProvider = do_QueryInterface(securityInfo);
+
+        if (sslStatusProvider)
+            sslStatusProvider->
+                GetSSLStatus(getter_AddRefs(sslStatus));
 
-    if (sslStatusProvider)
-        sslStatusProvider->
-            GetSSLStatus(getter_AddRefs(sslStatus));
+        if (sslStatus)
+            sslStatus->GetServerCert(getter_AddRefs(cert));
 
-    if (sslStatus)
-        sslStatus->GetServerCert(getter_AddRefs(cert));
-
-    if (cert)
-        ent->mCert = do_QueryInterface(cert);
-
+        if (cert)
+            ent->mCert = do_QueryInterface(cert);
+    }
+    
     ProcessSpdyPendingQ();
 }
 
 bool
 nsHttpConnectionMgr::GetSpdyAlternateProtocol(nsACString &hostPortKey)
 {
     // The Alternate Protocol hash is protected under the monitor because
     // it is read from both the main and the network thread.
@@ -577,36 +625,67 @@ nsHttpConnectionMgr::GetSpdyPreferred(ns
     if (!gHttpHandler->IsSpdyEnabled() ||
         !gHttpHandler->CoalesceSpdy() ||
         aOriginalEntry->mCoalescingKey.IsEmpty())
         return nsnull;
 
     nsConnectionEntry *preferred =
         mSpdyPreferredHash.Get(aOriginalEntry->mCoalescingKey);
 
+    // if there is no redirection no cert validation is required
     if (preferred == aOriginalEntry)
-        return aOriginalEntry;   /* no redirection so no cert check required */
+        return aOriginalEntry;
+
+    // if there is no server cert stored for the destination host
+    // then no validation can be made and we need to skip pooling
+    if (!preferred || !preferred->mCert || !preferred->mUsingSpdy)
+        return nsnull;                         
+
+    // if there is not an active spdy session in this entry then
+    // we cannot pool because the cert upon activation may not
+    // be the same as the old one. Active sessions are prohibited
+    // from changing certs.
+
+    bool activeSpdy = false;
 
-    if (!preferred || !preferred->mCert)
-        return nsnull;                         /* no ip pooling */
+    for (PRUint32 index = 0; index < preferred->mActiveConns.Length(); ++index)
+        if (preferred->mActiveConns[index]->CanDirectlyActivate()) {
+            activeSpdy = true;
+            break;
+        }
+    
+    if (!activeSpdy) {
+        // remove the preferred status of this entry if it cannot be
+        // used for pooling.
+        preferred->mSpdyPreferred = false;
+        RemoveSpdyPreferred(preferred->mCoalescingKey);
+        LOG(("nsHttpConnectionMgr::GetSpdyPreferredConnection "
+             "preferred host mapping %s to %s removed due to inactivity.\n",
+             aOriginalEntry->mConnInfo->Host(),
+             preferred->mConnInfo->Host()));
 
+        return nsnull;
+    }
+
+    // Check that the server cert supports redirection
     nsresult rv;
     bool validCert = false;
 
     rv = preferred->mCert->IsValidForHostname(
         aOriginalEntry->mConnInfo->GetHost(), &validCert);
 
     if (NS_FAILED(rv) || !validCert) {
         LOG(("nsHttpConnectionMgr::GetSpdyPreferredConnection "
              "Host %s has cert which cannot be confirmed to use "
              "with %s connections",
              preferred->mConnInfo->Host(), aOriginalEntry->mConnInfo->Host()));
-        return nsnull;                            /* no ip pooling */
+        return nsnull;
     }
 
+    // IP pooling confirmed
     LOG(("nsHttpConnectionMgr::GetSpdyPreferredConnection "
          "Host %s has cert valid for %s connections",
          preferred->mConnInfo->Host(), aOriginalEntry->mConnInfo->Host()));
     return preferred;
 }
 
 void
 nsHttpConnectionMgr::SetSpdyPreferred(nsConnectionEntry *ent)
@@ -995,20 +1074,24 @@ nsHttpConnectionMgr::GetConnection(nsCon
 
     *result = nsnull;
 
     nsHttpConnection *conn = nsnull;
     bool addConnToActiveList = true;
 
     if (trans->Caps() & NS_HTTP_ALLOW_KEEPALIVE) {
 
-        conn = GetSpdyPreferredConn(ent);
-        if (conn)
-            addConnToActiveList = false;
-
+        // if willing to use spdy look for an active spdy connections
+        // before considering idle http ones.
+        if (gHttpHandler->IsSpdyEnabled()) {
+            conn = GetSpdyPreferredConn(ent);
+            if (conn)
+                addConnToActiveList = false;
+        }
+        
         // 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()) {
@@ -1032,31 +1115,26 @@ nsHttpConnectionMgr::GetConnection(nsCon
 
     if (!conn) {
 
         // If the onlyReusedConnection parameter is TRUE, then GetConnection()
         // does not create new transports under any circumstances.
         if (onlyReusedConnection)
             return;
         
-        // If this is a possible Spdy connection we need to limit the number of
-        // connections outstanding to 1 while we wait for the spdy/https
-        // ReportSpdyConnection()
-    
         if (gHttpHandler->IsSpdyEnabled() &&
             ent->mConnInfo->UsingSSL() &&
             !ent->mConnInfo->UsingHttpProxy())
         {
-            nsConnectionEntry *preferred = GetSpdyPreferred(ent);
-            if (preferred)
-                ent = preferred;
-
+            // If this is a possible Spdy connection we need to limit the number
+            // of connections outstanding to 1 while we wait for the spdy/https
+            // ReportSpdyConnection()
+    
             if ((!ent->mTestedSpdy || ent->mUsingSpdy) &&
-                (ent->mSpdyRedir || ent->mHalfOpens.Length() ||
-                 ent->mActiveConns.Length()))
+                (ent->mHalfOpens.Length() || ent->mActiveConns.Length()))
                 return;
         }
         
         // 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
@@ -1138,16 +1216,20 @@ nsHttpConnectionMgr::DispatchTransaction
 {
     LOG(("nsHttpConnectionMgr::DispatchTransaction [ci=%s trans=%x caps=%x conn=%x]\n",
         ent->mConnInfo->HashKey().get(), aTrans, caps, conn));
     nsresult rv;
     
     PRInt32 priority = aTrans->Priority();
 
     if (conn->UsingSpdy()) {
+        LOG(("Spdy Dispatch Transaction via Activate(). Transaction host = %s,"
+             "Connection host = %s\n",
+             aTrans->ConnectionInfo()->Host(),
+             conn->ConnectionInfo()->Host()));
         rv = conn->Activate(aTrans, caps, priority);
         NS_ABORT_IF_FALSE(NS_SUCCEEDED(rv), "SPDY Cannot Fail Dispatch");
         return rv;
     }
 
     nsConnectionHandle *handle = new nsConnectionHandle(conn);
     if (!handle)
         return NS_ERROR_OUT_OF_MEMORY;
@@ -1258,20 +1340,21 @@ nsHttpConnectionMgr::ProcessNewTransacti
         if (!ent)
             return NS_ERROR_OUT_OF_MEMORY;
         mCT.Put(ci->HashKey(), ent);
     }
 
     // SPDY coalescing of hostnames means we might redirect from this
     // connection entry onto the preferred one.
     nsConnectionEntry *preferredEntry = GetSpdyPreferred(ent);
-    if (preferredEntry) {
+    if (preferredEntry && (preferredEntry != ent)) {
         LOG(("nsHttpConnectionMgr::ProcessNewTransaction trans=%p "
              "redirected via coalescing from %s to %s\n", trans,
              ent->mConnInfo->Host(), preferredEntry->mConnInfo->Host()));
+
         ent = preferredEntry;
     }
 
     // If we are doing a force reload then close out any existing conns
     // to this host so that changes in DNS, LBs, etc.. are reflected
     if (caps & NS_HTTP_CLEAR_KEEPALIVES)
         ClosePersistentConnections(ent);
 
@@ -1304,16 +1387,22 @@ nsHttpConnectionMgr::ProcessNewTransacti
     else {
         rv = DispatchTransaction(ent, trans, caps, conn);
         NS_RELEASE(conn);
     }
 
     return rv;
 }
 
+// This function tries to dispatch the pending spdy transactions on
+// the connection entry sent in as an argument. It will do so on the
+// active spdy connection either in that same entry or in the
+// redirected 'preferred' entry for the same coalescing hash key if
+// coalescing is enabled.
+
 void
 nsHttpConnectionMgr::ProcessSpdyPendingQ(nsConnectionEntry *ent)
 {
     nsHttpConnection *conn = GetSpdyPreferredConn(ent);
     if (!conn)
         return;
 
     for (PRInt32 index = ent->mPendingQ.Length() - 1;
@@ -1352,28 +1441,20 @@ nsHttpConnectionMgr::ProcessSpdyPendingQ
 nsHttpConnection *
 nsHttpConnectionMgr::GetSpdyPreferredConn(nsConnectionEntry *ent)
 {
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     NS_ABORT_IF_FALSE(ent, "no connection entry");
 
     nsConnectionEntry *preferred = GetSpdyPreferred(ent);
 
-    // this entry is spdy-enabled if it is a redirect to another spdy host
-    if (preferred && preferred != ent) {
+    // this entry is spdy-enabled if it is involved in a redirect
+    if (preferred)
         ent->mUsingSpdy = true;
-        ent->mSpdyRedir = true;
-    }
-    else {
-        ent->mSpdyRedir = false;
-        // don't clear usingSpdy, that will be reset in ReportSpdyConnection
-        // if it no longer applies
-    }
-
-    if (!preferred)
+    else
         preferred = ent;
     
     nsHttpConnection *conn = nsnull;
     
     if (preferred->mUsingSpdy) {
         for (PRUint32 index = 0;
              index < preferred->mActiveConns.Length();
              ++index) {
@@ -1417,18 +1498,19 @@ nsHttpConnectionMgr::OnMsgNewTransaction
 void
 nsHttpConnectionMgr::OnMsgReschedTransaction(PRInt32 priority, void *param)
 {
     LOG(("nsHttpConnectionMgr::OnMsgNewTransaction [trans=%p]\n", param));
 
     nsHttpTransaction *trans = (nsHttpTransaction *) param;
     trans->SetPriority(priority);
 
-    nsHttpConnectionInfo *ci = trans->ConnectionInfo();
-    nsConnectionEntry *ent = mCT.Get(ci->HashKey());
+    nsConnectionEntry *ent = LookupConnectionEntry(trans->ConnectionInfo(),
+                                                   nsnull, trans);
+
     if (ent) {
         PRInt32 index = ent->mPendingQ.IndexOf(trans);
         if (index >= 0) {
             ent->mPendingQ.RemoveElementAt(index);
             InsertTransactionSorted(ent->mPendingQ, trans);
         }
     }
 
@@ -1445,18 +1527,19 @@ nsHttpConnectionMgr::OnMsgCancelTransact
     // if the transaction owns a connection and the transaction is not done,
     // then ask the connection to close the transaction.  otherwise, close the
     // transaction directly (removing it from the pending queue first).
     //
     nsAHttpConnection *conn = trans->Connection();
     if (conn && !trans->IsDone())
         conn->CloseTransaction(trans, reason);
     else {
-        nsHttpConnectionInfo *ci = trans->ConnectionInfo();
-        nsConnectionEntry *ent = mCT.Get(ci->HashKey());
+        nsConnectionEntry *ent = LookupConnectionEntry(trans->ConnectionInfo(),
+                                                       nsnull, trans);
+
         if (ent) {
             PRInt32 index = ent->mPendingQ.IndexOf(trans);
             if (index >= 0) {
                 ent->mPendingQ.RemoveElementAt(index);
                 nsHttpTransaction *temp = trans;
                 NS_RELEASE(temp); // b/c NS_RELEASE nulls its argument!
             }
         }
@@ -1513,23 +1596,28 @@ nsHttpConnectionMgr::OnMsgReclaimConnect
     nsHttpConnection *conn = (nsHttpConnection *) param;
 
     // 
     // 1) remove the connection from the active list
     // 2) if keep-alive, add connection to idle list
     // 3) post event to process the pending transaction queue
     //
 
-    nsHttpConnectionInfo *ci = conn->ConnectionInfo();
-    NS_ADDREF(ci);
+    nsConnectionEntry *ent = LookupConnectionEntry(conn->ConnectionInfo(),
+                                                   conn, nsnull);
+    nsHttpConnectionInfo *ci = nsnull;
 
-    nsConnectionEntry *ent = mCT.Get(ci->HashKey());
+    if (!ent) {
+        // this should never happen
+        NS_ASSERTION(ent, "no connection entry");
+        NS_ADDREF(ci = conn->ConnectionInfo());
+    }
+    else {
+        NS_ADDREF(ci = ent->mConnInfo);
 
-    NS_ASSERTION(ent, "no connection entry");
-    if (ent) {
         // 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->mUsingSpdy)
             conn->DontReuse();
 
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -172,17 +172,16 @@ private:
     // pending transactions.
     //
     struct nsConnectionEntry
     {
         nsConnectionEntry(nsHttpConnectionInfo *ci)
           : mConnInfo(ci),
             mUsingSpdy(false),
             mTestedSpdy(false),
-            mSpdyRedir(false),
             mSpdyPreferred(false)
         {
             NS_ADDREF(mConnInfo);
         }
         ~nsConnectionEntry();
 
         nsHttpConnectionInfo        *mConnInfo;
         nsTArray<nsHttpTransaction*> mPendingQ;    // pending transaction queue
@@ -191,24 +190,27 @@ private:
         nsTArray<nsHalfOpenSocket*>  mHalfOpens;
 
         // Spdy sometimes resolves the address in the socket manager in order
         // to re-coalesce sharded HTTP hosts. The dotted decimal address is
         // combined with the Anonymous flag from the connection information
         // to build the hash key for hosts in the same ip pool.
         //
         // When a set of hosts are coalesced together one of them is marked
-        // mSpdyPreferred, and the others are marked mSpdyRedir. The mapping is
-        // maintained in the conection manager mSpdyPreferred hash.
+        // mSpdyPreferred. The mapping is maintained in the connection mananger
+        // mSpdyPreferred hash.
         //
         nsCString mCoalescingKey;
 
+        // To have the UsingSpdy flag means some host with the same hash information
+        // has done NPN=spdy/2 at some point. It does not mean every connection
+        // is currently using spdy.
         bool mUsingSpdy;
+
         bool mTestedSpdy;
-        bool mSpdyRedir;
         bool mSpdyPreferred;
         nsCOMPtr<nsIX509Cert3> mCert;
     };
 
     // nsConnectionHandle
     //
     // thin wrapper around a real connection, used to keep track of references
     // to the connection to determine when the connection may be reused.  the
@@ -320,16 +322,19 @@ private:
     void     RecvdConnect();
 
     // Manage the preferred spdy connection entry for this address
     nsConnectionEntry *GetSpdyPreferred(nsConnectionEntry *aOriginalEntry);
     void               SetSpdyPreferred(nsConnectionEntry *ent);
     void               RemoveSpdyPreferred(nsACString &aDottedDecimal);
     nsHttpConnection  *GetSpdyPreferredConn(nsConnectionEntry *ent);
     nsDataHashtable<nsCStringHashKey, nsConnectionEntry *>   mSpdyPreferredHash;
+    nsConnectionEntry *LookupConnectionEntry(nsHttpConnectionInfo *ci,
+                                             nsHttpConnection *conn,
+                                             nsHttpTransaction *trans);
 
     void               ProcessSpdyPendingQ(nsConnectionEntry *ent);
     void               ProcessSpdyPendingQ();
     static PLDHashOperator ProcessSpdyPendingQCB(
         const nsACString &key, nsAutoPtr<nsConnectionEntry> &ent,
         void *closure);
 
     // message handlers have this signature