Bug 1050329 - part 1 - Re-start transactions on totally busted h2 sessions. r=francois,dragana
authorNicholas Hurley <hurley@mozilla.com>
Thu, 25 Oct 2018 20:46:24 +0000
changeset 443051 dd75cefee794aaaef19940f717f69fc70f9a3ff0
parent 443050 336552a30e06dcf6e557163594b87165d2c9b819
child 443052 8ee3ec2214429e42c473844b86c8c1cbd0d94d15
push id34935
push userccoroiu@mozilla.com
push dateFri, 26 Oct 2018 04:43:55 +0000
treeherdermozilla-central@d0b577458d53 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfrancois, dragana
bugs1050329
milestone65.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 1050329 - part 1 - Re-start transactions on totally busted h2 sessions. r=francois,dragana Previously, we would just let these fail. But, when a peer claims to speak h2 via ALPN, and then plainly doesn't speak h2 (by not doing the opening handshake properly), we should re-try any transactions dispatched to that session using http/1.1 only. No use in giving the user a horrible experience. We will also collect telemetry on how often we have sessions where this happens, so we can see how big of a problem this is (and thus if we need to do any kind of outreach). Depends on D8432 Differential Revision: https://phabricator.services.mozilla.com/D8433
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/Http2Session.h
toolkit/components/telemetry/Histograms.json
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -110,16 +110,17 @@ Http2Session::Http2Session(nsISocketTran
   , mWaitingForSettingsAck(false)
   , mGoAwayOnPush(false)
   , mUseH2Deps(false)
   , mAttemptingEarlyData(attemptingEarlyData)
   , mOriginFrameActivated(false)
   , mTlsHandshakeFinished(false)
   , mCheckNetworkStallsWithTFO(false)
   , mLastRequestBytesSentTime(0)
+  , mPeerFailedHandshake(false)
   , mTrrStreams(0)
 {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
   static uint64_t sSerial;
   mSerial = ++sSerial;
 
   LOG3(("Http2Session::Http2Session %p serial=0x%" PRIX64 "\n", this, mSerial));
@@ -178,16 +179,17 @@ Http2Session::~Http2Session()
     Telemetry::Accumulate(Telemetry::DNS_TRR_REQUEST_PER_CONN, mTrrStreams);
   }
   Telemetry::Accumulate(Telemetry::SPDY_PARALLEL_STREAMS, mConcurrentHighWater);
   Telemetry::Accumulate(Telemetry::SPDY_REQUEST_PER_CONN, (mNextStreamID - 1) / 2);
   Telemetry::Accumulate(Telemetry::SPDY_SERVER_INITIATED_STREAMS,
                         mServerPushedResources);
   Telemetry::Accumulate(Telemetry::SPDY_GOAWAY_LOCAL, mClientGoAwayReason);
   Telemetry::Accumulate(Telemetry::SPDY_GOAWAY_PEER, mPeerGoAwayReason);
+  Telemetry::Accumulate(Telemetry::HTTP2_FAIL_BEFORE_SETTINGS, mPeerFailedHandshake);
 }
 
 inline nsresult
 Http2Session::SessionError(enum errorType reason)
 {
   mGoAwayReason = reason;
   return NS_ERROR_ILLEGAL_VALUE;
 }
@@ -3151,16 +3153,24 @@ Http2Session::WriteSegmentsAgain(nsAHttp
       LOG3(("Expected CONTINUATION of PUSH PROMISE for ID 0x%X\n",
             mExpectedPushPromiseID));
       return SessionError(PROTOCOL_ERROR);
     }
 
     if (mDownstreamState == BUFFERING_OPENING_SETTINGS &&
         mInputFrameType != FRAME_TYPE_SETTINGS) {
       LOG3(("First Frame Type Must Be Settings\n"));
+      mPeerFailedHandshake = true;
+      // Go through and re-start all of our transactions with h2 disabled.
+      for (auto iter = mStreamTransactionHash.Iter(); !iter.Done(); iter.Next()) {
+        nsAutoPtr<Http2Stream>& stream = iter.Data();
+        stream->Transaction()->DisableSpdy();
+        CloseStream(stream, NS_ERROR_NET_RESET);
+      }
+      mStreamTransactionHash.Clear();
       return SessionError(PROTOCOL_ERROR);
     }
 
     if (mInputFrameType != FRAME_TYPE_DATA) { // control frame
       EnsureBuffer(mInputFrameBuffer, mInputFrameDataSize + kFrameHeaderBytes,
                    kFrameHeaderBytes, mInputFrameBufferSize);
       ChangeDownstreamState(BUFFERING_CONTROL_FRAME);
     } else if (mInputFrameFlags & kFlag_PADDED) {
--- a/netwerk/protocol/http/Http2Session.h
+++ b/netwerk/protocol/http/Http2Session.h
@@ -564,16 +564,18 @@ private:
   // A h2 session will be created before all socket events are trigered,
   // e.g. NS_NET_STATUS_TLS_HANDSHAKE_ENDED and for TFO many others.
   // We should propagate this events to the first nsHttpTransaction.
   RefPtr<nsHttpTransaction> mFirstHttpTransaction;
   bool mTlsHandshakeFinished;
 
   bool mCheckNetworkStallsWithTFO;
   PRIntervalTime mLastRequestBytesSentTime;
+
+  bool mPeerFailedHandshake;
 private:
 /// connect tunnels
   void DispatchOnTunnel(nsAHttpTransaction *, nsIInterfaceRequestor *);
   void CreateTunnel(nsHttpTransaction *, nsHttpConnectionInfo *, nsIInterfaceRequestor *);
   void RegisterTunnel(Http2Stream *);
   void UnRegisterTunnel(Http2Stream *);
   uint32_t FindTunnelCount(nsHttpConnectionInfo *);
   nsDataHashtable<nsCStringHashKey, uint32_t> mTunnelHash;
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -2545,16 +2545,24 @@
     "expires_in_version": "never",
     "kind": "exponential",
     "high": 16384,
     "n_buckets": 100,
     "description": "HPACK: peak size in bytes of the table",
     "alert_emails": ["necko@mozilla.com", "hurley@mozilla.com"],
     "bug_numbers": [1296280]
   },
+  "HTTP2_FAIL_BEFORE_SETTINGS": {
+    "record_in_processes": ["main"],
+    "expires_in_version": "never",
+    "kind": "boolean",
+    "description": "Whether an HTTP/2 session failed because the peer did not handshake properly",
+    "alert_emails": ["hurley@mozilla.com"],
+    "bug_numbers": [1050329]
+  },
   "HTTP_CHANNEL_DISPOSITION" : {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["necko@mozilla.com"],
     "bug_numbers": [1341128],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 16,
     "releaseChannelCollection": "opt-out",