Commit d8d3e267 authored by Mike Kolupaev's avatar Mike Kolupaev Committed by Dave Watson

Fixed folly::AtomicHashMap::iterator not advancing past empty submaps

Summary: A potential "real life" scenario that maybe can hit this bug is if we erase almost all elements and then iterate over the whole map.

Test Plan: Added a test for it.

Reviewed By: mwilliams@fb.com

Subscribers: folly-diffs@, lovro

FB internal diff: D1751455

Tasks: 5866368

Signature: t1:1751455:1419016611:b44c41d348f54397844460cb38002ad0d9704972
parent 0b028d4a
...@@ -408,7 +408,7 @@ struct AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::ahm_iterator ...@@ -408,7 +408,7 @@ struct AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::ahm_iterator
SubMap* thisMap = ahm_->subMaps_[subMap_]. SubMap* thisMap = ahm_->subMaps_[subMap_].
load(std::memory_order_relaxed); load(std::memory_order_relaxed);
if (subIt_ == thisMap->end()) { while (subIt_ == thisMap->end()) {
// This sub iterator is done, advance to next one // This sub iterator is done, advance to next one
if (subMap_ + 1 < if (subMap_ + 1 <
ahm_->numMapsAllocated_.load(std::memory_order_acquire)) { ahm_->numMapsAllocated_.load(std::memory_order_acquire)) {
...@@ -417,6 +417,7 @@ struct AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::ahm_iterator ...@@ -417,6 +417,7 @@ struct AtomicHashMap<KeyT, ValueT, HashFcn, EqualFcn, Allocator>::ahm_iterator
subIt_ = thisMap->begin(); subIt_ = thisMap->begin();
} else { } else {
ahm_ = nullptr; ahm_ = nullptr;
return;
} }
} }
} }
......
...@@ -618,6 +618,35 @@ TEST(Ahm, atomic_hash_array_insert_race) { ...@@ -618,6 +618,35 @@ TEST(Ahm, atomic_hash_array_insert_race) {
} }
} }
// Repro for a bug when iterator didn't skip empty submaps.
TEST(Ahm, iterator_skips_empty_submaps) {
AtomicHashMap<uint64_t, uint64_t>::Config config;
config.growthFactor = 1;
AtomicHashMap<uint64_t, uint64_t> map(1, config);
map.insert(1, 1);
map.insert(2, 2);
map.insert(3, 3);
map.erase(2);
auto it = map.find(1);
ASSERT_NE(map.end(), it);
ASSERT_EQ(1, it->first);
ASSERT_EQ(1, it->second);
++it;
ASSERT_NE(map.end(), it);
ASSERT_EQ(3, it->first);
ASSERT_EQ(3, it->second);
++it;
ASSERT_EQ(map.end(), it);
}
namespace { namespace {
void loadGlobalAha() { void loadGlobalAha() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment