From: Kefu Chai <kchai@redhat.com>
Date: Wed, 29 Jul 2020 06:37:57 +0000 (+0800)
Subject: crimson/osd: erase an element by iterator instead
X-Git-Tag: v16.1.0~1600^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=815490bb2c6937e1ee0e76fa2592bb1a747e60bb;p=ceph.git

crimson/osd: erase an element by iterator instead

we should not remove an element while iterating it in a map, as erasing
the element invalidates the iterator, which causes segmfault when we are
advancing it after erasing the dereferenced element.

in this change, an iterator is used for walking through the map, in
comparision with creating a to-be-removed list, this one is more
efficient and more idiomatic.

Signed-off-by: Kefu Chai <kchai@redhat.com>
---

diff --git a/src/crimson/osd/heartbeat.cc b/src/crimson/osd/heartbeat.cc
index 3a33d7f93fbc..d92401bc3ab5 100644
--- a/src/crimson/osd/heartbeat.cc
+++ b/src/crimson/osd/heartbeat.cc
@@ -126,16 +126,20 @@ void Heartbeat::add_peer(osd_id_t _peer, epoch_t epoch)
 
 Heartbeat::osds_t Heartbeat::remove_down_peers()
 {
-  osds_t osds;
-  for (auto& [osd, peer] : peers) {
+  osds_t old_osds; // osds not added in this epoch
+  for (auto i = peers.begin(); i != peers.end(); ) {
     auto osdmap = service.get_osdmap_service().get_map();
+    const auto& [osd, peer] = *i;
     if (!osdmap->is_up(osd)) {
-      remove_peer(osd);
-    } else if (peer.get_epoch() < osdmap->get_epoch()) {
-      osds.push_back(osd);
+      i = peers.erase(i);
+    } else {
+      if (peer.get_epoch() < osdmap->get_epoch()) {
+        old_osds.push_back(osd);
+      }
+      ++i;
     }
   }
-  return osds;
+  return old_osds;
 }
 
 void Heartbeat::add_reporter_peers(int whoami)
diff --git a/src/crimson/osd/heartbeat.h b/src/crimson/osd/heartbeat.h
index 7e8608aa23af..d71d4e0c87c2 100644
--- a/src/crimson/osd/heartbeat.h
+++ b/src/crimson/osd/heartbeat.h
@@ -65,7 +65,7 @@ private:
   seastar::future<> handle_you_died();
 
   /// remove down OSDs
-  /// @return peers not needed in this epoch
+  /// @return peers not added in this epoch
   osds_t remove_down_peers();
   /// add enough reporters for fast failure detection
   void add_reporter_peers(int whoami);