more ConcurrentHashMap deletion tests

Summary:
Re-committing.  Original: D6993857 revert: D7051998

Fixes for hazptr deletion on map destruction.
Several iteration cases don't properly release their hazard pointers.
This isn't disastrous, but means objects may stick around longer than necessary.
Adds tests and fixes for iteration to ensure we eagerly reset all hazard pointers.

Reviewed By: yfeldblum

Differential Revision: D7063727

fbshipit-source-id: 2ae32ead4965b37866096d2a8c6ea8c42c201335

folly/concurrency/ConcurrentHashMap.h

@@ -444,13 +444,17 @@ class ConcurrentHashMap {
     void next() {
       while (it_ == parent_->ensureSegment(segment_)->cend() &&
              segment_ < parent_->NumShards) {
-        segment_++;
-        auto seg = parent_->segments_[segment_].load(std::memory_order_acquire);
-        if (segment_ < parent_->NumShards) {
-          if (!seg) {
-            continue;
+        SegmentT* seg{nullptr};
+        while (!seg) {
+          segment_++;
+          seg = parent_->segments_[segment_].load(std::memory_order_acquire);
+          if (segment_ < parent_->NumShards) {
+            if (!seg) {
+              continue;
+            }
+            it_ = seg->cbegin();
           }
-          it_ = seg->cbegin();
+          break;
         }
       }
     }

folly/concurrency/detail/ConcurrentHashMap-detail.h

@@ -589,6 +589,8 @@ class alignas(64) ConcurrentHashMapSegment {
   // throw if hash or key_eq functions throw.
   void erase(Iterator& res, Iterator& pos) {
     erase_internal(pos->first, &res);
+    // Invalidate the iterator.
+    pos = cend();
   }
 
   void clear() {

folly/concurrency/test/ConcurrentHashMapTest.cpp

@@ -594,25 +594,107 @@ TEST(ConcurrentHashMap, RefcountTest) {
 }
 
 struct Wrapper {
-  Wrapper() = default;
+  explicit Wrapper(bool& del_) : del(del_) {}
   ~Wrapper() {
     del = true;
   }
 
-  static bool del;
+  bool& del;
 };
 
-bool Wrapper::del = false;
-
 TEST(ConcurrentHashMap, Deletion) {
-  EXPECT_FALSE(Wrapper::del);
+  bool del{false};
 
   {
     ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map;
 
-    map.insert(0, std::make_shared<Wrapper>());
+    map.insert(0, std::make_shared<Wrapper>(del));
+  }
+
+  EXPECT_TRUE(del);
+}
+
+TEST(ConcurrentHashMap, DeletionWithErase) {
+  bool del{false};
+
+  {
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map;
+
+    map.insert(0, std::make_shared<Wrapper>(del));
     map.erase(0);
   }
 
-  EXPECT_TRUE(Wrapper::del);
+  EXPECT_TRUE(del);
+}
+
+TEST(ConcurrentHashMap, DeletionWithIterator) {
+  bool del{false};
+
+  {
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map;
+
+    map.insert(0, std::make_shared<Wrapper>(del));
+    auto it = map.find(0);
+    map.erase(it);
+  }
+
+  EXPECT_TRUE(del);
+}
+
+TEST(ConcurrentHashMap, DeletionWithForLoop) {
+  bool del{false};
+
+  {
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map;
+
+    map.insert(0, std::make_shared<Wrapper>(del));
+    for (auto it = map.cbegin(); it != map.cend(); ++it) {
+      EXPECT_EQ(it->first, 0);
+    }
+  }
+
+  EXPECT_TRUE(del);
+}
+
+TEST(ConcurrentHashMap, DeletionMultiple) {
+  bool del1{false}, del2{false};
+
+  {
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map;
+
+    map.insert(0, std::make_shared<Wrapper>(del1));
+    map.insert(1, std::make_shared<Wrapper>(del2));
+  }
+
+  EXPECT_TRUE(del1);
+  EXPECT_TRUE(del2);
+}
+
+TEST(ConcurrentHashMap, DeletionAssigned) {
+  bool del1{false}, del2{false};
+
+  {
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map;
+
+    map.insert(0, std::make_shared<Wrapper>(del1));
+    map.insert_or_assign(0, std::make_shared<Wrapper>(del2));
+  }
+
+  EXPECT_TRUE(del1);
+  EXPECT_TRUE(del2);
+}
+
+TEST(ConcurrentHashMap, DeletionMultipleMaps) {
+  bool del1{false}, del2{false};
+
+  {
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map1;
+    ConcurrentHashMap<int, std::shared_ptr<Wrapper>> map2;
+
+    map1.insert(0, std::make_shared<Wrapper>(del1));
+    map2.insert(0, std::make_shared<Wrapper>(del2));
+  }
+
+  EXPECT_TRUE(del1);
+  EXPECT_TRUE(del2);
 }