Bug 1430659 - Network requests in "blocked" state for long time (1+ minute) when ipv6 hosts are no longer available. r=valentin

2018-03-16 09:06:00 -04:00 · 2018-03-16 09:06:00 -04:00 · c940bbdf06
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@ -1794,6 +1794,13 @@ pref("network.http.connection-timeout", 90);
 // seconds.
 pref("network.http.tls-handshake-timeout", 30);

+// The number of seconds after which we time out a connection of a retry (fallback)
+// socket when a certain IP family is already preferred.  This shorter connection
+// timeout allows us to find out more quickly that e.g. an IPv6 host is no longer
+// available and let us try an IPv4 address, if provided for the host name.
+// Set to '0' to use the default connection timeout.
+pref("network.http.fallback-connection-timeout", 5);
+
 // The number of seconds to allow active connections to prove that they have
 // traffic before considered stalled, after a network change has been detected
 // and signalled.
--- a/netwerk/base/nsISocketTransport.idl
+++ b/netwerk/base/nsISocketTransport.idl
@ -253,6 +253,12 @@ interface nsISocketTransport : nsITransport
     */
    const unsigned long REFRESH_CACHE = (1 << 9);

+    /**
+     * If this flag is set then it means that if connecting the preferred ip
+     * family has failed, retry with the oppsite one once more.
+     */
+    const unsigned long RETRY_WITH_DIFFERENT_IP_FAMILY = (1 << 10);
+
    /**
     * An opaque flags for non-standard behavior of the TLS system.
     * It is unlikely this will need to be set outside of telemetry studies
@ -289,4 +295,11 @@ interface nsISocketTransport : nsITransport
    [noscript] void setFastOpenCallback(in TCPFastOpenPtr aFastOpen);

    readonly attribute nsresult firstRetryError;
+
+    /**
+     * If true, this socket transport has found out the prefered family
+     * according it's connection flags could not be used to establish
+     * connections any more.  Hence, the preference should be reset.
+     */
+    readonly attribute boolean resetIPFamilyPreference;
 };
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@ -775,6 +775,7 @@ nsSocketTransport::nsSocketTransport()
    , mProxyTransparentResolvesHost(false)
    , mHttpsProxy(false)
    , mConnectionFlags(0)
+    , mResetFamilyPreference(false)
    , mTlsFlags(0)
    , mReuseAddrPort(false)
    , mState(STATE_CLOSED)
@ -1703,8 +1704,10 @@ nsSocketTransport::RecoverFromError()
 #endif

    // can only recover from errors in these states
-    if (mState != STATE_RESOLVING && mState != STATE_CONNECTING)
+    if (mState != STATE_RESOLVING && mState != STATE_CONNECTING) {
+        SOCKET_LOG(("  not in a recoverable state"));
        return false;
+    }

    nsresult rv;

@ -1729,15 +1732,19 @@ nsSocketTransport::RecoverFromError()
        mCondition != NS_ERROR_NET_TIMEOUT &&
        mCondition != NS_ERROR_UNKNOWN_HOST &&
        mCondition != NS_ERROR_UNKNOWN_PROXY_HOST &&
-        !(mFDFastOpenInProgress && (mCondition == NS_ERROR_FAILURE)))
+        !(mFDFastOpenInProgress && (mCondition == NS_ERROR_FAILURE))) {
+        SOCKET_LOG(("  not a recoverable error %" PRIx32, static_cast<uint32_t>(mCondition)));
        return false;
+    }
 #else
    if (mCondition != NS_ERROR_CONNECTION_REFUSED &&
        mCondition != NS_ERROR_PROXY_CONNECTION_REFUSED &&
        mCondition != NS_ERROR_NET_TIMEOUT &&
        mCondition != NS_ERROR_UNKNOWN_HOST &&
-        mCondition != NS_ERROR_UNKNOWN_PROXY_HOST)
+        mCondition != NS_ERROR_UNKNOWN_PROXY_HOST) {
+        SOCKET_LOG(("  not a recoverable error %" PRIx32, static_cast<uint32_t>(mCondition)));
        return false;
+    }
 #endif

    bool tryAgain = false;
