Bug 1362821 - Properly destroy a HalfOpeSocket that is used as a backup for a TFO connection if another H2 connection is established for the same host. r=mcmanus
authorDragana Damjanovic <dd.mozilla@gmail.com>
Mon, 08 May 2017 18:22:45 +0200
changeset 357112 b31a06dd49366305167651bcec29750d7dcb925c
parent 357111 0f9d66b6a6d90cc1b51f356601ad08178354ea19
child 357113 b21b974d60d3075ae24f6fb1bae75d0f122f28fc
child 357249 77584adad5266c17b01a6afffc255f78440242e9
push id31782
push userkwierso@gmail.com
push dateMon, 08 May 2017 23:07:35 +0000
treeherdermozilla-central@b21b974d60d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1362821
milestone55.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 1362821 - Properly destroy a HalfOpeSocket that is used as a backup for a TFO connection if another H2 connection is established for the same host. r=mcmanus
netwerk/base/nsSocketTransport2.cpp
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -1713,18 +1713,22 @@ nsSocketTransport::RecoverFromError()
 
     bool tryAgain = false;
     if (mFDFastOpenInProgress &&
         ((mCondition == NS_ERROR_CONNECTION_REFUSED) ||
          (mCondition == NS_ERROR_NET_TIMEOUT))) {
         // TCP Fast Open can be blocked by middle boxes so we will retry
         // without it.
         tryAgain = true;
-        MOZ_ASSERT(mFastOpenCallback);
-        mFastOpenCallback->SetFastOpenConnected(mCondition);
+        // If we cancel the connection because backup socket was successfully
+        // connected, mFDFastOpenInProgress will be true but mFastOpenCallback
+        // will be nullptr.
+        if (mFastOpenCallback) {
+            mFastOpenCallback->SetFastOpenConnected(mCondition);
+        }
         mFastOpenCallback = nullptr;
     } else {
 
         if ((mState == STATE_CONNECTING) && mDNSRecord &&
             mSocketTransportService->IsTelemetryEnabledAndNotSleepPhase()) {
             if (mNetAddr.raw.family == AF_INET) {
                 Telemetry::Accumulate(Telemetry::IPV4_AND_IPV6_ADDRESS_CONNECTIVITY,
                                       UNSUCCESSFUL_CONNECTING_TO_IPV4_ADDRESS);
@@ -2461,16 +2465,20 @@ nsSocketTransport::OpenOutputStream(uint
 }
 
 NS_IMETHODIMP
 nsSocketTransport::Close(nsresult reason)
 {
     if (NS_SUCCEEDED(reason))
         reason = NS_BASE_STREAM_CLOSED;
 
+    if (mFastOpenCallback) {
+        mFastOpenCallback->SetFastOpenConnected(reason);
+        mFastOpenCallback = nullptr;
+    }
     mInput.CloseWithStatus(reason);
     mOutput.CloseWithStatus(reason);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSocketTransport::GetSecurityInfo(nsISupports **secinfo)
 {
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -784,18 +784,31 @@ nsHttpConnectionMgr::UpdateCoalescingFor
         }
         listOfWeakConns->AppendElement(
             do_GetWeakReference(static_cast<nsISupportsWeakReference*>(newConn)));
     }
 
     // Cancel any other pending connections - their associated transactions
     // are in the pending queue and will be dispatched onto this new connection
     for (int32_t index = ent->mHalfOpens.Length() - 1; index >= 0; --index) {
+        nsHalfOpenSocket *half = ent->mHalfOpens[index];
         LOG(("UpdateCoalescingForNewConn() forcing halfopen abandon %p\n",
-             ent->mHalfOpens[index]));
+             half));
+
+        RefPtr<nsHalfOpenSocket> deleteProtector;
+        if (half->IsFastOpenBackupHalfOpen()) {
+            LOG(("UpdateCoalescingForNewConn() halfOpen %p is in Fast Open "
+                 "state.\n", half));
+            // Hold pointer to halfOpen because it might go away if
+            // BackupSocketTransport has not ben started yet.
+            // On the other hand we want to call Abandon if
+            // BackupSocketTransport was started to clean up everything.
+            deleteProtector = half;
+            half->CancelFastOpenConnection();
+        }
         ent->mHalfOpens[index]->Abandon();
     }
 
     if (ent->mActiveConns.Length() > 1) {
         // this is a new connection that can be coalesced onto. hooray!
         // if there are other connection to this entry (e.g.
         // some could still be handshaking, shutting down, etc..) then close
         // them down after any transactions that are on them are complete.
@@ -2835,16 +2848,21 @@ nsHttpConnectionMgr::TimeoutTick()
         if (ent->mHalfOpens.Length()) {
             TimeStamp currentTime = TimeStamp::Now();
             double maxConnectTime_ms = gHttpHandler->ConnectTimeout();
 
             for (uint32_t index = ent->mHalfOpens.Length(); index > 0; ) {
                 index--;
 
                 nsHalfOpenSocket *half = ent->mHalfOpens[index];
+                if (half->IsFastOpenBackupHalfOpen()) {
+                    // this transport belongs to a fast open connection.
+                    // It will be canceled through that connection.
+                    continue;
+                }
                 double delta = half->Duration(currentTime);
                 // If the socket has timed out, close it so the waiting
                 // transaction will get the proper signal.
                 if (delta > maxConnectTime_ms) {
                     LOG(("Force timeout of half open to %s after %.2fms.\n",
                          ent->mConnInfo->HashKey().get(), delta));
                     if (half->SocketTransport()) {
                         half->SocketTransport()->Close(NS_ERROR_NET_TIMEOUT);
@@ -3602,16 +3620,49 @@ nsHalfOpenSocket::SetFastOpenStatus(uint
     mConnectionNegotiatingFastOpen->SetFastOpenStatus(tfoStatus);
     if (mConnectionNegotiatingFastOpen->Transaction()->QueryHttpTransaction()) {
         mConnectionNegotiatingFastOpen->Transaction()->SetFastOpenStatus(tfoStatus);
     }
 }
 
 void
 nsHttpConnectionMgr::
+nsHalfOpenSocket::CancelFastOpenConnection()
+{
+    // This must be called on a halfOpenSocket that has
+    // mConnectionNegotiatingFastOpen.
+    if (!mConnectionNegotiatingFastOpen) {
+        return;
+    }
+
+    // This function will cancel the mConnectionNegotiatingFastOpen connection
+    // and return transaction to the pending queue.
+    mSocketTransport->SetFastOpenCallback(nullptr);
+    RefPtr<nsAHttpTransaction> trans =
+        mConnectionNegotiatingFastOpen->CloseConnectionFastOpenTakesTooLongOrError(true);
+    mConnectionNegotiatingFastOpen = nullptr;
+    mSocketTransport = nullptr;
+    mStreamOut = nullptr;
+    mStreamIn = nullptr;
+
+    if (trans && trans->QueryHttpTransaction()) {
+        RefPtr<PendingTransactionInfo> pendingTransInfo =
+            new PendingTransactionInfo(trans->QueryHttpTransaction());
+
+        if (trans->Caps() & NS_HTTP_URGENT_START) {
+            gHttpHandler->ConnMgr()->InsertTransactionSorted(mEnt->mUrgentStartQ,
+                                                             pendingTransInfo);
+        } else {
+            mEnt->InsertTransaction(pendingTransInfo);
+        }
+    }
+}
+
+void
+nsHttpConnectionMgr::
 nsHalfOpenSocket::FastOpenNotSupported()
 {
   MOZ_ASSERT(mUsingFastOpen);
   gHttpHandler->SetFastOpenNotSupported();
 }
 
 nsresult
 nsHttpConnectionMgr::
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -374,16 +374,23 @@ private:
         bool Claim();
         void Unclaim();
 
         bool FastOpenEnabled() override;
         nsresult StartFastOpen() override;
         void SetFastOpenConnected(nsresult) override;
         void FastOpenNotSupported() override;
         void SetFastOpenStatus(uint8_t tfoStatus) override;
+
+        bool IsFastOpenBackupHalfOpen()
+        {
+            return mConnectionNegotiatingFastOpen;
+        }
+
+        void CancelFastOpenConnection();
     private:
         nsresult SetupConn(nsIAsyncOutputStream *out,
                            bool aFastOpen);
 
         // To find out whether |mTransaction| is still in the connection entry's
         // pending queue. If the transaction is found and |removeWhenFound| is
         // true, the transaction will be removed from the pending queue.
         already_AddRefed<PendingTransactionInfo>