Bug 1565518, emergency preferences to turn off individual bug fixes: 1563695, 1556491, 1562315, r=kershaw, a=jcristau
authorHonza Bambas <honzab.moz@firemni.cz>
Fri, 12 Jul 2019 12:55:41 +0000
changeset 537244 5d966839d6dd3779cea41a10128d630a0811c164
parent 537243 f0aaf22db57dd4e2121d93f02574e7b354f20b40
child 537245 3675e5309981a071d20d35db13b3fa3bec32fc98
push id2105
push userjcristau@mozilla.com
push dateWed, 17 Jul 2019 14:59:21 +0000
treeherdermozilla-release@035cb9d1676b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskershaw, jcristau
bugs1565518, 1563695, 1556491, 1562315
milestone68.0.1
Bug 1565518, emergency preferences to turn off individual bug fixes: 1563695, 1556491, 1562315, r=kershaw, a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D37848
modules/libpref/init/all.js
netwerk/protocol/http/TunnelUtils.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2328,16 +2328,19 @@ pref("network.http.tailing.delay-quantum
 // Upper limit for the calculated delay, prevents long standing and comet-like requests
 // tail forever.  This is in milliseconds as well.
 pref("network.http.tailing.delay-max", 6000);
 // Total limit we delay tailed requests since a page load beginning.
 pref("network.http.tailing.total-max", 45000);
 
 // Enable or disable the whole fix from bug 1563538
 pref("network.http.spdy.bug1563538", true);
+pref("network.http.spdy.bug1563695", true);
+pref("network.http.spdy.bug1562315", true);
+pref("network.http.spdy.bug1556491", true);
 
 pref("permissions.default.image",           1); // 1-Accept, 2-Deny, 3-dontAcceptForeign
 
 pref("network.proxy.type",                  5);
 pref("network.proxy.ftp",                   "");
 pref("network.proxy.ftp_port",              0);
 pref("network.proxy.http",                  "");
 pref("network.proxy.http_port",             0);
--- a/netwerk/protocol/http/TunnelUtils.cpp
+++ b/netwerk/protocol/http/TunnelUtils.cpp
@@ -127,16 +127,30 @@ void TLSFilterTransaction::Close(nsresul
 
   if (mTimer) {
     mTimer->Cancel();
     mTimer = nullptr;
   }
   mTransaction->Close(aReason);
   mTransaction = nullptr;
 
+  if (!gHttpHandler->Bug1563695()) {
+    RefPtr<NullHttpTransaction> baseTrans(do_QueryReferent(mWeakTrans));
+    SpdyConnectTransaction* trans =
+        baseTrans ? baseTrans->QuerySpdyConnectTransaction() : nullptr;
+
+    LOG(("TLSFilterTransaction::Close %p aReason=%" PRIx32 " trans=%p\n", this,
+         static_cast<uint32_t>(aReason), trans));
+
+    if (trans) {
+      trans->Close(aReason);
+      trans = nullptr;
+    }
+  }
+
   if (gHttpHandler->Bug1563538()) {
     if (NS_FAILED(aReason)) {
       mCloseReason = aReason;
     } else {
       mCloseReason = NS_BASE_STREAM_CLOSED;
     }
   } else {
     MOZ_ASSERT(NS_ERROR_UNEXPECTED == mCloseReason);
@@ -364,27 +378,48 @@ nsresult TLSFilterTransaction::WriteSegm
 
   if (!mTransaction) {
     return mCloseReason;
   }
 
   bool againBeforeWriteSegmentsCall = *again;
 
   mSegmentWriter = aWriter;
+
+  /*
+   * Bug 1562315 replaced TLSFilterTransaction::WriteSegments with
+   * WriteSegmentsAgain and the call of WriteSegments on the associated
+   * transaction was replaced with WriteSegmentsAgain call.
+   *
+   * When TLSFilterTransaction::WriteSegmentsAgain was called from outside, it
+   * internally called WriteSegments (nsAHttpTransaction default impl) and did
+   * not modify the 'again' out flag.
+   *
+   * So, to disable the bug fix, we only need two things:
+   * - call mTransaction->WriteSegments
+   * - don't modify 'again' (which is an automatic outcome of step 1, this
+   * method doesn't touch it itself)
+   */
+
   nsresult rv =
-      mTransaction->WriteSegmentsAgain(this, aCount, outCountWritten, again);
+      gHttpHandler->Bug1562315()
+          ? mTransaction->WriteSegmentsAgain(this, aCount, outCountWritten,
+                                             again)
+          : mTransaction->WriteSegments(this, aCount, outCountWritten);
+
   if (NS_SUCCEEDED(rv) && !(*outCountWritten)) {
     if (NS_FAILED(mFilterReadCode)) {
       // nsPipe turns failures into silent OK.. undo that!
       rv = mFilterReadCode;
       if (Connection() && (mFilterReadCode == NS_BASE_STREAM_WOULD_BLOCK)) {
         Unused << Connection()->ResumeRecv();
       }
     }
     if (againBeforeWriteSegmentsCall && !*again) {
+      // This code can never be reached if bug1562315 pref is off.
       LOG(
           ("TLSFilterTransaction %p called trans->WriteSegments which dropped "
            "the 'again' flag",
            this));
       // The transaction (=h2 session) wishes to break the loop.  There is a
       // pending close of the transaction that is being handled by the current
       // input stream of the session.  After cancellation of that transaction
       // the state of the stream will change and move the state machine of the
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -1807,17 +1807,17 @@ void nsHttpConnection::CloseTransaction(
   if (mUsingSpdyVersion != SpdyVersion::NONE) {
     DontReuse();
     // if !mSpdySession then mUsingSpdyVersion must be false for canreuse()
     mSpdySession->SetCleanShutdown(aIsShutdown);
     mUsingSpdyVersion = SpdyVersion::NONE;
     mSpdySession = nullptr;
   }
 
-  if (!mTransaction && mTLSFilter) {
+  if (!mTransaction && mTLSFilter && gHttpHandler->Bug1556491()) {
     // In case of a race when the transaction is being closed before the tunnel
     // is established we need to carry closing status on the proxied
     // transaction.
     // Not doing this leads to use of this closed connection to activate the
     // not closed transaction what will likely lead to a use of a closed ssl
     // socket and may cause a crash because of an unexpected use.
     //
     // There can possibly be two states: the actual transaction is still hanging
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -280,16 +280,19 @@ nsHttpHandler::nsHttpHandler()
       mTCPKeepaliveShortLivedEnabled(false),
       mTCPKeepaliveShortLivedTimeS(60),
       mTCPKeepaliveShortLivedIdleTimeS(10),
       mTCPKeepaliveLongLivedEnabled(false),
       mTCPKeepaliveLongLivedIdleTimeS(600),
       mEnforceH1Framing(FRAMECHECK_BARELY),
       mDefaultHpackBuffer(4096),
       mBug1563538(true),
+      mBug1563695(true),
+      mBug1562315(true),
+      mBug1556491(true),
       mMaxHttpResponseHeaderSize(393216),
       mFocusedWindowTransactionRatio(0.9f),
       mSpeculativeConnectEnabled(false),
       mUseFastOpen(true),
       mFastOpenConsecutiveFailureLimit(5),
       mFastOpenConsecutiveFailureCounter(0),
       mFastOpenStallsLimit(3),
       mFastOpenStallsCounter(0),
@@ -1903,16 +1906,34 @@ void nsHttpHandler::PrefsChanged(const c
   }
 
   if (PREF_CHANGED(HTTP_PREF("spdy.bug1563538"))) {
     rv = Preferences::GetBool(HTTP_PREF("spdy.bug1563538"), &cVar);
     if (NS_SUCCEEDED(rv)) {
       mBug1563538 = cVar;
     }
   }
+  if (PREF_CHANGED(HTTP_PREF("spdy.bug1563695"))) {
+    rv = Preferences::GetBool(HTTP_PREF("spdy.bug1563695"), &cVar);
+    if (NS_SUCCEEDED(rv)) {
+      mBug1563695 = cVar;
+    }
+  }
+  if (PREF_CHANGED(HTTP_PREF("spdy.bug1562315"))) {
+    rv = Preferences::GetBool(HTTP_PREF("spdy.bug1562315"), &cVar);
+    if (NS_SUCCEEDED(rv)) {
+      mBug1562315 = cVar;
+    }
+  }
+  if (PREF_CHANGED(HTTP_PREF("spdy.bug1556491"))) {
+    rv = Preferences::GetBool(HTTP_PREF("spdy.bug1556491"), &cVar);
+    if (NS_SUCCEEDED(rv)) {
+      mBug1556491 = cVar;
+    }
+  }
 
   // Enable HTTP response timeout if TCP Keepalives are disabled.
   mResponseTimeoutEnabled =
       !mTCPKeepaliveShortLivedEnabled && !mTCPKeepaliveLongLivedEnabled;
 
 #undef PREF_CHANGED
 #undef MULTI_PREF_CHANGED
 }
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -423,16 +423,19 @@ class nsHttpHandler final : public nsIHt
     return mRequestContextService.get();
   }
 
   void ShutdownConnectionManager();
 
   uint32_t DefaultHpackBuffer() const { return mDefaultHpackBuffer; }
 
   bool Bug1563538() const { return mBug1563538; }
+  bool Bug1563695() const { return mBug1563695; }
+  bool Bug1562315() const { return mBug1562315; }
+  bool Bug1556491() const { return mBug1556491; }
 
   uint32_t MaxHttpResponseHeaderSize() const {
     return mMaxHttpResponseHeaderSize;
   }
 
   float FocusedWindowTransactionRatio() const {
     return mFocusedWindowTransactionRatio;
   }
@@ -668,16 +671,19 @@ class nsHttpHandler final : public nsIHt
 
   nsCOMPtr<nsIRequestContextService> mRequestContextService;
 
   // The default size (in bytes) of the HPACK decompressor table.
   uint32_t mDefaultHpackBuffer;
 
   // Pref for the whole fix that bug provides
   Atomic<bool, Relaxed> mBug1563538;
+  Atomic<bool, Relaxed> mBug1563695;
+  Atomic<bool, Relaxed> mBug1562315;
+  Atomic<bool, Relaxed> mBug1556491;
 
   // The max size (in bytes) for received Http response header.
   uint32_t mMaxHttpResponseHeaderSize;
 
   // The ratio for dispatching transactions from the focused window.
   float mFocusedWindowTransactionRatio;
 
   // We may disable speculative connect if the browser has user certificates