[core] Remove requests from the pending queue on cancel

If we don't do this, we can activate a request twice.

The issue here is that we use memory address as the 'key'. So if
we destroy a request, another request might take the same memory
address, or the same 'key'.

The entry at 'pendingRequests' survives the 'cancel()' and is
ultimatelly removed at 'activatePendingRequest()'. This is fine
unless a request is created using the same 'key' before we call
'activatePendingRequest()'.

The check if the request was canceled at 'activatePendingRequest()'
will be no longer valid, activating the request. At the same time
the request will be activated again by the task scheduled at the
constructor of the 'OnlineFileRequestImpl' class.

1 files changed, 21 insertions, 13 deletions

diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp
index a095b86fba..43c707775d 100644
--- a/platform/default/online_file_source.cpp
+++ b/platform/default/online_file_source.cpp
@@ -15,6 +15,7 @@
 
 #include <algorithm>
 #include <cassert>
+#include <list>
 #include <set>
 #include <unordered_map>
 
@@ -62,6 +63,12 @@ public:
         allRequests.erase(key);
         if (activeRequests.erase(key)) {
             activatePendingRequest();
+        } else {
+            auto it = pendingRequestsMap.find(key);
+            if (it != pendingRequestsMap.end()) {
+                pendingRequestsList.erase(it->second);
+                pendingRequestsMap.erase(it);
+            }
         }
     }
 
@@ -78,7 +85,8 @@ public:
     }
 
     void queueRequest(OnlineFileRequestImpl* impl) {
-        pendingRequests.push(impl->key);
+        auto it = pendingRequestsList.insert(pendingRequestsList.end(), impl->key);
+        pendingRequestsMap.emplace(impl->key, std::move(it));
     }
 
     void activateRequest(OnlineFileRequestImpl* impl) {
@@ -92,19 +100,18 @@ public:
     }
 
     void activatePendingRequest() {
-        while (!pendingRequests.empty()) {
-            FileRequest* key = pendingRequests.front();
-            pendingRequests.pop();
-
-            auto it = allRequests.find(key);
-            if (it == allRequests.end()) {
-                // It must have been cancelled.
-                continue;
-            }
-
-            activateRequest(it->second.get());
+        if (pendingRequestsList.empty()) {
             return;
         }
+
+        FileRequest* key = pendingRequestsList.front();
+        pendingRequestsList.pop_front();
+
+        pendingRequestsMap.erase(key);
+
+        auto it = allRequests.find(key);
+        assert(it != allRequests.end());
+        activateRequest(it->second.get());
     }
 
 private:
@@ -126,7 +133,8 @@ private:
      * `pendingRequests`. Requests in the active state are in `activeRequests`.
      */
     std::unordered_map<FileRequest*, std::unique_ptr<OnlineFileRequestImpl>> allRequests;
-    std::queue<FileRequest*> pendingRequests;
+    std::list<FileRequest*> pendingRequestsList;
+    std::unordered_map<FileRequest*, std::list<FileRequest*>::iterator> pendingRequestsMap;
     std::set<FileRequest*> activeRequests;
 
     const std::unique_ptr<HTTPContextBase> httpContext { HTTPContextBase::createContext() };