Remove incorrect DCHECKS

Summary:
While the returned iterators are always 'valid' and can be read and iterated from,
they may still be concurrently erased from the map.  Current erase(iter) has DCHECKS that assert we can always
erase an iterator, but it may have already been removed.

Reviewed By: davidtgoldblatt

Differential Revision: D6221382

fbshipit-source-id: 70b21f53e2fc3daa126df4fb60bc5d3ecb253c71

diff --git a/folly/concurrency/detail/ConcurrentHashMap-detail.h b/folly/concurrency/detail/ConcurrentHashMap-detail.h
index ebbb44fc400181ceb3ee937ead97b71d2f303b7a..6af07feef71e461977e2ee9aaa33ab827b52ffc0 100644 (file)
--- a/folly/concurrency/detail/ConcurrentHashMap-detail.h
+++ b/folly/concurrency/detail/ConcurrentHashMap-detail.h
@@ -543,7 +543,6 @@ class FOLLY_ALIGNED(64) ConcurrentHashMapSegment {
       node->release();
       return 1;
     }
-    DCHECK(!iter);
 
     return 0;
   }
@@ -555,8 +554,7 @@ class FOLLY_ALIGNED(64) ConcurrentHashMapSegment {
   // This is a small departure from standard stl containers: erase may
   // throw if hash or key_eq functions throw.
   void erase(Iterator& res, Iterator& pos) {
-    auto cnt = erase_internal(pos->first, &res);
-    DCHECK(cnt == 1);
+    erase_internal(pos->first, &res);
   }
 
   void clear() {

diff --git a/folly/concurrency/test/ConcurrentHashMapTest.cpp b/folly/concurrency/test/ConcurrentHashMapTest.cpp
index 32b3d8ae4de4176944d4ec6912f3b4a451d2b799..824d37da35f4940b711495fd1d95f63ef1f2f658 100644 (file)
--- a/folly/concurrency/test/ConcurrentHashMapTest.cpp
+++ b/folly/concurrency/test/ConcurrentHashMapTest.cpp
@@ -208,6 +208,14 @@ TEST(ConcurrentHashMap, MapIterateTest) {
   EXPECT_EQ(count, 2);
 }
 
+TEST(ConcurrentHashMap, EraseTest) {
+  ConcurrentHashMap<uint64_t, uint64_t> foomap(3);
+  foomap.insert(1, 0);
+  auto f1 = foomap.find(1);
+  EXPECT_EQ(1, foomap.erase(1));
+  foomap.erase(f1);
+}
+
 // TODO: hazptrs must support DeterministicSchedule
 
 #define Atom std::atomic // DeterministicAtomic