Bug 1364189 - Make sure not to retry socketTransaction if nsHttpConnectionMgr cancels it. r=mcmanus
authorDragana Damjanovic <dd.mozilla@gmail.com>
Wed, 24 May 2017 11:19:40 +0200
changeset 360320 c73ef4420951a1fad4b9d91d2216c3e3d3f31baf
parent 360319 fe8b783001368ba8e915884d7eb5f45ab7cb762f
child 360358 7fc3bfbb3e59ab0e065cc6160b4af9def63b40e6
push id90638
push userdd.mozilla@gmail.com
push dateWed, 24 May 2017 09:20:03 +0000
treeherdermozilla-inbound@c73ef4420951 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1364189
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 1364189 - Make sure not to retry socketTransaction if nsHttpConnectionMgr cancels it. r=mcmanus
netwerk/base/TCPFastOpen.h
netwerk/base/nsSocketTransport2.cpp
netwerk/base/nsSocketTransport2.h
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
--- a/netwerk/base/TCPFastOpen.h
+++ b/netwerk/base/TCPFastOpen.h
@@ -31,17 +31,17 @@ public:
   virtual bool FastOpenEnabled() = 0;
   // To use TFO we need to have a transaction prepared, e.g. also have
   // nsHttpConnection ready. This functions is call by nsSocketTransport to
   // setup a connection.
   virtual nsresult StartFastOpen() = 0;
   // Inform nsHalfopenSocket whether a connection using TFO succeeded or not.
   // This will cancel the backup connection and in case of a failure rewind
   // the transaction.
-  virtual void SetFastOpenConnected(nsresult error) = 0;
+  virtual void SetFastOpenConnected(nsresult error, bool aWillRetry) = 0;
   virtual void FastOpenNotSupported() = 0;
   virtual void SetFastOpenStatus(uint8_t tfoStatus) = 0;
 };
 
 }
 }
 
 #endif //TCPFASTOPEN_H__
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -789,16 +789,17 @@ nsSocketTransport::nsSocketTransport()
     , mOutput(this)
     , mQoSBits(0x00)
     , mKeepaliveEnabled(false)
     , mKeepaliveIdleTimeS(-1)
     , mKeepaliveRetryIntervalS(-1)
     , mKeepaliveProbeCount(-1)
     , mFastOpenCallback(nullptr)
     , mFastOpenLayerHasBufferedData(false)
+    , mDoNotRetryToConnect(false)
 {
     SOCKET_LOG(("creating nsSocketTransport @%p\n", this));
 
     mTimeouts[TIMEOUT_CONNECT]    = UINT16_MAX; // no timeout
     mTimeouts[TIMEOUT_READ_WRITE] = UINT16_MAX; // no timeout
 }
 
 nsSocketTransport::~nsSocketTransport()
