From 19c47ec7816e96247d2795ec9882f4c4d627c45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20D=C3=A9saulniers?= Date: Thu, 19 Jan 2017 19:45:30 -0500 Subject: [PATCH 1/2] dht: correctly tell if value is announced * A value should be considered announced if all good nodes up to a count of TARGET_NODES have been tried in the search . * Bug fix where SearchNode::isAnnounced criteria was not consistent with announce operation. --- src/dht.cpp | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/dht.cpp b/src/dht.cpp index 90954688..2e39243e 100644 --- a/src/dht.cpp +++ b/src/dht.cpp @@ -289,7 +289,7 @@ struct Dht::SearchNode { /** * Foreach value id, we keep track of a pair (net::Request, time_point) where the * request is the request returned by the network engine and the time_point - * is the last time at which the value has been refreshed. + * is the next time at which the value must be refreshed. */ using AnnounceStatus = std::map, time_point>>; /** @@ -440,11 +440,11 @@ struct Dht::SearchNode { }) != status.end(); } - bool isAnnounced(Value::Id vid, time_point now) const { + bool isAnnounced(Value::Id vid) const { auto ack = acked.find(vid); if (ack == acked.end() or not ack->second.first) return false; - return ack->second.second > now; + return ack->second.first->completed(); } bool isListening(time_point now) const { @@ -618,7 +618,7 @@ struct Dht::Search { time_point getUpdateTime(time_point now) const; - bool isAnnounced(Value::Id id, time_point now) const; + bool isAnnounced(Value::Id id) const; bool isListening(time_point now) const; /** @@ -733,7 +733,7 @@ struct Dht::Search { [this,&vid,&now](Announce& a) { if (vid != Value::INVALID_ID and (!a.value || a.value->id != vid)) return false; - if (isAnnounced(a.value->id, now)) { + if (isAnnounced(a.value->id)) { if (a.callback) { a.callback(true, getNodes()); a.callback = nullptr; @@ -1427,7 +1427,7 @@ Dht::searchStep(std::shared_ptr sr) } /* clearing callbacks for announced values */ - sr->checkAnnounced(now); + sr->checkAnnounced(); if (sr->callbacks.empty() && sr->announce.empty() && sr->listeners.empty()) sr->setDone(); @@ -1591,7 +1591,7 @@ Dht::Search::getUpdateTime(time_point now) const } bool -Dht::Search::isAnnounced(Value::Id id, time_point now) const +Dht::Search::isAnnounced(Value::Id id) const { if (nodes.empty()) return false; @@ -1599,10 +1599,10 @@ Dht::Search::isAnnounced(Value::Id id, time_point now) const for (const auto& n : nodes) { if (n.isBad()) continue; - if (not n.isAnnounced(id, now)) + if (not n.isAnnounced(id)) return false; if (++i == TARGET_NODES) - break; + return true; } return i; } @@ -1837,7 +1837,7 @@ Dht::announce(const InfoHash& id, n.probe_query.reset(); } } - if (sr->isAnnounced(value->id, now)) { + if (sr->isAnnounced(value->id)) { if (a_sr->callback) a_sr->callback(true, {}); a_sr->callback = {}; @@ -2674,9 +2674,9 @@ Dht::dumpSearch(const Search& sr, std::ostream& out) const out << *l.second.query << std::endl; } - for (const auto& n : sr.announce) { - bool announced = sr.isAnnounced(n.value->id, now); - out << "Announcement: " << *n.value << (announced ? " [announced]" : "") << std::endl; + for (const auto& a : sr.announce) { + bool announced = sr.isAnnounced(a.value->id); + out << "Announcement: " << *a.value << (announced ? " [announced]" : "") << std::endl; } out << " Common bits InfoHash Conn. Get Ops IP" << std::endl; @@ -3566,14 +3566,13 @@ Dht::onRefresh(std::shared_ptr node, const InfoHash& hash, const Blob& tok void Dht::onAnnounceDone(const std::shared_ptr& node, net::NetworkEngine::RequestAnswer& answer, std::shared_ptr& sr) { - const auto& now = scheduler.time(); DHT_LOG.d(sr->id, node->id, "[search %s] [node %s] got reply to put!", sr->id.toString().c_str(), node->toString().c_str()); searchSendGetValues(sr); /* if (auto sn = sr->getNode(req->node)) { */ /* sn->setRefreshTime(answer.vid, now + answer) */ /* } */ - sr->checkAnnounced(now, answer.vid); + sr->checkAnnounced(answer.vid); } } From efa6930de2b8f2ab7b86dbda7e9b58b9d8326bf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20D=C3=A9saulniers?= Date: Thu, 19 Jan 2017 19:48:49 -0500 Subject: [PATCH 2/2] dht: fix multiple put on a single search segfault "Iterators pointing to an element between the new logical end and the physical end of the range are still dereferenceable, but the elements themselves have unspecified values (as per MoveAssignable post-condition)." - http://en.cppreference.com/w/cpp/algorithm/remove --- src/dht.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dht.cpp b/src/dht.cpp index 2e39243e..a16c6b1a 100644 --- a/src/dht.cpp +++ b/src/dht.cpp @@ -728,20 +728,20 @@ struct Dht::Search { * @param types The sequence of existing types. * @param now The time reference to now. */ - void checkAnnounced(time_point now, Value::Id vid = Value::INVALID_ID) { - auto announced = std::remove_if(announce.begin(), announce.end(), - [this,&vid,&now](Announce& a) { + void checkAnnounced(Value::Id vid = Value::INVALID_ID) { + auto announced = std::partition(announce.begin(), announce.end(), + [this,&vid](Announce& a) { if (vid != Value::INVALID_ID and (!a.value || a.value->id != vid)) - return false; + return true; if (isAnnounced(a.value->id)) { if (a.callback) { a.callback(true, getNodes()); a.callback = nullptr; } if (not a.permanent) - return true; + return false; } - return false; + return true; }); // remove acked for cleared annouces for (auto it = announced; it != announce.end(); ++it) {