Bug 1451293 - single thread access to ConnectionHandle::mConn r=mayhemer
authorKershaw Chang <kershaw@mozilla.com>
Mon, 04 Feb 2019 08:42:09 +0000
changeset 456602 83f419699bf159a363b14930e1f26aec2a392b10
parent 456601 1879755bd58de7756d8f3f66a24dfee66df328a9
child 456603 51df10abf1da898f73057f63bfb736c296ad9b8b
push id35496
push userbtara@mozilla.com
push dateMon, 04 Feb 2019 17:36:40 +0000
treeherdermozilla-central@be1beccfb86d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1451293
milestone67.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 1451293 - single thread access to ConnectionHandle::mConn r=mayhemer The goal of this patch is to make sure single thread access to ConnectionHandle::mConn. It contains: - Remove nsHttpTransaction::GetConnectionReference() - For the cases where we need the sticky connection, save the reference of the sticky connection's transaction instead. Then, the sticky connection will be extracted in socket thread and set it to the new transaction. Differential Revision: https://phabricator.services.mozilla.com/D17221
netwerk/protocol/http/AlternateServices.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpHandler.h
netwerk/protocol/http/nsHttpTransaction.cpp
netwerk/protocol/http/nsHttpTransaction.h
--- a/netwerk/protocol/http/AlternateServices.cpp
+++ b/netwerk/protocol/http/AlternateServices.cpp
@@ -735,17 +735,17 @@ TransactionObserver::TransactionObserver
 void TransactionObserver::Complete(nsHttpTransaction *aTrans, nsresult reason) {
   // socket thread
   MOZ_ASSERT(!NS_IsMainThread());
   if (mRanOnce) {
     return;
   }
   mRanOnce = true;
 
-  RefPtr<nsAHttpConnection> conn = aTrans->GetConnectionReference();
+  RefPtr<nsAHttpConnection> conn = aTrans->Connection();
   LOG(("TransactionObserver::Complete %p aTrans %p reason %" PRIx32
        " conn %p\n",
        this, aTrans, static_cast<uint32_t>(reason), conn.get()));
   if (!conn) {
     return;
   }
   HttpVersion version = conn->Version();
   mVersionOK = (((reason == NS_BASE_STREAM_CLOSED) || (reason == NS_OK)) &&
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -790,30 +790,32 @@ nsresult nsHttpChannel::ContinueConnect(
     LOG(("  mLoadFlags & LOAD_NO_NETWORK_IO"));
     return NS_ERROR_DOCUMENT_NOT_CACHED;
   }
 
   // hit the net...
   return DoConnect();
 }
 
-nsresult nsHttpChannel::DoConnect(nsAHttpConnection *aConn) {
-  LOG(("nsHttpChannel::DoConnect [this=%p]\n", this));
+nsresult nsHttpChannel::DoConnect(nsHttpTransaction *aTransWithStickyConn) {
+  LOG(("nsHttpChannel::DoConnect [this=%p, aTransWithStickyConn=%p]\n", this,
+       aTransWithStickyConn));
 
   nsresult rv = SetupTransaction();
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  // transfer ownership of connection to transaction
-  if (aConn) {
-    mTransaction->SetConnection(aConn);
-  }
-
-  rv = gHttpHandler->InitiateTransaction(mTransaction, mPriority);
+  if (aTransWithStickyConn) {
+    rv = gHttpHandler->InitiateTransactionWithStickyConn(
+        mTransaction, mPriority, aTransWithStickyConn);
+  } else {
+    rv = gHttpHandler->InitiateTransaction(mTransaction, mPriority);
+  }
+
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   rv = mTransactionPump->AsyncRead(this, nullptr);
   if (NS_FAILED(rv)) {
     return rv;
   }
@@ -5925,25 +5927,17 @@ NS_IMETHODIMP nsHttpChannel::CloseSticky
   }
 
   if (!(mCaps & NS_HTTP_STICKY_CONNECTION ||
         mTransaction->Caps() & NS_HTTP_STICKY_CONNECTION)) {
     LOG(("  not sticky"));
     return NS_OK;
   }
 
-  RefPtr<nsAHttpConnection> conn = mTransaction->GetConnectionReference();
-  if (!conn) {
-    LOG(("  no connection"));
-    return NS_OK;
-  }
-
-  // This turns the IsPersistent() indicator on the connection to false,
-  // and makes us throw it away in OnStopRequest.
-  conn->DontReuse();
+  mTransaction->DontReuseConnection();
   return NS_OK;
 }
 
 NS_IMETHODIMP nsHttpChannel::ConnectionRestartable(bool aRestartable) {
   LOG(("nsHttpChannel::ConnectionRestartable this=%p, restartable=%d", this,
        aRestartable));
   mAuthConnectionRestartable = aRestartable;
   return NS_OK;
@@ -7470,66 +7464,53 @@ nsHttpChannel::OnStopRequest(nsIRequest 
 
   if (mTransaction) {
     // determine if we should call DoAuthRetry
     bool authRetry = mAuthRetryPending && NS_SUCCEEDED(status);
     mStronglyFramed = mTransaction->ResponseIsComplete();
     LOG(("nsHttpChannel %p has a strongly framed transaction: %d", this,
          mStronglyFramed));
 
-    //
-    // grab references to connection in case we need to retry an
-    // authentication request over it or use it for an upgrade
-    // to another protocol.
-    //
+    // Save the reference of |mTransaction| to |transactionWithStickyConn|
+    // when it has a sticky connection.
+    // In the case we need to retry an authentication request, we need to
+    // reuse the connection of |transactionWithStickyConn|.
+    RefPtr<nsHttpTransaction> transactionWithStickyConn;
+    if (mCaps & NS_HTTP_STICKY_CONNECTION ||
+        mTransaction->Caps() & NS_HTTP_STICKY_CONNECTION) {
+      transactionWithStickyConn = mTransaction;
+      LOG(("  transaction %p has sticky connection",
+           transactionWithStickyConn.get()));
+    }
+
     // this code relies on the code in nsHttpTransaction::Close, which
     // tests for NS_HTTP_STICKY_CONNECTION to determine whether or not to
     // keep the connection around after the transaction is finished.
     //
-    RefPtr<nsAHttpConnection> conn;
     LOG(("  mAuthRetryPending=%d, status=%" PRIx32 ", sticky conn cap=%d",
          mAuthRetryPending, static_cast<uint32_t>(status),
          mCaps & NS_HTTP_STICKY_CONNECTION));
     // We must check caps for stickinness also on the transaction because it
     // might have been updated by the transaction itself during inspection of
     // the reposnse headers yet on the socket thread (found connection based
     // auth schema).
-    if ((mAuthRetryPending || NS_FAILED(status)) &&
-        (mCaps & NS_HTTP_STICKY_CONNECTION ||
-         mTransaction->Caps() & NS_HTTP_STICKY_CONNECTION)) {
-      conn = mTransaction->GetConnectionReference();
-      LOG(("  transaction %p provides connection %p", mTransaction.get(),
-           conn.get()));
-
-      if (conn) {
-        if (NS_FAILED(status)) {
-          // Close (don't reuse) the sticky connection if it's in the middle
-          // of an NTLM negotiation and this channel has been cancelled.
-          // There are proxy servers known to get confused when we send
-          // a new request over such a half-stated connection.
-          if (!mAuthConnectionRestartable) {
-            LOG(("  not reusing a half-authenticated sticky connection"));
-            conn->DontReuse();
-          }
-          conn = nullptr;
-        } else if (!conn->IsPersistent()) {
-          // This is so far a workaround to fix leak when reusing unpersistent
-          // connection for authentication retry. See bug 459620 comment 4
-          // for details.
-          LOG(("  connection is not persistent, not reusing it"));
-          conn = nullptr;
+
+    if ((mAuthRetryPending || NS_FAILED(status)) && transactionWithStickyConn) {
+      if (NS_FAILED(status)) {
+        // Close (don't reuse) the sticky connection if it's in the middle
+        // of an NTLM negotiation and this channel has been cancelled.
+        // There are proxy servers known to get confused when we send
+        // a new request over such a half-stated connection.
+        if (!mAuthConnectionRestartable) {
+          LOG(("  not reusing a half-authenticated sticky connection"));
+          transactionWithStickyConn->DontReuseConnection();
         }
       }
     }
 
-    RefPtr<nsAHttpConnection> stickyConn;
-    if (mCaps & NS_HTTP_STICKY_CONNECTION) {
-      stickyConn = mTransaction->GetConnectionReference();
-    }
-
     mTransferSize = mTransaction->GetTransferSize();
 
     // If we are using the transaction to serve content, we also save the
     // time since async open in the cache entry so we can compare telemetry
     // between cache and net response.
     // Do not store the time of conditional requests because even if we
     // fetch the data from the server, the time includes loading of the old
     // cache entry which would skew the network load time.
@@ -7561,42 +7542,44 @@ nsHttpChannel::OnStopRequest(nsIRequest 
       mTransactionTimings.domainLookupEnd = mDNSPrefetch->EndTimestamp();
     }
     mDNSPrefetch = nullptr;
 
     // handle auth retry...
     if (authRetry) {
       mAuthRetryPending = false;
       auto continueOSR = [authRetry, isFromNet, contentComplete,
-                          stickyConn{std::move(stickyConn)}](auto *self,
-                                                             nsresult aStatus) {
+                          transactionWithStickyConn](auto *self,
+                                                     nsresult aStatus) {
         return self->ContinueOnStopRequestAfterAuthRetry(
-            aStatus, authRetry, isFromNet, contentComplete, stickyConn);
+            aStatus, authRetry, isFromNet, contentComplete,
+            transactionWithStickyConn);
       };
-      status = DoAuthRetry(conn, continueOSR);
+      status = DoAuthRetry(transactionWithStickyConn, continueOSR);
       if (NS_SUCCEEDED(status)) {
         return NS_OK;
       }
     }
     return ContinueOnStopRequestAfterAuthRetry(status, authRetry, isFromNet,
-                                               contentComplete, stickyConn);
+                                               contentComplete,
+                                               transactionWithStickyConn);
   }
 
   return ContinueOnStopRequest(status, isFromNet, contentComplete);
 }
 
 nsresult nsHttpChannel::ContinueOnStopRequestAfterAuthRetry(
     nsresult aStatus, bool aAuthRetry, bool aIsFromNet, bool aContentComplete,
-    nsAHttpConnection *aStickyConn) {
+    nsHttpTransaction *aTransWithStickyConn) {
   LOG(
       ("nsHttpChannel::ContinueOnStopRequestAfterAuthRetry "
        "[this=%p, aStatus=%" PRIx32
-       " aAuthRetry=%d, aIsFromNet=%d, aStickyConn=%p]\n",
+       " aAuthRetry=%d, aIsFromNet=%d, aTransWithStickyConn=%p]\n",
        this, static_cast<uint32_t>(aStatus), aAuthRetry, aIsFromNet,
-       aStickyConn));
+       aTransWithStickyConn));
 
   if (aAuthRetry && NS_SUCCEEDED(aStatus)) {
     return NS_OK;
   }
 
   // If DoAuthRetry failed, or if we have been cancelled since showing
   // the auth. dialog, then we need to send OnStartRequest now
   if (aAuthRetry || (mAuthRetryPending && NS_FAILED(aStatus))) {
@@ -7618,30 +7601,30 @@ nsresult nsHttpChannel::ContinueOnStopRe
   // if this transaction has been replaced, then bail.
   if (mTransactionReplaced) {
     LOG(("Transaction replaced\n"));
     // This was just the network check for a 304 response.
     mFirstResponseSource = RESPONSE_PENDING;
     return NS_OK;
   }
 
-  bool upgradeWebsocket = mUpgradeProtocolCallback && aStickyConn &&
+  bool upgradeWebsocket = mUpgradeProtocolCallback && aTransWithStickyConn &&
                           mResponseHead &&
                           ((mResponseHead->Status() == 101 &&
                             mResponseHead->Version() == HttpVersion::v1_1) ||
                            (mResponseHead->Status() == 200 &&
                             mResponseHead->Version() == HttpVersion::v2_0));
 
-  bool upgradeConnect = mUpgradeProtocolCallback && aStickyConn &&
+  bool upgradeConnect = mUpgradeProtocolCallback && aTransWithStickyConn &&
                         (mCaps & NS_HTTP_CONNECT_ONLY) && mResponseHead &&
                         mResponseHead->Status() == 200;
 
   if (upgradeWebsocket || upgradeConnect) {
     nsresult rv = gHttpHandler->ConnMgr()->CompleteUpgrade(
-        aStickyConn, mUpgradeProtocolCallback);
+        aTransWithStickyConn, mUpgradeProtocolCallback);
     if (NS_FAILED(rv)) {
       LOG(("  CompleteUpgrade failed with %08x", static_cast<uint32_t>(rv)));
     }
   }
 
   return ContinueOnStopRequest(aStatus, aIsFromNet, aContentComplete);
 }
 
@@ -8396,20 +8379,21 @@ nsHttpChannel::ResumeAt(uint64_t aStartP
        aStartPos, PromiseFlatCString(aEntityID).get()));
   mEntityID = aEntityID;
   mStartPos = aStartPos;
   mResuming = true;
   return NS_OK;
 }
 
 nsresult nsHttpChannel::DoAuthRetry(
-    nsAHttpConnection *conn,
+    nsHttpTransaction *aTransWithStickyConn,
     const std::function<nsresult(nsHttpChannel *, nsresult)>
         &aContinueOnStopRequestFunc) {
-  LOG(("nsHttpChannel::DoAuthRetry [this=%p]\n", this));
+  LOG(("nsHttpChannel::DoAuthRetry [this=%p, aTransWithStickyConn=%p]\n", this,
+       aTransWithStickyConn));
 
   MOZ_ASSERT(!mTransaction, "should not have a transaction");
 
   // Note that we don't have to toggle |mIsPending| anymore. See the reasons
   // below.
   // 1. We can't suspend the channel during "http-on-modify-request"
   // when |mIsPending| is false.
   // 2. We don't check |mIsPending| in SetRequestHeader now.
@@ -8422,25 +8406,25 @@ nsresult nsHttpChannel::DoAuthRetry(
   // the server response could have included cookies that must be sent with
   // this authentication attempt (bug 84794).
   // TODO: save cookies from auth response and send them here (bug 572151).
   AddCookiesToRequest();
 
   // notify "http-on-modify-request" observers
   CallOnModifyRequestObservers();
 
-  RefPtr<nsAHttpConnection> connRef(conn);
+  RefPtr<nsHttpTransaction> trans(aTransWithStickyConn);
   return CallOrWaitForResume(
-      [conn{std::move(connRef)}, aContinueOnStopRequestFunc](auto *self) {
-        return self->ContinueDoAuthRetry(conn, aContinueOnStopRequestFunc);
+      [trans{std::move(trans)}, aContinueOnStopRequestFunc](auto *self) {
+        return self->ContinueDoAuthRetry(trans, aContinueOnStopRequestFunc);
       });
 }
 
 nsresult nsHttpChannel::ContinueDoAuthRetry(
-    nsAHttpConnection *aConn,
+    nsHttpTransaction *aTransWithStickyConn,
     const std::function<nsresult(nsHttpChannel *, nsresult)>
         &aContinueOnStopRequestFunc) {
   LOG(("nsHttpChannel::ContinueDoAuthRetry [this=%p]\n", this));
 
   mIsPending = true;
 
   // get rid of the old response headers
   mResponseHead = nullptr;
@@ -8463,20 +8447,20 @@ nsresult nsHttpChannel::ContinueDoAuthRe
   } else {
     LOG(("  connection made non-restartable"));
     mCaps &= ~NS_HTTP_CONNECTION_RESTARTABLE;
   }
 
   // notify "http-on-before-connect" observers
   gHttpHandler->OnBeforeConnect(this);
 
-  RefPtr<nsAHttpConnection> connRef(aConn);
+  RefPtr<nsHttpTransaction> trans(aTransWithStickyConn);
   return CallOrWaitForResume(
-      [conn{std::move(connRef)}, aContinueOnStopRequestFunc](auto *self) {
-        nsresult rv = self->DoConnect(conn);
+      [trans{std::move(trans)}, aContinueOnStopRequestFunc](auto *self) {
+        nsresult rv = self->DoConnect(trans);
         return aContinueOnStopRequestFunc(self, rv);
       });
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIApplicationCacheChannel
 //-----------------------------------------------------------------------------
 
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -426,25 +426,28 @@ class nsHttpChannel final : public HttpB
   MOZ_MUST_USE nsresult ProcessPartialContent(
       const std::function<nsresult(nsHttpChannel *, nsresult)>
           &aContinueProcessResponseFunc);
   MOZ_MUST_USE nsresult
   ContinueProcessResponseAfterPartialContent(nsresult aRv);
   MOZ_MUST_USE nsresult OnDoneReadingPartialCacheEntry(bool *streamDone);
 
   MOZ_MUST_USE nsresult
-  DoAuthRetry(nsAHttpConnection *,
-              const std::function<nsresult(nsHttpChannel *, nsresult)> &aOuter);
-  MOZ_MUST_USE nsresult ContinueDoAuthRetry(
-      nsAHttpConnection *aConn,
-      const std::function<nsresult(nsHttpChannel *, nsresult)> &aOuter);
-  MOZ_MUST_USE nsresult DoConnect(nsAHttpConnection *aConn = nullptr);
+  DoAuthRetry(nsHttpTransaction *aTransWithStickyConn,
+              const std::function<nsresult(nsHttpChannel *, nsresult)>
+                  &aContinueOnStopRequestFunc);
+  MOZ_MUST_USE nsresult
+  ContinueDoAuthRetry(nsHttpTransaction *aTransWithStickyConn,
+                      const std::function<nsresult(nsHttpChannel *, nsresult)>
+                          &aContinueOnStopRequestFunc);
+  MOZ_MUST_USE nsresult
+  DoConnect(nsHttpTransaction *aTransWithStickyConn = nullptr);
   MOZ_MUST_USE nsresult ContinueOnStopRequestAfterAuthRetry(
       nsresult aStatus, bool aAuthRetry, bool aIsFromNet, bool aContentComplete,
-      nsAHttpConnection *aStickyConn);
+      nsHttpTransaction *aTransWithStickyConn);
   MOZ_MUST_USE nsresult ContinueOnStopRequest(nsresult status, bool aIsFromNet,
                                               bool aContentComplete);
 
   void HandleAsyncRedirectChannelToHttps();
   MOZ_MUST_USE nsresult StartRedirectChannelToHttps();
   MOZ_MUST_USE nsresult ContinueAsyncRedirectChannelToURI(nsresult rv);
   MOZ_MUST_USE nsresult OpenRedirectChannel(nsresult rv);
 
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -365,16 +365,47 @@ nsHttpConnectionMgr::Observe(nsISupports
 //-----------------------------------------------------------------------------
 
 nsresult nsHttpConnectionMgr::AddTransaction(nsHttpTransaction *trans,
                                              int32_t priority) {
   LOG(("nsHttpConnectionMgr::AddTransaction [trans=%p %d]\n", trans, priority));
   return PostEvent(&nsHttpConnectionMgr::OnMsgNewTransaction, priority, trans);
 }
 
+class NewTransactionData : public ARefBase {
+ public:
+  NewTransactionData(nsHttpTransaction *trans, int32_t priority,
+                     nsHttpTransaction *transWithStickyConn)
+      : mTrans(trans),
+        mPriority(priority),
+        mTransWithStickyConn(transWithStickyConn) {}
+
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(NewTransactionData, override)
+
+  RefPtr<nsHttpTransaction> mTrans;
+  int32_t mPriority;
+  RefPtr<nsHttpTransaction> mTransWithStickyConn;
+
+ private:
+  virtual ~NewTransactionData() = default;
+};
+
+nsresult nsHttpConnectionMgr::AddTransactionWithStickyConn(
+    nsHttpTransaction *trans, int32_t priority,
+    nsHttpTransaction *transWithStickyConn) {
+  LOG(
+      ("nsHttpConnectionMgr::AddTransactionWithStickyConn "
+       "[trans=%p %d transWithStickyConn=%p]\n",
+       trans, priority, transWithStickyConn));
+  RefPtr<NewTransactionData> data =
+      new NewTransactionData(trans, priority, transWithStickyConn);
+  return PostEvent(&nsHttpConnectionMgr::OnMsgNewTransactionWithStickyConn, 0,
+                   data);
+}
+
 nsresult nsHttpConnectionMgr::RescheduleTransaction(nsHttpTransaction *trans,
                                                     int32_t priority) {
   LOG(("nsHttpConnectionMgr::RescheduleTransaction [trans=%p %d]\n", trans,
        priority));
   return PostEvent(&nsHttpConnectionMgr::OnMsgReschedTransaction, priority,
                    trans);
 }
 
@@ -510,48 +541,48 @@ nsresult nsHttpConnectionMgr::GetSocketT
   return NS_OK;
 }
 
 nsresult nsHttpConnectionMgr::ReclaimConnection(nsHttpConnection *conn) {
   LOG(("nsHttpConnectionMgr::ReclaimConnection [conn=%p]\n", conn));
   return PostEvent(&nsHttpConnectionMgr::OnMsgReclaimConnection, 0, conn);
 }
 
-// A structure used to marshall 5 pointers across the various necessary
+// A structure used to marshall 6 pointers across the various necessary
 // threads to complete an HTTP upgrade.
 class nsCompleteUpgradeData : public ARefBase {
  public:
-  nsCompleteUpgradeData(nsAHttpConnection *aConn,
+  nsCompleteUpgradeData(nsHttpTransaction *aTrans,
                         nsIHttpUpgradeListener *aListener, bool aJsWrapped)
-      : mConn(aConn), mUpgradeListener(aListener), mJsWrapped(aJsWrapped) {}
+      : mTrans(aTrans), mUpgradeListener(aListener), mJsWrapped(aJsWrapped) {}
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsCompleteUpgradeData, override)
 
-  RefPtr<nsAHttpConnection> mConn;
+  RefPtr<nsHttpTransaction> mTrans;
   nsCOMPtr<nsIHttpUpgradeListener> mUpgradeListener;
 
   nsCOMPtr<nsISocketTransport> mSocketTransport;
   nsCOMPtr<nsIAsyncInputStream> mSocketIn;
   nsCOMPtr<nsIAsyncOutputStream> mSocketOut;
 
   bool mJsWrapped;
 
  private:
   virtual ~nsCompleteUpgradeData() = default;
 };
 
 nsresult nsHttpConnectionMgr::CompleteUpgrade(
-    nsAHttpConnection *aConn, nsIHttpUpgradeListener *aUpgradeListener) {
+    nsHttpTransaction *aTrans, nsIHttpUpgradeListener *aUpgradeListener) {
   // test if aUpgradeListener is a wrapped JsObject
   nsCOMPtr<nsIXPConnectWrappedJS> wrapper = do_QueryInterface(aUpgradeListener);
 
   bool wrapped = !!wrapper;
 
   RefPtr<nsCompleteUpgradeData> data =
-      new nsCompleteUpgradeData(aConn, aUpgradeListener, wrapped);
+      new nsCompleteUpgradeData(aTrans, aUpgradeListener, wrapped);
   return PostEvent(&nsHttpConnectionMgr::OnMsgCompleteUpgrade, 0, data);
 }
 
 nsresult nsHttpConnectionMgr::UpdateParam(nsParamName name, uint16_t value) {
   uint32_t param = (uint32_t(name) << 16) | uint32_t(value);
   return PostEvent(&nsHttpConnectionMgr::OnMsgUpdateParam,
                    static_cast<int32_t>(param), nullptr);
 }
@@ -2304,16 +2335,48 @@ void nsHttpConnectionMgr::OnMsgNewTransa
   LOG(("nsHttpConnectionMgr::OnMsgNewTransaction [trans=%p]\n", param));
 
   nsHttpTransaction *trans = static_cast<nsHttpTransaction *>(param);
   trans->SetPriority(priority);
   nsresult rv = ProcessNewTransaction(trans);
   if (NS_FAILED(rv)) trans->Close(rv);  // for whatever its worth
 }
 
+void nsHttpConnectionMgr::OnMsgNewTransactionWithStickyConn(int32_t priority,
+                                                            ARefBase *param) {
+  MOZ_ASSERT(OnSocketThread(), "not on socket thread");
+
+  NewTransactionData *data = static_cast<NewTransactionData *>(param);
+  LOG(
+      ("nsHttpConnectionMgr::OnMsgNewTransactionWithStickyConn "
+       "[trans=%p, transWithStickyConn=%p, conn=%p]\n",
+       data->mTrans.get(), data->mTransWithStickyConn.get(),
+       data->mTransWithStickyConn->Connection()));
+
+  MOZ_ASSERT(data->mTransWithStickyConn &&
+             data->mTransWithStickyConn->Caps() & NS_HTTP_STICKY_CONNECTION);
+
+  data->mTrans->SetPriority(data->mPriority);
+
+  RefPtr<nsAHttpConnection> conn = data->mTransWithStickyConn->Connection();
+  if (conn && conn->IsPersistent()) {
+    // This is so far a workaround to only reuse persistent
+    // connection for authentication retry. See bug 459620 comment 4
+    // for details.
+    LOG((" Reuse connection [%p] for transaction [%p]", conn.get(),
+         data->mTrans.get()));
+    data->mTrans->SetConnection(conn);
+  }
+
+  nsresult rv = ProcessNewTransaction(data->mTrans);
+  if (NS_FAILED(rv)) {
+    data->mTrans->Close(rv);  // for whatever its worth
+  }
+}
+
 static uint64_t TabIdForQueuing(nsAHttpTransaction *transaction) {
   return gHttpHandler->ActiveTabPriority()
              ? transaction->TopLevelOuterContentWindowId()
              : 0;
 }
 
 nsTArray<RefPtr<nsHttpConnectionMgr::PendingTransactionInfo>> *
 nsHttpConnectionMgr::GetTransactionPendingQHelper(nsConnectionEntry *ent,
@@ -2787,53 +2850,64 @@ void nsHttpConnectionMgr::OnMsgReclaimCo
     LOG(("  connection cannot be reused; closing connection\n"));
     conn->Close(NS_ERROR_ABORT);
   }
 
   OnMsgProcessPendingQ(0, ci);
 }
 
 void nsHttpConnectionMgr::OnMsgCompleteUpgrade(int32_t, ARefBase *param) {
-  nsCompleteUpgradeData *data = static_cast<nsCompleteUpgradeData *>(param);
-  MOZ_ASSERT(OnSocketThread() || (data->mJsWrapped == NS_IsMainThread()),
-             "not on socket thread");
-  LOG((
-      "nsHttpConnectionMgr::OnMsgCompleteUpgrade "
-      "this=%p conn=%p listener=%p wrapped=%d\n",
-      this, data->mConn.get(), data->mUpgradeListener.get(), data->mJsWrapped));
+  MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   nsresult rv = NS_OK;
-  if (!data->mSocketTransport) {
-    rv = data->mConn->TakeTransport(getter_AddRefs(data->mSocketTransport),
-                                    getter_AddRefs(data->mSocketIn),
-                                    getter_AddRefs(data->mSocketOut));
-  }
-
-  if (NS_SUCCEEDED(rv)) {
-    if (!data->mJsWrapped || !OnSocketThread()) {
-      rv = data->mUpgradeListener->OnTransportAvailable(
-          data->mSocketTransport, data->mSocketIn, data->mSocketOut);
-      if (NS_FAILED(rv)) {
-        LOG(
-            ("nsHttpConnectionMgr::OnMsgCompleteUpgrade "
-             "this=%p conn=%p listener=%p wrapped=%d\n",
-             this, data->mConn.get(), data->mUpgradeListener.get(),
-             data->mJsWrapped));
-      }
-    } else {
+  nsCompleteUpgradeData *data = static_cast<nsCompleteUpgradeData *>(param);
+  MOZ_ASSERT(data->mTrans && data->mTrans->Caps() & NS_HTTP_STICKY_CONNECTION);
+
+  RefPtr<nsAHttpConnection> conn(data->mTrans->Connection());
+  LOG(
+      ("nsHttpConnectionMgr::OnMsgCompleteUpgrade "
+       "conn=%p listener=%p wrapped=%d\n",
+       conn.get(), data->mUpgradeListener.get(), data->mJsWrapped));
+
+  if (!conn) {
+    return;
+  }
+
+  MOZ_ASSERT(!data->mSocketTransport);
+  rv = conn->TakeTransport(getter_AddRefs(data->mSocketTransport),
+                           getter_AddRefs(data->mSocketIn),
+                           getter_AddRefs(data->mSocketOut));
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  RefPtr<nsCompleteUpgradeData> upgradeData(data);
+  auto transportAvailableFunc = [upgradeData{std::move(upgradeData)}]() {
+    nsresult rv = upgradeData->mUpgradeListener->OnTransportAvailable(
+        upgradeData->mSocketTransport, upgradeData->mSocketIn,
+        upgradeData->mSocketOut);
+    if (NS_FAILED(rv)) {
       LOG(
-          ("nsHttpConnectionMgr::OnMsgCompleteUpgrade "
-           "this=%p conn=%p listener=%p wrapped=%d pass to main thread\n",
-           this, data->mConn.get(), data->mUpgradeListener.get(),
-           data->mJsWrapped));
-
-      nsCOMPtr<nsIRunnable> event = new ConnEvent(
-          this, &nsHttpConnectionMgr::OnMsgCompleteUpgrade, 0, param);
-      NS_DispatchToMainThread(event);
+          ("nsHttpConnectionMgr::OnMsgCompleteUpgrade OnTransportAvailable "
+           "failed. listener=%p\n",
+           upgradeData->mUpgradeListener.get()));
     }
+  };
+
+  if (data->mJsWrapped) {
+    LOG(
+        ("nsHttpConnectionMgr::OnMsgCompleteUpgrade "
+         "conn=%p listener=%p wrapped=%d pass to main thread\n",
+         conn.get(), data->mUpgradeListener.get(), data->mJsWrapped));
+    NS_DispatchToMainThread(
+        NS_NewRunnableFunction("net::nsHttpConnectionMgr::OnMsgCompleteUpgrade",
+                               transportAvailableFunc));
+  } else {
+    transportAvailableFunc();
   }
 }
 
 void nsHttpConnectionMgr::OnMsgUpdateParam(int32_t inParam, ARefBase *) {
   uint32_t param = static_cast<uint32_t>(inParam);
   uint16_t name = ((param)&0xFFFF0000) >> 16;
   uint16_t value = param & 0x0000FFFF;
 
@@ -3791,21 +3865,30 @@ void nsHttpConnectionMgr::OnMsgSpeculati
     MOZ_ASSERT(NS_SUCCEEDED(rv));
   } else {
     LOG(
         ("OnMsgSpeculativeConnect Transport "
          "not created due to existing connection count\n"));
   }
 }
 
