diff --git a/src/net/p2p/managerbase.cpp b/src/net/p2p/managerbase.cpp index f65cc876..303499b5 100644 --- a/src/net/p2p/managerbase.cpp +++ b/src/net/p2p/managerbase.cpp @@ -264,14 +264,16 @@ namespace P2P { LOGDEBUG("Peer " + toString(nodeId) + " is a discovery node, cannot send request"); return nullptr; } + // Track the request + auto request = std::make_shared(message->command(), message->id(), session->hostNodeId(), message); + insertRequest(request); + // If message was never sent, then no request has been made. Fail and return. if (!session->write(message)) { - // If message was never sent, then no request has been made. Fail and return. + // We know the latest insert went into the active bucket, so we can just remove it. + std::unique_lock lock(this->requestsMutex_); + requests_[activeRequests_].erase(message->id()); return nullptr; } - // Message was sent, so create a pending request entry and insert it in the active bucket. - // If the active bucket is full, clear the old bucket and make it the new active bucket. - auto request = std::make_shared(message->command(), message->id(), session->hostNodeId(), message); - insertRequest(request); return request; } @@ -731,16 +733,17 @@ namespace P2P { std::make_shared(std::move(socket), ConnectionType::INBOUND, *this)->run(); } - void ManagerBase::insertRequest(const std::shared_ptr& request) { + void ManagerBase::insertRequest(std::shared_ptr request) { // The previous logic would silently overwrite the previous entry in case of an (improbable) // request ID collision; this logic is retained here, since lookups will be directed to the // active bucket first (so a colliding entry that is in the older bucket is shadowed anyway). std::unique_lock lock(this->requestsMutex_); - requests_[activeRequests_][request->id()] = request; if (requests_[activeRequests_].size() >= REQUEST_BUCKET_SIZE_LIMIT) { activeRequests_ = 1 - activeRequests_; requests_[activeRequests_].clear(); } + // Make sure the latest insertion always ended up in what is the current active bucket + requests_[activeRequests_][request->id()] = std::move(request); } void ManagerBase::handleRequestAnswer(const NodeID& nodeId, const std::shared_ptr& message) { diff --git a/src/net/p2p/managerbase.h b/src/net/p2p/managerbase.h index b1d12d38..b1f08cdb 100644 --- a/src/net/p2p/managerbase.h +++ b/src/net/p2p/managerbase.h @@ -11,7 +11,7 @@ See the LICENSE.txt file in the project root for more information. /// Size of a sync-requests active bucket, in items (requests); there are two buckets. /// This needs to be large enough to hold the maximum number of requests that can be conceivable generated /// in the maximum time period that it takes to answer a request. -#define REQUEST_BUCKET_SIZE_LIMIT 100000 +#define REQUEST_BUCKET_SIZE_LIMIT 100'000 #include "discovery.h" #include "session.h" // encoding.h -> utils/options.h @@ -147,7 +147,7 @@ namespace P2P { * Insert a new request into requests_. * @param request The newly created request to be tracked by requests_. */ - void insertRequest(const std::shared_ptr& request); + void insertRequest(std::shared_ptr request); /** * Handles a message that is a generic answer to a synchronous generic remote request.