Remove incorrect DCHECKS
authorDave Watson <davejwatson@fb.com>
Tue, 7 Nov 2017 15:38:44 +0000 (07:38 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Tue, 7 Nov 2017 15:56:04 +0000 (07:56 -0800)
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

folly/concurrency/detail/ConcurrentHashMap-detail.h
folly/concurrency/test/ConcurrentHashMapTest.cpp

index ebbb44f..6af07fe 100644 (file)
@@ -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() {
index 32b3d8a..824d37d 100644 (file)
@@ -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