-bool ConnectionHandle::IsPersistent() { return mConn->IsPersistent(); }
-
-bool ConnectionHandle::IsReused() { return mConn->IsReused(); }
-
-void ConnectionHandle::DontReuse() { mConn->DontReuse(); }
+bool ConnectionHandle::IsPersistent() {
+  MOZ_ASSERT(OnSocketThread());
+  return mConn->IsPersistent();
+}
+
+bool ConnectionHandle::IsReused() {
+  MOZ_ASSERT(OnSocketThread());
+  return mConn->IsReused();
+}
+
+void ConnectionHandle::DontReuse() {
+  MOZ_ASSERT(OnSocketThread());
+  mConn->DontReuse();
+}
 
 nsresult ConnectionHandle::PushBack(const char *buf, uint32_t bufLen) {
   return mConn->PushBack(buf, bufLen);
 }
 
 //////////////////////// nsHalfOpenSocket
 NS_IMPL_ADDREF(nsHttpConnectionMgr::nsHalfOpenSocket)
 NS_IMPL_RELEASE(nsHttpConnectionMgr::nsHalfOpenSocket)
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -101,16 +101,21 @@ class nsHttpConnectionMgr final : public
 
   // Stops timer used for the read timeout tick if there are no currently
   // active connections.
   void ConditionallyStopTimeoutTick();
 
   // adds a transaction to the list of managed transactions.
   MOZ_MUST_USE nsresult AddTransaction(nsHttpTransaction *, int32_t priority);
 
