diff options
author | Arne Juul <arnej@verizonmedia.com> | 2021-08-15 11:11:20 +0000 |
---|---|---|
committer | Arne Juul <arnej@verizonmedia.com> | 2021-08-15 11:11:24 +0000 |
commit | 5756300fcb367a11ed71c35b186618d9ba870bb6 (patch) | |
tree | 30245977813e0f207d187dae2b465b5eecda7393 /slobrok | |
parent | a97c478777f7c254d772df406b2afa0b6735261c (diff) |
remove locking in utility classes
* we need full restructuring to be thread-safe, so there's no
point in having locking (with potential for deadlocks) in
these particular utility classes at this time.
Diffstat (limited to 'slobrok')
4 files changed, 19 insertions, 44 deletions
diff --git a/slobrok/src/vespa/slobrok/server/service_map_history.cpp b/slobrok/src/vespa/slobrok/server/service_map_history.cpp index 5a2f8c6ff43..551ea367bc9 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_history.cpp +++ b/slobrok/src/vespa/slobrok/server/service_map_history.cpp @@ -43,8 +43,7 @@ ServiceMapHistory::UpdateLog::updatedSince(const Generation &gen) const { //----------------------------------------------------------------------------- ServiceMapHistory::ServiceMapHistory() - : _lock(), - _map(), + : _map(), _waitList(), _log() {} @@ -56,63 +55,49 @@ ServiceMapHistory::~ServiceMapHistory() { void ServiceMapHistory::notify_updated() { WaitList waitList; - { - std::lock_guard guard(_lock); - std::swap(waitList, _waitList); - } + std::swap(waitList, _waitList); for (auto & [ handler, gen ] : waitList) { handler->handle(makeDiffFrom(gen)); } } void ServiceMapHistory::asyncGenerationDiff(DiffCompletionHandler *handler, const Generation &fromGen) { - { - std::lock_guard guard(_lock); - if (fromGen == myGen()) { - _waitList.emplace_back(handler, fromGen); - return; - } + if (fromGen == myGen()) { + _waitList.emplace_back(handler, fromGen); + return; } handler->handle(makeDiffFrom(fromGen)); } bool ServiceMapHistory::cancel(DiffCompletionHandler *handler) { - std::lock_guard guard(_lock); size_t removed = std::erase_if(_waitList, [=](const Waiter &elem){ return elem.first == handler; }); return (removed > 0); } void ServiceMapHistory::remove(const ServiceMapping &mapping) { - { - std::lock_guard guard(_lock); - auto iter = _map.find(mapping.name); - if (iter == _map.end()) { - LOG(debug, "already removed: %s", mapping.name.c_str()); - return; // already removed - } - LOG_ASSERT(iter->second == mapping.spec); - _map.erase(iter); - _log.add(mapping.name); + auto iter = _map.find(mapping.name); + if (iter == _map.end()) { + LOG(debug, "already removed: %s", mapping.name.c_str()); + return; // already removed } + LOG_ASSERT(iter->second == mapping.spec); + _map.erase(iter); + _log.add(mapping.name); notify_updated(); } void ServiceMapHistory::add(const ServiceMapping &mapping) { - { - std::lock_guard guard(_lock); - auto iter = _map.find(mapping.name); - if (iter != _map.end() && iter->second == mapping.spec) { - // already ok - return; - } - _map.insert_or_assign(mapping.name, mapping.spec); - _log.add(mapping.name); + auto iter = _map.find(mapping.name); + if (iter != _map.end() && iter->second == mapping.spec) { + // already ok + return; } + _map.insert_or_assign(mapping.name, mapping.spec); + _log.add(mapping.name); notify_updated(); } MapDiff ServiceMapHistory::makeDiffFrom(const Generation &fromGen) const { - std::lock_guard guard(_lock); if (_log.isInRange(fromGen)) { std::vector<vespalib::string> removes; ServiceMappingList updates; diff --git a/slobrok/src/vespa/slobrok/server/service_map_history.h b/slobrok/src/vespa/slobrok/server/service_map_history.h index 9026586b945..726ba778ca2 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_history.h +++ b/slobrok/src/vespa/slobrok/server/service_map_history.h @@ -6,7 +6,6 @@ #include <vespa/vespalib/util/gencnt.h> #include <vespa/vespalib/util/arrayqueue.hpp> #include <map> -#include <mutex> #include "map_listener.h" #include "service_mapping.h" #include "map_diff.h" @@ -52,7 +51,6 @@ private: using Waiter = std::pair<DiffCompletionHandler *, Generation>; using WaitList = std::vector<Waiter>; - mutable std::mutex _lock; Map _map; WaitList _waitList; UpdateLog _log; diff --git a/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp b/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp index 51db84e24a4..90deaa0f09c 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp +++ b/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp @@ -8,8 +8,7 @@ namespace slobrok { ServiceMapMirror::ServiceMapMirror() : _map(), - _currGen(0), - _lock() + _currGen(0) {} ServiceMapMirror::~ServiceMapMirror() { @@ -17,7 +16,6 @@ ServiceMapMirror::~ServiceMapMirror() { } void ServiceMapMirror::apply(const MapDiff &diff) { - std::lock_guard guard(_lock); LOG(debug, "Applying diff from gen %u", diff.fromGen.getAsInt()); LOG_ASSERT(diff.fromGen == _currGen); for (const auto & name : diff.removed) { @@ -54,7 +52,6 @@ void ServiceMapMirror::apply(const MapDiff &diff) { } void ServiceMapMirror::clear() { - std::lock_guard guard(_lock); for (const auto & [ k, v ] : _map) { ServiceMapping mapping{k, v}; for (auto * listener : _listeners) { @@ -66,7 +63,6 @@ void ServiceMapMirror::clear() { } ServiceMappingList ServiceMapMirror::allMappings() const { - std::lock_guard guard(_lock); ServiceMappingList result; result.reserve(_map.size()); for (const auto & [ k, v ] : _map) { @@ -76,12 +72,10 @@ ServiceMappingList ServiceMapMirror::allMappings() const { } void ServiceMapMirror::registerListener(MapListener &listener) { - std::lock_guard guard(_lock); _listeners.insert(&listener); } void ServiceMapMirror::unregisterListener(MapListener &listener) { - std::lock_guard guard(_lock); _listeners.erase(&listener); } diff --git a/slobrok/src/vespa/slobrok/server/service_map_mirror.h b/slobrok/src/vespa/slobrok/server/service_map_mirror.h index 4cd1c91b7f1..7418aeaec92 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_mirror.h +++ b/slobrok/src/vespa/slobrok/server/service_map_mirror.h @@ -7,7 +7,6 @@ #include "map_source.h" #include <vespa/vespalib/util/gencnt.h> #include <map> -#include <mutex> #include <set> namespace slobrok { @@ -41,7 +40,6 @@ private: using Map = std::map<vespalib::string, vespalib::string>; Map _map; Generation _currGen; - mutable std::mutex _lock; std::set<MapListener *> _listeners; }; |