[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.
This commit is contained in:
Thiago Marcos P. Santos
2016-02-16 15:51:40 +02:00
parent d66d145c6e
commit f5b521ffb5

View File

@@ -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() };