+  // Add a new transaction with a sticky connection from |transWithStickyConn|.
+  MOZ_MUST_USE nsresult
+  AddTransactionWithStickyConn(nsHttpTransaction *trans, int32_t priority,
+                               nsHttpTransaction *transWithStickyConn);
+
   // called to reschedule the given transaction.  it must already have been
   // added to the connection manager via AddTransaction.
   MOZ_MUST_USE nsresult RescheduleTransaction(nsHttpTransaction *,
                                               int32_t priority);
 
   // TOOD
   void UpdateClassOfServiceOnTransaction(nsHttpTransaction *,
                                          uint32_t classOfService);
@@ -156,18 +161,20 @@ class nsHttpConnectionMgr final : public
   // connection can be reused then it will be added to the idle list, else
   // it will be closed.
   MOZ_MUST_USE nsresult ReclaimConnection(nsHttpConnection *conn);
 
   // called by the main thread to execute the taketransport() logic on the
   // socket thread after a 101 response has been received and the socket
   // needs to be transferred to an expectant upgrade listener such as
   // websockets.
+  // @param aTrans: a transaction that contains a sticky connection. We'll
+  //                take the transport of this connection.
   MOZ_MUST_USE nsresult CompleteUpgrade(
-      nsAHttpConnection *aConn, nsIHttpUpgradeListener *aUpgradeListener);
+      nsHttpTransaction *aTrans, nsIHttpUpgradeListener *aUpgradeListener);
 
   // called to update a parameter after the connection manager has already
   // been initialized.
   MOZ_MUST_USE nsresult UpdateParam(nsParamName name, uint16_t value);
 
   // called from main thread to post a new request token bucket
   // to the socket thread
   MOZ_MUST_USE nsresult UpdateRequestTokenBucket(EventTokenBucket *aBucket);
