Merge pull request #28006 from vespa-engine/balder/use-unique_ptr-to-enable-safe-referencing

- Since the vespalib::hash_map iterators are invalidated on modificat…

7 files changed, 16 insertions, 20 deletions

diff --git a/storage/src/tests/distributor/distributor_bucket_space_test.cpp b/storage/src/tests/distributor/distributor_bucket_space_test.cpp
index 41e0dafdaaf..6e5863298ce 100644
--- a/storage/src/tests/distributor/distributor_bucket_space_test.cpp
+++ b/storage/src/tests/distributor/distributor_bucket_space_test.cpp
@@ -102,7 +102,7 @@ DistributorBucketSpaceTest::count_service_layer_buckets(const std::vector<Bucket
     CountVector result(3);
     std::vector<uint16_t> ideal_nodes;
     for (auto& bucket : buckets) {
-        auto &ideal_nodes_bundle = bucket_space.get_ideal_service_layer_nodes_bundle(bucket);
+        const auto & ideal_nodes_bundle = bucket_space.get_ideal_service_layer_nodes_bundle(bucket);
         for (uint32_t i = 0; i < 3; ++i) {
             switch (i) {
             case 0:
diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
index 5969ccad4cb..5ccdb214854 100644
--- a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
@@ -155,13 +155,13 @@ DistributorBucketSpace::get_ideal_service_layer_nodes_bundle(document::BucketId
     document::BucketId lookup_bucket(is_split_group_bucket(bucket) ? bucket.getUsedBits() : _distribution_bits, bucket.getId());
     auto itr = _ideal_nodes.find(lookup_bucket);
     if (itr != _ideal_nodes.end()) {
-        return itr->second;
+        return *itr->second;
     }
-    IdealServiceLayerNodesBundle ideal_nodes_bundle;
-    setup_ideal_nodes_bundle(ideal_nodes_bundle, *_distribution, *_clusterState, lookup_bucket);
+    auto ideal_nodes_bundle = std::make_unique<IdealServiceLayerNodesBundle>();
+    setup_ideal_nodes_bundle(*ideal_nodes_bundle, *_distribution, *_clusterState, lookup_bucket);
     auto insres = _ideal_nodes.insert(std::make_pair(lookup_bucket, std::move(ideal_nodes_bundle)));
     assert(insres.second);
-    return insres.first->second;
+    return *insres.first->second;
 }
 
 BucketOwnershipFlags
diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h
index f38556a664c..a66f0e5e983 100644
--- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h
+++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h
@@ -41,7 +41,7 @@ class DistributorBucketSpace {
     std::shared_ptr<const lib::ClusterState> _pending_cluster_state;
     std::vector<bool>                        _available_nodes;
     mutable vespalib::hash_map<document::BucketId, BucketOwnershipFlags, document::BucketId::hash>  _ownerships;
-    mutable vespalib::hash_map<document::BucketId, IdealServiceLayerNodesBundle, document::BucketId::hash> _ideal_nodes;
+    mutable vespalib::hash_map<document::BucketId, std::unique_ptr<IdealServiceLayerNodesBundle>, document::BucketId::hash> _ideal_nodes;
 
     void clear();
     void enumerate_available_nodes();
diff --git a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h
index 929ec7aadc1..8f9cb8e14ec 100644
--- a/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h
+++ b/storage/src/vespa/storage/distributor/ideal_service_layer_nodes_bundle.h
@@ -27,9 +27,9 @@ public:
     void set_available_nonretired_or_maintenance_nodes(std::vector<uint16_t> available_nonretired_or_maintenance_nodes) {
         _available_nonretired_or_maintenance_nodes = std::move(available_nonretired_or_maintenance_nodes);
     }
-    std::vector<uint16_t> get_available_nodes() const { return _available_nodes; }
-    std::vector<uint16_t> get_available_nonretired_nodes() const { return _available_nonretired_nodes; }
-    std::vector<uint16_t> get_available_nonretired_or_maintenance_nodes() const {
+    const std::vector<uint16_t> & get_available_nodes() const { return _available_nodes; }
+    const std::vector<uint16_t> & get_available_nonretired_nodes() const { return _available_nonretired_nodes; }
+    const std::vector<uint16_t> & get_available_nonretired_or_maintenance_nodes() const {
         return _available_nonretired_or_maintenance_nodes;
     }
 };
diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
index 8c6fdb314f3..324cb3691e4 100644
--- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp
@@ -67,8 +67,7 @@ PutOperation::insertDatabaseEntryAndScheduleCreateBucket(const OperationTargetLi
         assert(!multipleBuckets);
         (void) multipleBuckets;
         BucketDatabase::Entry entry(_bucket_space.getBucketDatabase().get(lastBucket));
-        std::vector<uint16_t> idealState(
-                _bucket_space.get_ideal_service_layer_nodes_bundle(lastBucket).get_available_nodes());
+        const std::vector<uint16_t> & idealState = _bucket_space.get_ideal_service_layer_nodes_bundle(lastBucket).get_available_nodes();
         active = ActiveCopy::calculate(idealState, _bucket_space.getDistribution(), entry,
                                        _op_ctx.distributor_config().max_activation_inhibited_out_of_sync_groups());
         LOG(debug, "Active copies for bucket %s: %s", entry.getBucketId().toString().c_str(), active.toString().c_str());
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
index 6a9d7e0e6da..e3aa7e1ef6e 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
+++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp
@@ -56,15 +56,12 @@ BucketInstanceList::contains(lib::Node node) const {
 }
 
 void
-BucketInstanceList::add(BucketDatabase::Entry& e,
-                        const lib::IdealNodeList& idealState)
+BucketInstanceList::add(const BucketDatabase::Entry& e, const lib::IdealNodeList& idealState)
 {
     for (uint32_t i = 0; i < e.getBucketInfo().getNodeCount(); ++i) {
         const BucketCopy& copy(e.getBucketInfo().getNodeRef(i));
         lib::Node node(lib::NodeType::STORAGE, copy.getNode());
-        _instances.push_back(BucketInstance(
-                e.getBucketId(), copy.getBucketInfo(), node,
-                idealState.indexOf(node), copy.trusted()));
+        _instances.emplace_back(e.getBucketId(), copy.getBucketInfo(), node, idealState.indexOf(node), copy.trusted());
     }
 }
 
@@ -73,9 +70,9 @@ BucketInstanceList::populate(const document::BucketId& specificId, const Distrib
 {
     std::vector<BucketDatabase::Entry> entries;
     db.getParents(specificId, entries);
-    for (uint32_t i=0; i<entries.size(); ++i) {
-        lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(entries[i].getBucketId()).get_available_nonretired_or_maintenance_nodes()));
-        add(entries[i], idealNodes);
+    for (const auto & entry : entries) {
+        lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(entry.getBucketId()).get_available_nonretired_or_maintenance_nodes()));
+        add(entry, idealNodes);
     }
 }
 
diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h
index 9ff65475fa4..0caeee466e0 100644
--- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h
+++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.h
@@ -65,7 +65,7 @@ public:
                               const document::BucketId& mostSpecificId);
 
     void populate(const document::BucketId&, const DistributorBucketSpace&, BucketDatabase&);
-    void add(BucketDatabase::Entry& e, const lib::IdealNodeList& idealState);
+    void add(const BucketDatabase::Entry& e, const lib::IdealNodeList& idealState);
 
     template <typename Order>
     void sort(const Order& order) {