@@ -1693,16 +1694,23 @@ nsSocketTransport::InitiateSocket()
 bool
 nsSocketTransport::RecoverFromError()
 {
     NS_ASSERTION(NS_FAILED(mCondition), "there should be something wrong");
 
     SOCKET_LOG(("nsSocketTransport::RecoverFromError [this=%p state=%x cond=%" PRIx32 "]\n",
                 this, mState, static_cast<uint32_t>(mCondition)));
 
+    if (mDoNotRetryToConnect) {
+        SOCKET_LOG(("nsSocketTransport::RecoverFromError do not retry because "
+                    "mDoNotRetryToConnect is set [this=%p]\n",
+                    this));
+        return false;
+    }
+
 #if defined(XP_UNIX)
     // Unix domain connections don't have multiple addresses to try,
     // so the recovery techniques here don't apply.
     if (mNetAddrIsSet && mNetAddr.raw.family == AF_LOCAL)
         return false;
 #endif
 
     // can only recover from errors in these states
@@ -1740,17 +1748,17 @@ nsSocketTransport::RecoverFromError()
          (mCondition == NS_ERROR_PROXY_CONNECTION_REFUSED))) {
         // TCP Fast Open can be blocked by middle boxes so we will retry
         // without it.
         tryAgain = true;
         // 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->SetFastOpenConnected(mCondition, true);
         }
         mFastOpenCallback = nullptr;
     } else {
 
         if ((mState == STATE_CONNECTING) && mDNSRecord &&
             mSocketTransportService->IsTelemetryEnabledAndNotSleepPhase()) {
             if (mNetAddr.raw.family == AF_INET) {
                 Telemetry::Accumulate(Telemetry::IPV4_AND_IPV6_ADDRESS_CONNECTIVITY,
@@ -1870,17 +1878,17 @@ nsSocketTransport::OnSocketConnected()
     // Set the m*AddrIsSet flags only when state has reached TRANSFERRING
     // because we need to make sure its value does not change due to failover
     mNetAddrIsSet = true;
 
     if (mFDFastOpenInProgress && mFastOpenCallback) {
         // mFastOpenCallback can be null when for example h2 is negotiated on
         // another connection to the same host and all connections are
         // abandoned.
-        mFastOpenCallback->SetFastOpenConnected(NS_OK);
+        mFastOpenCallback->SetFastOpenConnected(NS_OK, false);
     }
     mFastOpenCallback = nullptr;
 
     // assign mFD (must do this within the transport lock), but take care not
     // to trample over mFDref if mFD is already set.
     {
         MutexAutoLock lock(mLock);
         NS_ASSERTION(mFD.IsInitialized(), "no socket");
@@ -2297,17 +2305,17 @@ nsSocketTransport::OnSocketDetached(PRFi
 
         // The error can happened before we start fast open. In that case do not
         // call mFastOpenCallback->SetFastOpenConnected; If error happends during
         // fast open, inform the halfOpenSocket.
         // If we cancel the connection because backup socket was successfully
         // connected, mFDFastOpenInProgress will be true but mFastOpenCallback
         // will be nullptr.
         if (mFDFastOpenInProgress && mFastOpenCallback) {
-            mFastOpenCallback->SetFastOpenConnected(mCondition);
+            mFastOpenCallback->SetFastOpenConnected(mCondition, false);
         }
         mFastOpenCallback = nullptr;
 
         // make sure there isn't any pending DNS request
         if (mDNSRequest) {
             mDNSRequest->Cancel(NS_ERROR_ABORT);
             mDNSRequest = nullptr;
         }
@@ -2498,18 +2506,20 @@ nsSocketTransport::OpenOutputStream(uint
 }
 
 NS_IMETHODIMP
 nsSocketTransport::Close(nsresult reason)
 {
     if (NS_SUCCEEDED(reason))
         reason = NS_BASE_STREAM_CLOSED;
 
+    mDoNotRetryToConnect = true;
+
     if (mFDFastOpenInProgress && mFastOpenCallback) {
-        mFastOpenCallback->SetFastOpenConnected(reason);
+        mFastOpenCallback->SetFastOpenConnected(reason, false);
     }
     mFastOpenCallback = nullptr;
 
     mInput.CloseWithStatus(reason);
     mOutput.CloseWithStatus(reason);
     return NS_OK;
 }
 
--- a/netwerk/base/nsSocketTransport2.h
+++ b/netwerk/base/nsSocketTransport2.h
@@ -472,14 +472,16 @@ private:
     // Keepalive config (support varies by platform).
     int32_t mKeepaliveIdleTimeS;
     int32_t mKeepaliveRetryIntervalS;
     int32_t mKeepaliveProbeCount;
 
     // A Fast Open callback.
     TCPFastOpen *mFastOpenCallback;
     bool mFastOpenLayerHasBufferedData;
+
+    bool mDoNotRetryToConnect;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // !nsSocketTransport_h__
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -3562,23 +3562,24 @@ nsHalfOpenSocket::StartFastOpen()
         mStreamIn = nullptr;
         mSocketTransport = nullptr;
     }
     return rv;
 }
 
 void
 nsHttpConnectionMgr::
-nsHalfOpenSocket::SetFastOpenConnected(nsresult aError)
+nsHalfOpenSocket::SetFastOpenConnected(nsresult aError, bool aWillRetry)
 {
     RefPtr<nsHalfOpenSocket> deleteProtector(this);
 
     // Check if we want to restart connection!
-    if ((aError == NS_ERROR_CONNECTION_REFUSED) ||
-        (aError == NS_ERROR_NET_TIMEOUT)) {
+    if (aWillRetry &&
+        ((aError == NS_ERROR_CONNECTION_REFUSED) ||
+         (aError == NS_ERROR_NET_TIMEOUT))) {
         if (mEnt->mUseFastOpen) {
             gHttpHandler->IncrementFastOpenConsecutiveFailureCounter();
             mEnt->mUseFastOpen = false;
         }
         // This is called from nsSocketTransport::RecoverFromError. The
         // socket will try connect and we need to rewind nsHttpTransaction
         MOZ_ASSERT(mConnectionNegotiatingFastOpen);
         RefPtr<nsAHttpTransaction> trans = mConnectionNegotiatingFastOpen->CloseConnectionFastOpenTakesTooLongOrError(false);
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -371,17 +371,17 @@ private:
 
         void PrintDiagnostics(nsCString &log);
 
         bool Claim();
         void Unclaim();
 
         bool FastOpenEnabled() override;
         nsresult StartFastOpen() override;
-        void SetFastOpenConnected(nsresult) override;
+        void SetFastOpenConnected(nsresult, bool aWillRetry) override;
         void FastOpenNotSupported() override;
         void SetFastOpenStatus(uint8_t tfoStatus) override;
 
         bool IsFastOpenBackupHalfOpen()
         {
             return mConnectionNegotiatingFastOpen;
         }