@@ -661,16 +668,17 @@ class nsHttpConnectionMgr final : public
       nsTArray<RefPtr<PendingTransactionInfo>> &pendingQ,
       const nsHttpConnectionInfo *ci, const nsConnectionEntry *ent,
       nsresult reason);
 
   // message handlers
   void OnMsgShutdown(int32_t, ARefBase *);
   void OnMsgShutdownConfirm(int32_t, ARefBase *);
   void OnMsgNewTransaction(int32_t, ARefBase *);
+  void OnMsgNewTransactionWithStickyConn(int32_t, ARefBase *);
   void OnMsgReschedTransaction(int32_t, ARefBase *);
   void OnMsgUpdateClassOfServiceOnTransaction(int32_t, ARefBase *);
   void OnMsgCancelTransaction(int32_t, ARefBase *);
   void OnMsgCancelTransactions(int32_t, ARefBase *);
   void OnMsgProcessPendingQ(int32_t, ARefBase *);
   void OnMsgPruneDeadConnections(int32_t, ARefBase *);
   void OnMsgSpeculativeConnect(int32_t, ARefBase *);
   void OnMsgReclaimConnection(int32_t, ARefBase *);
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -235,16 +235,26 @@ class nsHttpHandler final : public nsIHt
   // Called to kick-off a new transaction, by default the transaction
   // will be put on the pending transaction queue if it cannot be
   // initiated at this time.  Callable from any thread.
   MOZ_MUST_USE nsresult InitiateTransaction(nsHttpTransaction *trans,
                                             int32_t priority) {
     return mConnMgr->AddTransaction(trans, priority);
   }
 
