From baa03294ee862eb532108797267dc72497afaa16 Mon Sep 17 00:00:00 2001 From: Yanbo Xu Date: Fri, 15 Dec 2017 13:43:53 -0800 Subject: [PATCH] Fix erase in Iterate Summary: The iterator returned from erase api could skip nodes. The fix is to initialize the returned iterator with right value. Reviewed By: djwatson Differential Revision: D6579707 fbshipit-source-id: a45f47a53e106d22daa9cf57be6c40c4f6a430d9 --- folly/concurrency/ConcurrentHashMap.h | 4 +++- .../concurrency/test/ConcurrentHashMapTest.cpp | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/folly/concurrency/ConcurrentHashMap.h b/folly/concurrency/ConcurrentHashMap.h index b4d2fc3a..a2cf218a 100644 --- a/folly/concurrency/ConcurrentHashMap.h +++ b/folly/concurrency/ConcurrentHashMap.h @@ -421,7 +421,9 @@ class ConcurrentHashMap { } ConstIterator(const ConcurrentHashMap* parent, uint64_t segment) - : segment_(segment), parent_(parent) {} + : it_(parent->ensureSegment(segment)->cbegin()), + segment_(segment), + parent_(parent) {} private: // cbegin iterator diff --git a/folly/concurrency/test/ConcurrentHashMapTest.cpp b/folly/concurrency/test/ConcurrentHashMapTest.cpp index b1b18e42..c16985cd 100644 --- a/folly/concurrency/test/ConcurrentHashMapTest.cpp +++ b/folly/concurrency/test/ConcurrentHashMapTest.cpp @@ -257,6 +257,24 @@ TEST(ConcurrentHashMap, EraseTest) { foomap.erase(f1); } +TEST(ConcurrentHashMap, EraseInIterateTest) { + ConcurrentHashMap foomap(3); + for (uint64_t k = 0; k < 10; ++k) { + foomap.insert(k, k); + } + for (auto it = foomap.cbegin(); it != foomap.cend();) { + if (it->second > 3) { + it = foomap.erase(it); + } else { + ++it; + } + } + EXPECT_EQ(4, foomap.size()); + for (auto it = foomap.cbegin(); it != foomap.cend(); ++it) { + EXPECT_GE(3, it->second); + } +} + // TODO: hazptrs must support DeterministicSchedule #define Atom std::atomic // DeterministicAtomic -- 2.34.1