From ad11782029886819ec46bfbd7b237fd26774d2b4 Mon Sep 17 00:00:00 2001
From: Andrew Noyes <andrew@weaselab.dev>
Date: Sun, 3 Mar 2024 20:44:13 -0800
Subject: [PATCH] Fix iterator invalidation bug

Standard says operator[] may invalidate iterators. Never actually
crashed though /shrug
---
 HashTable.cpp | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/HashTable.cpp b/HashTable.cpp
index 8c42207..2886c4c 100644
--- a/HashTable.cpp
+++ b/HashTable.cpp
@@ -53,17 +53,21 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     if (keyUpdates < 100) {
       return;
     }
-
-    for (; keyUpdates > 0 && removalIter != map.end(); --keyUpdates) {
-      if (removalIter->second <= oldestVersion) {
-        removalIter = map.erase(removalIter);
+    auto iter = map.find(removalKey);
+    if (iter == map.end()) {
+      iter = map.begin();
+    }
+    for (; keyUpdates > 0 && iter != map.end(); --keyUpdates) {
+      if (iter->second <= oldestVersion) {
+        iter = map.erase(iter);
       } else {
-        ++removalIter;
+        ++iter;
       }
     }
-
-    if (removalIter == map.end()) {
-      removalIter = map.begin();
+    if (iter == map.end()) {
+      removalKey.clear();
+    } else {
+      removalKey = iter->first;
     }
   }
 
@@ -71,10 +75,7 @@ private:
   int64_t keyUpdates = 0;
   int64_t oldestVersion;
   std::unordered_map<std::string, int64_t, string_hash, std::equal_to<>> map;
-
-  // This iterator outliving the call to setOldestVersion is only safe because
-  // we only erase from within setOldestVersion
-  decltype(map.begin()) removalIter;
+  std::string removalKey;
 };
 
 void ConflictSet::check(const ReadRange *reads, Result *results,