+  // This function is also called to kick-off a new transaction. But the new
+  // transaction will take a sticky connection from |transWithStickyConn|
+  // and reuse it.
+  MOZ_MUST_USE nsresult
+  InitiateTransactionWithStickyConn(nsHttpTransaction *trans, int32_t priority,
+                                    nsHttpTransaction *transWithStickyConn) {
+    return mConnMgr->AddTransactionWithStickyConn(trans, priority,
+                                                  transWithStickyConn);
+  }
+
   // Called to change the priority of an existing transaction that has
   // already been initiated.
   MOZ_MUST_USE nsresult RescheduleTransaction(nsHttpTransaction *trans,
                                               int32_t priority) {
     return mConnMgr->RescheduleTransaction(trans, priority);
   }
 
   void UpdateClassOfServiceOnTransaction(nsHttpTransaction *trans,
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -457,30 +457,16 @@ nsresult nsHttpTransaction::Init(
 }
 
 // This method should only be used on the socket thread
 nsAHttpConnection *nsHttpTransaction::Connection() {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
   return mConnection.get();
 }
 
-already_AddRefed<nsAHttpConnection>
-nsHttpTransaction::GetConnectionReference() {
-  if (mH2WSTransaction) {
-    // Need to let the websocket transaction/connection know we've reached
-    // this point so it can stop forwarding information through us and
-    // instead communicate directly with the websocket channel.
-    mH2WSTransaction->SetConnRefTaken();
-    mH2WSTransaction = nullptr;
-  }
-  MutexAutoLock lock(mLock);
-  RefPtr<nsAHttpConnection> connection(mConnection);
-  return connection.forget();
-}
-
 nsHttpResponseHead *nsHttpTransaction::TakeResponseHead() {
   MOZ_ASSERT(!mResponseHeadTaken, "TakeResponseHead called 2x");
 
   // Lock TakeResponseHead() against main thread
   MutexAutoLock lock(*nsHttp::GetLock());
 
   mResponseHeadTaken = true;
 
@@ -877,16 +863,32 @@ bool nsHttpTransaction::ShouldThrottle()
     // as throttleable because trackers and (specially) downloads should
     // keep throttling even under pressure.
     return false;
   }
 
   return true;
 }
 