@ -1779,12 +1786,15 @@ nsSocketTransport::RecoverFromError()
            }
        }

-        if (mConnectionFlags & (DISABLE_IPV6 | DISABLE_IPV4) &&
+        if (mConnectionFlags & RETRY_WITH_DIFFERENT_IP_FAMILY &&
            mCondition == NS_ERROR_UNKNOWN_HOST &&
            mState == STATE_RESOLVING &&
            !mProxyTransparentResolvesHost) {
-            SOCKET_LOG(("  trying lookup again with both ipv4/ipv6 enabled\n"));
-            mConnectionFlags &= ~(DISABLE_IPV6 | DISABLE_IPV4);
+            SOCKET_LOG(("  trying lookup again with opposite ip family\n"));
+            mConnectionFlags ^= (DISABLE_IPV6 | DISABLE_IPV4);
+            mConnectionFlags &= ~RETRY_WITH_DIFFERENT_IP_FAMILY;
+            // This will tell the consuming half-open to reset preference on the connection entry
+            mResetFamilyPreference = true;
            tryAgain = true;
        }

@ -1794,17 +1804,15 @@ nsSocketTransport::RecoverFromError()
            if (NS_SUCCEEDED(rv)) {
                SOCKET_LOG(("  trying again with next ip address\n"));
                tryAgain = true;
-            }
-            else if (mConnectionFlags & (DISABLE_IPV6 | DISABLE_IPV4)) {
+            } else if (mConnectionFlags & RETRY_WITH_DIFFERENT_IP_FAMILY) {
+                SOCKET_LOG(("  failed to connect, trying with opposite ip family\n"));
                // Drop state to closed.  This will trigger new round of DNS
                // resolving bellow.
-                // XXX Could be optimized to only switch the flags to save
-                // duplicate connection attempts.
-                SOCKET_LOG(("  failed to connect all ipv4-only or ipv6-only "
-                            "hosts, trying lookup/connect again with both "
-                            "ipv4/ipv6\n"));
                mState = STATE_CLOSED;
-                mConnectionFlags &= ~(DISABLE_IPV6 | DISABLE_IPV4);
+                mConnectionFlags ^= (DISABLE_IPV6 | DISABLE_IPV4);
+                mConnectionFlags &= ~RETRY_WITH_DIFFERENT_IP_FAMILY;
+                // This will tell the consuming half-open to reset preference on the connection entry
+                mResetFamilyPreference = true;
                tryAgain = true;
            } else if (!(mConnectionFlags & DISABLE_TRR)) {
                bool trrEnabled;
@ -2327,6 +2335,8 @@ nsSocketTransport::OnSocketDetached(PRFileDesc *fd)

    MOZ_ASSERT(OnSocketThread(), "not on socket thread");

+    mAttached = false;
+
    // if we didn't initiate this detach, then be sure to pass an error
    // condition up to our consumers.  (e.g., STS is shutting down.)
    if (NS_SUCCEEDED(mCondition)) {
@ -2551,6 +2561,9 @@ nsSocketTransport::OpenOutputStream(uint32_t flags,
 NS_IMETHODIMP
 nsSocketTransport::Close(nsresult reason)
 {
+    SOCKET_LOG(("nsSocketTransport::Close %p reason=%" PRIx32,
+                this, static_cast<uint32_t>(reason)));
+
    if (NS_SUCCEEDED(reason))
        reason = NS_BASE_STREAM_CLOSED;

@ -3014,8 +3027,11 @@ nsSocketTransport::GetConnectionFlags(uint32_t *value)
 NS_IMETHODIMP
 nsSocketTransport::SetConnectionFlags(uint32_t value)
 {
+    SOCKET_LOG(("nsSocketTransport::SetConnectionFlags %p flags=%u", this, value));
+
    mConnectionFlags = value;
    mIsPrivate = value & nsISocketTransport::NO_PERMANENT_STORAGE;
+
    return NS_OK;
 }

@ -3558,5 +3574,12 @@ nsSocketTransport::GetFirstRetryError(nsresult *aFirstRetryError)
  return NS_OK;
 }

+NS_IMETHODIMP
+nsSocketTransport::GetResetIPFamilyPreference(bool *aReset)
+{
+  *aReset = mResetFamilyPreference;
+  return NS_OK;
+}
+
 } // namespace net
 } // namespace mozilla
--- a/netwerk/base/nsSocketTransport2.h
+++ b/netwerk/base/nsSocketTransport2.h
@ -301,6 +301,9 @@ private:
    bool mProxyTransparentResolvesHost;
    bool mHttpsProxy;
    uint32_t     mConnectionFlags;
+    // When we fail to connect using a prefered IP family, we tell the consumer to reset
+    // the IP family preference on the connection entry.
+    bool         mResetFamilyPreference;
    uint32_t     mTlsFlags;
    bool mReuseAddrPort;

--- a/netwerk/protocol/http/TunnelUtils.cpp
+++ b/netwerk/protocol/http/TunnelUtils.cpp
@ -1562,6 +1562,7 @@ FWD_TS_PTR(GetTlsFlags, uint32_t);
 FWD_TS(SetTlsFlags, uint32_t);
 FWD_TS_PTR(GetRecvBufferSize, uint32_t);
 FWD_TS(SetRecvBufferSize, uint32_t);
+FWD_TS_PTR(GetResetIPFamilyPreference, bool);

 nsresult
 SocketTransportShim::GetOriginAttributes(mozilla::OriginAttributes* aOriginAttributes)
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@ -3989,7 +3989,6 @@ nsHttpConnectionMgr::nsHalfOpenSocket::~nsHalfOpenSocket()
 {
    MOZ_ASSERT(!mStreamOut);
    MOZ_ASSERT(!mBackupStreamOut);
-    MOZ_ASSERT(!mSynTimer);
    LOG(("Destroying nsHalfOpenSocket [this=%p]\n", this));

    if (mEnt)
@ -4071,16 +4070,29 @@ nsHalfOpenSocket::SetupStreams(nsISocketTransport **transport,
        tmpFlags |= nsISocketTransport::BE_CONSERVATIVE;
    }

-    // For backup connections, we disable IPv6. That's because some users have
-    // broken IPv6 connectivity (leading to very long timeouts), and disabling
-    // IPv6 on the backup connection gives them a much better user experience
-    // with dual-stack hosts, though they still pay the 250ms delay for each new
-    // connection. This strategy is also known as "happy eyeballs".
-    if (mEnt->mPreferIPv6) {
-        tmpFlags |= nsISocketTransport::DISABLE_IPV4;
-    }
-    else if (mEnt->mPreferIPv4 ||
-             (isBackup && gHttpHandler->FastFallbackToIPv4())) {
+    if (mEnt->PreferenceKnown()) {
+        if (mEnt->mPreferIPv6) {
+            tmpFlags |= nsISocketTransport::DISABLE_IPV4;
+        } else if (mEnt->mPreferIPv4) {
+            tmpFlags |= nsISocketTransport::DISABLE_IPV6;
+        }
+
+        // In case the host is no longer accessible via the preferred IP family,
+        // try the opposite one and potentially restate the preference.
+        tmpFlags |= nsISocketTransport::RETRY_WITH_DIFFERENT_IP_FAMILY;
+
+        // From the same reason, let the backup socket fail faster to try the other family.
+        uint16_t fallbackTimeout = isBackup ? gHttpHandler->GetFallbackSynTimeout() : 0;
+        if (fallbackTimeout) {
+            socketTransport->SetTimeout(nsISocketTransport::TIMEOUT_CONNECT,
+                                        fallbackTimeout);
+        }
+    } else if (isBackup) {
+        // For backup connections, we disable IPv6. That's because some users have
+        // broken IPv6 connectivity (leading to very long timeouts), and disabling
+        // IPv6 on the backup connection gives them a much better user experience
+        // with dual-stack hosts, though they still pay the 250ms delay for each new
+        // connection. This strategy is also known as "happy eyeballs".
        tmpFlags |= nsISocketTransport::DISABLE_IPV6;
    }

@ -4155,6 +4167,7 @@ nsHttpConnectionMgr::nsHalfOpenSocket::SetupPrimaryStreams()
                      getter_AddRefs(mStreamIn),
                      getter_AddRefs(mStreamOut),
                      false);
+
    LOG(("nsHalfOpenSocket::SetupPrimaryStream [this=%p ent=%s rv=%" PRIx32 "]",
         this, mEnt->mConnInfo->Origin(), static_cast<uint32_t>(rv)));
    if (NS_FAILED(rv)) {
@ -4180,6 +4193,7 @@ nsHttpConnectionMgr::nsHalfOpenSocket::SetupBackupStreams()
                               getter_AddRefs(mBackupStreamIn),
                               getter_AddRefs(mBackupStreamOut),
                               true);
+
    LOG(("nsHalfOpenSocket::SetupBackupStream [this=%p ent=%s rv=%" PRIx32 "]",
         this, mEnt->mConnInfo->Origin(), static_cast<uint32_t>(rv)));
    if (NS_FAILED(rv)) {
@ -4230,7 +4244,9 @@ nsHttpConnectionMgr::nsHalfOpenSocket::CancelBackupTimer()

    LOG(("nsHalfOpenSocket::CancelBackupTimer()"));
    mSynTimer->Cancel();
-    mSynTimer = nullptr;
+
+    // Keeping the reference to the timer to remember we have already
+    // performed the backup connection.
 }

 void
@ -4318,7 +4334,9 @@ nsHttpConnectionMgr::nsHalfOpenSocket::Notify(nsITimer *timer)
    DebugOnly<nsresult> rv = SetupBackupStreams();
    MOZ_ASSERT(NS_SUCCEEDED(rv));

-    mSynTimer = nullptr;
+    // Keeping the reference to the timer to remember we have already
+    // performed the backup connection.
+
    return NS_OK;
 }

@ -4819,6 +4837,12 @@ nsHalfOpenSocket::SetupConn(nsIAsyncOutputStream *out,
                        PR_MillisecondsToInterval(
                          static_cast<uint32_t>(rtt.ToMilliseconds())));

+        bool resetPreference = false;
+        mSocketTransport->GetResetIPFamilyPreference(&resetPreference);
+        if (resetPreference) {
+            mEnt->ResetIPFamilyPreference();
+        }
+
        if (!aFastOpen &&
            NS_SUCCEEDED(mSocketTransport->GetPeerAddr(&peeraddr))) {
            mEnt->RecordIPFamilyPreference(peeraddr.raw.family);
@ -4841,8 +4865,15 @@ nsHalfOpenSocket::SetupConn(nsIAsyncOutputStream *out,
                        PR_MillisecondsToInterval(
                          static_cast<uint32_t>(rtt.ToMilliseconds())));

-        if (NS_SUCCEEDED(mBackupTransport->GetPeerAddr(&peeraddr)))
+        bool resetPreference = false;
+        mBackupTransport->GetResetIPFamilyPreference(&resetPreference);
+        if (resetPreference) {
+            mEnt->ResetIPFamilyPreference();
+        }
+
+        if (NS_SUCCEEDED(mBackupTransport->GetPeerAddr(&peeraddr))) {
            mEnt->RecordIPFamilyPreference(peeraddr.raw.family);
+        }

        // The nsHttpConnection object now owns these streams and sockets
        mBackupStreamOut = nullptr;
@ -5365,21 +5396,34 @@ void
 nsHttpConnectionMgr::
 nsConnectionEntry::RecordIPFamilyPreference(uint16_t family)
 {
-  if (family == PR_AF_INET && !mPreferIPv6)
-    mPreferIPv4 = true;
+  LOG(("nsConnectionEntry::RecordIPFamilyPreference %p, af=%u", this, family));

-  if (family == PR_AF_INET6 && !mPreferIPv4)
+  if (family == PR_AF_INET && !mPreferIPv6) {
+    mPreferIPv4 = true;
+  }
+
+  if (family == PR_AF_INET6 && !mPreferIPv4) {
    mPreferIPv6 = true;
+  }
+
+  LOG(("  %p prefer ipv4=%d, ipv6=%d", this, (bool)mPreferIPv4, (bool)mPreferIPv6));
 }

 void
 nsHttpConnectionMgr::
 nsConnectionEntry::ResetIPFamilyPreference()
 {
+  LOG(("nsConnectionEntry::ResetIPFamilyPreference %p", this));
+
  mPreferIPv4 = false;
  mPreferIPv6 = false;
 }

+bool net::nsHttpConnectionMgr::nsConnectionEntry::PreferenceKnown() const
+{
+  return (bool)mPreferIPv4 || (bool)mPreferIPv6;
+}
+
 size_t
 nsHttpConnectionMgr::nsConnectionEntry::PendingQLength() const
 {
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@ -331,6 +331,8 @@ private:
        void RecordIPFamilyPreference(uint16_t family);
        // Resets all flags to their default values
        void ResetIPFamilyPreference();
+        // True iff there is currently an established IP family preference
+        bool PreferenceKnown() const;

        // Return the count of pending transactions for all window ids.
        size_t PendingQLength() const;
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@ -207,6 +207,7 @@ nsHttpHandler::nsHttpHandler()
    , mMaxRequestAttempts(6)
    , mMaxRequestDelay(10)
    , mIdleSynTimeout(250)
+    , mFallbackSynTimeout(5)
    , mH2MandatorySuiteEnabled(false)
    , mMaxUrgentExcessiveConns(3)
    , mMaxConnections(24)
@ -1377,6 +1378,12 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
            mFastFallbackToIPv4 = cVar;
    }

+    if (PREF_CHANGED(HTTP_PREF("fallback-connection-timeout"))) {
+        rv = prefs->GetIntPref(HTTP_PREF("fallback-connection-timeout"), &val);
+        if (NS_SUCCEEDED(rv))
+            mFallbackSynTimeout = (uint16_t) clamped(val, 0, 10 * 60);
+    }
+
    if (PREF_CHANGED(HTTP_PREF("version"))) {
        nsAutoCString httpVersion;
        prefs->GetCharPref(HTTP_PREF("version"), httpVersion);
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@ -101,6 +101,7 @@ public:
    uint32_t       PhishyUserPassLength()    { return mPhishyUserPassLength; }
    uint8_t        GetQoSBits()              { return mQoSBits; }
    uint16_t       GetIdleSynTimeout()       { return mIdleSynTimeout; }
+    uint16_t       GetFallbackSynTimeout()   { return mFallbackSynTimeout; }
    bool           FastFallbackToIPv4()      { return mFastFallbackToIPv4; }
    uint32_t       MaxSocketCount();
    bool           EnforceAssocReq()         { return mEnforceAssocReq; }
@ -484,6 +485,7 @@ private:
    uint16_t mMaxRequestAttempts;
    uint16_t mMaxRequestDelay;
    uint16_t mIdleSynTimeout;
+    uint16_t mFallbackSynTimeout; // seconds

    bool     mH2MandatorySuiteEnabled;
    uint16_t mMaxUrgentExcessiveConns;