Bug 749209 - Happy Eyeballs implementation still not quite right, r=mcmanus a=akeybl
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 22 May 2012 22:12:40 +0200
changeset 95948 762d16222aba62fd45c00ca98262ae5856e39133
parent 95947 1851b8c13061fe22eb15a90a9284ec5b19dec808
child 95949 49bd90effd98d2e8d51f6c43c782206e57aad47d
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus, akeybl
bugs749209
milestone14.0a2
Bug 749209 - Happy Eyeballs implementation still not quite right, r=mcmanus a=akeybl
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -1080,36 +1080,51 @@ nsHttpConnectionMgr::AtActiveConnectionL
     for (i=0; i<totalCount; ++i) {
         conn = ent->mActiveConns[i];
         if (conn->IsKeepAlive()) // XXX make sure this is thread-safe
             persistCount++;
     }
 
     // Add in the in-progress tcp connections, we will assume they are
     // keepalive enabled.
-    totalCount += ent->mHalfOpens.Length();
-    persistCount += ent->mHalfOpens.Length();
+    PRUint32 pendingHalfOpens = 0;
+    for (i = 0; i < ent->mHalfOpens.Length(); ++i) {
+        nsHalfOpenSocket *halfOpen = ent->mHalfOpens[i];
+
+        // Exclude half-open's that has already created a usable connection.
+        // This prevents the limit being stuck on ipv6 connections that 
+        // eventually time out after typical 21 seconds of no ACK+SYN reply.
+        if (halfOpen->HasConnected())
+            continue;
+
+        ++pendingHalfOpens;
+    }
     
+    totalCount += pendingHalfOpens;
+    persistCount += pendingHalfOpens;
+
     LOG(("   total=%d, persist=%d\n", totalCount, persistCount));
 
     PRUint16 maxConns;
     PRUint16 maxPersistConns;
 
     if (ci->UsingHttpProxy() && !ci->UsingSSL()) {
         maxConns = mMaxConnsPerProxy;
         maxPersistConns = mMaxPersistConnsPerProxy;
     }
     else {
         maxConns = mMaxConnsPerHost;
         maxPersistConns = mMaxPersistConnsPerHost;
     }
 
     // use >= just to be safe