+void nsHttpTransaction::DontReuseConnection() {
+  LOG(("nsHttpTransaction::DontReuseConnection %p\n", this));
+  if (!OnSocketThread()) {
+    LOG(("DontReuseConnection %p not on socket thread\n", this));
+    nsCOMPtr<nsIRunnable> event =
+        NewRunnableMethod("nsHttpTransaction::DontReuseConnection", this,
+                          &nsHttpTransaction::DontReuseConnection);
+    gSocketTransportService->Dispatch(event, NS_DISPATCH_NORMAL);
+    return;
+  }
+
+  if (mConnection) {
+    mConnection->DontReuse();
+  }
+}
+
 nsresult nsHttpTransaction::WriteSegments(nsAHttpSegmentWriter *writer,
                                           uint32_t count,
                                           uint32_t *countWritten) {
   LOG(("nsHttpTransaction::WriteSegments %p", this));
 
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   if (mTransactionDone) {
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -106,20 +106,16 @@ class nsHttpTransaction final : public n
   // Called to take ownership of the response headers; the transaction
   // will drop any reference to the response headers after this call.
   nsHttpResponseHead *TakeResponseHead();
 
   // Called to take ownership of the trailer headers.
   // Returning null if there is no trailer.
   nsHttpHeaderArray *TakeResponseTrailers();
 
-  // Provides a thread safe reference of the connection
-  // nsHttpTransaction::Connection should only be used on the socket thread
-  already_AddRefed<nsAHttpConnection> GetConnectionReference();
-
   // Called to set/find out if the transaction generated a complete response.
   bool ResponseIsComplete() { return mResponseIsComplete; }
   void SetResponseIsComplete() { mResponseIsComplete = true; }
 
   bool ProxyConnectFailed() { return mProxyConnectFailed; }
 
   void EnableKeepAlive() { mCaps |= NS_HTTP_ALLOW_KEEPALIVE; }
   void MakeSticky() { mCaps |= NS_HTTP_STICKY_CONNECTION; }
@@ -423,16 +419,18 @@ class nsHttpTransaction final : public n
   // Called by the connetion manager on the socket thread when reading for this
   // previously throttled transaction has to be resumed.
   void ResumeReading();
 
   // This examins classification of this transaction whether the Throttleable
   // class has been set while Leader, Unblocked, DontThrottle has not.
   bool EligibleForThrottling() const;
 
+  void DontReuseConnection();
+
  private:
   bool mSubmittedRatePacing;
   bool mPassedRatePacing;
   bool mSynchronousRatePaceRequest;
   nsCOMPtr<nsICancelable> mTokenBucketCancel;
 
  public:
   void SetClassOfService(uint32_t cos);