Bug 1816258 - Websocket opening takes ages after repeated failures to same address r=necko-reviewers,valentin

Although our implementation follows the intention of rfc6455#section-4.1, we can handle the described situation more gracefully by prioritizing new WS connections to paths that have _not_ previously failed.

Differential Revision: https://phabricator.services.mozilla.com/D193386

Author: acreskeyMoz

netwerk/protocol/websocket/WebSocketChannel.cpp

@@ -75,8 +75,7 @@
 using namespace mozilla;
 using namespace mozilla::net;
 
-namespace mozilla {
-namespace net {
+namespace mozilla::net {
 
 NS_IMPL_ISUPPORTS(WebSocketChannel, nsIWebSocketChannel, nsIHttpUpgradeListener,
                   nsIRequestObserver, nsIStreamListener, nsIProtocolHandler,
@@ -121,11 +120,11 @@ const uint32_t kWSReconnectBaseLifeTime = 60 * 1000;
 const uint32_t kWSReconnectMaxDelay = 60 * 1000;
 
 // hold record of failed connections, and calculates needed delay for reconnects
-// to same host/port.
+// to same host/path/port.
 class FailDelay {
  public:
-  FailDelay(nsCString address, int32_t port)
-      : mAddress(std::move(address)), mPort(port) {
+  FailDelay(nsCString address, nsCString path, int32_t port)
+      : mAddress(std::move(address)), mPath(std::move(path)), mPort(port) {
     mLastFailure = TimeStamp::Now();
     mNextDelay = kWSReconnectInitialBaseDelay +
                  (rand() % kWSReconnectInitialRandomDelay);
@@ -139,9 +138,10 @@ class FailDelay {
     mNextDelay = static_cast<uint32_t>(
         std::min<double>(kWSReconnectMaxDelay, mNextDelay * 1.5));
     LOG(
-        ("WebSocket: FailedAgain: host=%s, port=%d: incremented delay to "
+        ("WebSocket: FailedAgain: host=%s, path=%s, port=%d: incremented delay "
+         "to "
          "%" PRIu32,
-         mAddress.get(), mPort, mNextDelay));
+         mAddress.get(), mPath.get(), mPort, mNextDelay));
   }
 
   // returns 0 if there is no need to delay (i.e. delay interval is over)
@@ -160,6 +160,7 @@ class FailDelay {
   }
 
   nsCString mAddress;  // IP address (or hostname if using proxy)
+  nsCString mPath;
   int32_t mPort;
 
  private:
@@ -191,16 +192,16 @@ class FailDelayManager {
 
   ~FailDelayManager() { MOZ_COUNT_DTOR(FailDelayManager); }
 
-  void Add(nsCString& address, int32_t port) {
+  void Add(nsCString& address, nsCString& path, int32_t port) {
     if (mDelaysDisabled) return;
 
-    UniquePtr<FailDelay> record(new FailDelay(address, port));
+    UniquePtr<FailDelay> record(new FailDelay(address, path, port));
     mEntries.AppendElement(std::move(record));
   }
 
   // Element returned may not be valid after next main thread event: don't keep
   // pointer to it around
-  FailDelay* Lookup(nsCString& address, int32_t port,
+  FailDelay* Lookup(nsCString& address, nsCString& path, int32_t port,
                     uint32_t* outIndex = nullptr) {
     if (mDelaysDisabled) return nullptr;
 
@@ -211,7 +212,8 @@ class FailDelayManager {
     // indexing simpler
     for (int32_t i = mEntries.Length() - 1; i >= 0; --i) {
       FailDelay* fail = mEntries[i].get();
-      if (fail->mAddress.Equals(address) && fail->mPort == port) {
+      if (fail->mAddress.Equals(address) && fail->mPath.Equals(path) &&
+          fail->mPort == port) {
         if (outIndex) *outIndex = i;
         result = fail;
         // break here: removing more entries would mess up *outIndex.
@@ -230,7 +232,7 @@ class FailDelayManager {
   void DelayOrBegin(WebSocketChannel* ws) {
     if (!mDelaysDisabled) {
       uint32_t failIndex = 0;
-      FailDelay* fail = Lookup(ws->mAddress, ws->mPort, &failIndex);
+      FailDelay* fail = Lookup(ws->mAddress, ws->mPath, ws->mPort, &failIndex);
 
       if (fail) {
         TimeStamp rightNow = TimeStamp::Now();
@@ -266,13 +268,14 @@ class FailDelayManager {
 
   // Remove() also deletes all expired entries as it iterates: better for
   // battery life than using a periodic timer.
-  void Remove(nsCString& address, int32_t port) {
+  void Remove(nsCString& address, nsCString& path, int32_t port) {
     TimeStamp rightNow = TimeStamp::Now();
 
     // iterate from end, to make deletion indexing easier
     for (int32_t i = mEntries.Length() - 1; i >= 0; --i) {
       FailDelay* entry = mEntries[i].get();
-      if ((entry->mAddress.Equals(address) && entry->mPort == port) ||
+      if ((entry->mAddress.Equals(address) && entry->mPath.Equals(path) &&
+           entry->mPort == port) ||
           entry->IsExpired(rightNow)) {
         mEntries.RemoveElementAt(i);
       }
@@ -321,14 +324,29 @@ class nsWSAdmissionManager {
 
     // If there is already another WS channel connecting to this IP address,
     // defer BeginOpen and mark as waiting in queue.
-    bool found = (sManager->IndexOf(ws->mAddress, ws->mOriginSuffix) >= 0);
+    bool hostFound = (sManager->IndexOf(ws->mAddress, ws->mOriginSuffix) >= 0);
+
+    uint32_t failIndex = 0;
+    FailDelay* fail = sManager->mFailures.Lookup(ws->mAddress, ws->mPath,
+                                                 ws->mPort, &failIndex);
+    bool existingFail = fail != nullptr;
 
     // Always add ourselves to queue, even if we'll connect immediately
     UniquePtr<nsOpenConn> newdata(
-        new nsOpenConn(ws->mAddress, ws->mOriginSuffix, ws));
-    sManager->mQueue.AppendElement(std::move(newdata));
+        new nsOpenConn(ws->mAddress, ws->mOriginSuffix, existingFail, ws));
 
-    if (found) {
+    // If a connection has not previously failed then prioritize it over
+    // connections that have
+    if (existingFail) {
+      sManager->mQueue.AppendElement(std::move(newdata));
+    } else {
+      uint32_t insertionIndex = sManager->IndexOfFirstFailure();
+      MOZ_ASSERT(insertionIndex <= sManager->mQueue.Length(),
+                 "Insertion index outside bounds");
+      sManager->mQueue.InsertElementAt(insertionIndex, std::move(newdata));
+    }
+
+    if (hostFound) {
       LOG(
           ("Websocket: some other channel is connecting, changing state to "
            "CONNECTING_QUEUED"));
@@ -357,7 +375,8 @@ class nsWSAdmissionManager {
     sManager->RemoveFromQueue(aChannel);
 
     // Connection succeeded, so stop keeping track of any previous failures
-    sManager->mFailures.Remove(aChannel->mAddress, aChannel->mPort);
+    sManager->mFailures.Remove(aChannel->mAddress, aChannel->mPath,
+                               aChannel->mPort);
 
     // Check for queued connections to same host.
     // Note: still need to check for failures, since next websocket with same
@@ -378,24 +397,27 @@ class nsWSAdmissionManager {
 
     if (NS_FAILED(aReason)) {
       // Have we seen this failure before?
-      FailDelay* knownFailure =
-          sManager->mFailures.Lookup(aChannel->mAddress, aChannel->mPort);
+      FailDelay* knownFailure = sManager->mFailures.Lookup(
+          aChannel->mAddress, aChannel->mPath, aChannel->mPort);
       if (knownFailure) {
         if (aReason == NS_ERROR_NOT_CONNECTED) {
           // Don't count close() before connection as a network error
           LOG(
-              ("Websocket close() before connection to %s, %d completed"
+              ("Websocket close() before connection to %s, %s,  %d completed"
                " [this=%p]",
-               aChannel->mAddress.get(), (int)aChannel->mPort, aChannel));
+               aChannel->mAddress.get(), aChannel->mPath.get(),
+               (int)aChannel->mPort, aChannel));
         } else {
           // repeated failure to connect: increase delay for next connection
           knownFailure->FailedAgain();
         }
       } else {
         // new connection failure: record it.
-        LOG(("WebSocket: connection to %s, %d failed: [this=%p]",
-             aChannel->mAddress.get(), (int)aChannel->mPort, aChannel));
-        sManager->mFailures.Add(aChannel->mAddress, aChannel->mPort);
+        LOG(("WebSocket: connection to %s, %s, %d failed: [this=%p]",
+             aChannel->mAddress.get(), aChannel->mPath.get(),
+             (int)aChannel->mPort, aChannel));
+        sManager->mFailures.Add(aChannel->mAddress, aChannel->mPath,
+                                aChannel->mPort);
       }
     }
 
@@ -480,15 +502,19 @@ class nsWSAdmissionManager {
 
   class nsOpenConn {
    public:
-    nsOpenConn(nsCString& addr, nsCString& originSuffix,
+    nsOpenConn(nsCString& addr, nsCString& originSuffix, bool failed,
                WebSocketChannel* channel)
-        : mAddress(addr), mOriginSuffix(originSuffix), mChannel(channel) {
+        : mAddress(addr),
+          mOriginSuffix(originSuffix),
+          mFailed(failed),
+          mChannel(channel) {
       MOZ_COUNT_CTOR(nsOpenConn);
     }
     MOZ_COUNTED_DTOR(nsOpenConn)
 
     nsCString mAddress;
     nsCString mOriginSuffix;
+    bool mFailed = false;
     RefPtr<WebSocketChannel> mChannel;
   };
 
@@ -535,6 +561,15 @@ class nsWSAdmissionManager {
     return -1;
   }
 
+  // Returns the index of the first entry that failed, or else the last entry if
+  // none found
+  uint32_t IndexOfFirstFailure() {
+    for (uint32_t i = 0; i < mQueue.Length(); i++) {
+      if (mQueue[i]->mFailed) return i;
+    }
+    return mQueue.Length();
+  }
+
   // SessionCount might be decremented from the main or the socket
   // thread, so manage it with atomic counters
   Atomic<int32_t> mSessionCount;
@@ -2853,6 +2888,10 @@ nsresult WebSocketChannel::DoAdmissionDNS() {
   rv = mURI->GetHost(hostName);
   NS_ENSURE_SUCCESS(rv, rv);
   mAddress = hostName;
+  nsCString path;
+  rv = mURI->GetFilePath(path);
+  NS_ENSURE_SUCCESS(rv, rv);
+  mPath = path;
   rv = mURI->GetPort(&mPort);
   NS_ENSURE_SUCCESS(rv, rv);
   if (mPort == -1) mPort = (mEncrypted ? kDefaultWSSPort : kDefaultWSPort);
@@ -4292,7 +4331,6 @@ nsresult WebSocketChannel::OnDataReceived(uint8_t* aData, uint32_t aCount) {
   return ProcessInput(aData, aCount);
 }
 
-}  // namespace net
-}  // namespace mozilla
+}  // namespace mozilla::net
 
 #undef CLOSE_GOING_AWAY

netwerk/protocol/websocket/WebSocketChannel.h

@@ -229,6 +229,7 @@ class WebSocketChannel : public BaseWebSocketChannel,
   // then to IP address (unless we're leaving DNS resolution to a proxy server)
   // MainThread only
   nsCString mAddress;
+  nsCString mPath;
   int32_t mPort;  // WS server port
   // Secondary key for the connection queue. Used by nsWSAdmissionManager.
   nsCString mOriginSuffix;  // MainThread only