-    return (totalCount >= maxConns) || ( (caps & NS_HTTP_ALLOW_KEEPALIVE) &&
-                                         (persistCount >= maxPersistConns) );
+    bool result = (totalCount >= maxConns) || ( (caps & NS_HTTP_ALLOW_KEEPALIVE) &&
+                                              (persistCount >= maxPersistConns) );
+    LOG(("  result: %s", result ? "true" : "false"));
+    return result;
 }
 
 void
 nsHttpConnectionMgr::ClosePersistentConnections(nsConnectionEntry *ent)
 {
     LOG(("nsHttpConnectionMgr::ClosePersistentConnections [ci=%s]\n",
          ent->mConnInfo->HashKey().get()));
     while (ent->mIdleConns.Length()) {
@@ -2163,17 +2178,18 @@ NS_IMPL_THREADSAFE_ISUPPORTS4(nsHttpConn
                               nsITransportEventSink,
                               nsIInterfaceRequestor,
                               nsITimerCallback)
 
 nsHttpConnectionMgr::
 nsHalfOpenSocket::nsHalfOpenSocket(nsConnectionEntry *ent,
                                    nsHttpTransaction *trans)
     : mEnt(ent),
-      mTransaction(trans)
+      mTransaction(trans),
+      mHasConnected(false)
 {
     NS_ABORT_IF_FALSE(ent && trans, "constructor with null arguments");
     LOG(("Creating nsHalfOpenSocket [this=%p trans=%p ent=%s]\n",
          this, trans, ent->mConnInfo->Host()));
 }
 
 nsHttpConnectionMgr::nsHalfOpenSocket::~nsHalfOpenSocket()
 {
@@ -2322,19 +2338,23 @@ nsHttpConnectionMgr::nsHalfOpenSocket::S
         // 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.
         nsresult rv;
         mSynTimer = do_CreateInstance(NS_TIMER_CONTRACTID, &rv);
         if (NS_SUCCEEDED(rv)) {
             mSynTimer->InitWithCallback(this, timeout, nsITimer::TYPE_ONE_SHOT);
-            LOG(("nsHalfOpenSocket::SetupBackupTimer()"));
+            LOG(("nsHalfOpenSocket::SetupBackupTimer() [this=%p]", this));
         }
     }
+    else if (timeout) {
+        LOG(("nsHalfOpenSocket::SetupBackupTimer() [this=%p],"
+             " transaction already done!", this));
+    }
 }
 
 void
 nsHttpConnectionMgr::nsHalfOpenSocket::CancelBackupTimer()
 {
     // If the syntimer is still armed, we can cancel it because no backup
     // socket should be formed at this point
     if (!mSynTimer)
@@ -2372,20 +2392,17 @@ nsHttpConnectionMgr::nsHalfOpenSocket::A
 }
 
 NS_IMETHODIMP // method for nsITimerCallback
 nsHttpConnectionMgr::nsHalfOpenSocket::Notify(nsITimer *timer)
 {
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     NS_ABORT_IF_FALSE(timer == mSynTimer, "wrong timer");
 
-    if (!gHttpHandler->ConnMgr()->
-        AtActiveConnectionLimit(mEnt, mTransaction->Caps())) {
-        SetupBackupStreams();
-    }
+    SetupBackupStreams();
 
     mSynTimer = nsnull;
     return NS_OK;
 }
 
 // method for nsIAsyncOutputStreamCallback
 NS_IMETHODIMP
 nsHttpConnectionMgr::
@@ -2444,16 +2461,20 @@ nsHalfOpenSocket::OnOutputStreamReady(ns
     }
 
     if (NS_FAILED(rv)) {
         LOG(("nsHalfOpenSocket::OnOutputStreamReady "
              "conn->init (%p) failed %x\n", conn.get(), rv));
         return rv;
     }
 
+    // This half-open socket has created a connection.  This flag excludes it
+    // from counter of actual connections used for checking limits.
+    mHasConnected = true;
+
     // if this is still in the pending list, remove it and dispatch it
     index = mEnt->mPendingQ.IndexOf(mTransaction);
     if (index != -1) {
         mEnt->mPendingQ.RemoveElementAt(index);
         nsHttpTransaction *temp = mTransaction;
         NS_RELEASE(temp);
         gHttpHandler->ConnMgr()->AddActiveConn(conn, mEnt);
         rv = gHttpHandler->ConnMgr()->DispatchTransaction(mEnt, mTransaction,
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -393,31 +393,35 @@ private:
         nsresult SetupPrimaryStreams();
         nsresult SetupBackupStreams();
         void     SetupBackupTimer();
         void     CancelBackupTimer();
         void     Abandon();
         
         nsHttpTransaction *Transaction() { return mTransaction; }
 
+        bool HasConnected() { return mHasConnected; }
+
     private:
         nsConnectionEntry              *mEnt;
         nsRefPtr<nsHttpTransaction>    mTransaction;
         nsCOMPtr<nsISocketTransport>   mSocketTransport;
         nsCOMPtr<nsIAsyncOutputStream> mStreamOut;
         nsCOMPtr<nsIAsyncInputStream>  mStreamIn;
 
         mozilla::TimeStamp             mPrimarySynStarted;
         mozilla::TimeStamp             mBackupSynStarted;
 
         // for syn retry
         nsCOMPtr<nsITimer>             mSynTimer;
         nsCOMPtr<nsISocketTransport>   mBackupTransport;
         nsCOMPtr<nsIAsyncOutputStream> mBackupStreamOut;
         nsCOMPtr<nsIAsyncInputStream>  mBackupStreamIn;
+
+        bool                           mHasConnected;
     };
     friend class nsHalfOpenSocket;
 
     //-------------------------------------------------------------------------
     // NOTE: these members may be accessed from any thread (use mReentrantMonitor)
     //-------------------------------------------------------------------------
 
     PRInt32                      mRef;