Skip to content

Commit e4245f8

Browse files
committed
Fixed race where insertion into implicit producer hash could accidentally skip a tombstoned slot, forcing an empty slot to be sacrificed for a reused producer, which in extreme cases can cause the hash to break its 0.75 load factor invariant (see #288)
1 parent 38d01f4 commit e4245f8

1 file changed

Lines changed: 8 additions & 12 deletions

File tree

concurrentqueue.h

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3418,14 +3418,13 @@ class ConcurrentQueue
34183418
index = hashedId;
34193419
while (true) {
34203420
index &= mainHash->capacity - 1;
3421-
probedKey = mainHash->entries[index].key.load(std::memory_order_relaxed);
34223421
auto empty = details::invalid_thread_id;
34233422
#ifdef MOODYCAMEL_CPP11_THREAD_LOCAL_SUPPORTED
34243423
auto reusable = details::invalid_thread_id2;
3425-
if ((probedKey == empty && mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_relaxed, std::memory_order_relaxed)) ||
3426-
(probedKey == reusable && mainHash->entries[index].key.compare_exchange_strong(reusable, id, std::memory_order_acquire, std::memory_order_acquire))) {
3424+
if (mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_seq_cst, std::memory_order_relaxed) ||
3425+
mainHash->entries[index].key.compare_exchange_strong(reusable, id, std::memory_order_seq_cst, std::memory_order_relaxed)) {
34273426
#else
3428-
if ((probedKey == empty && mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_relaxed, std::memory_order_relaxed))) {
3427+
if (mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_seq_cst, std::memory_order_relaxed)) {
34293428
#endif
34303429
mainHash->entries[index].value = value;
34313430
break;
@@ -3506,15 +3505,13 @@ class ConcurrentQueue
35063505
auto index = hashedId;
35073506
while (true) {
35083507
index &= mainHash->capacity - 1;
3509-
auto probedKey = mainHash->entries[index].key.load(std::memory_order_relaxed);
3510-
35113508
auto empty = details::invalid_thread_id;
35123509
#ifdef MOODYCAMEL_CPP11_THREAD_LOCAL_SUPPORTED
35133510
auto reusable = details::invalid_thread_id2;
3514-
if ((probedKey == empty && mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_relaxed, std::memory_order_relaxed)) ||
3515-
(probedKey == reusable && mainHash->entries[index].key.compare_exchange_strong(reusable, id, std::memory_order_acquire, std::memory_order_acquire))) {
3511+
if (mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_seq_cst, std::memory_order_relaxed) ||
3512+
mainHash->entries[index].key.compare_exchange_strong(reusable, id, std::memory_order_seq_cst, std::memory_order_relaxed)) {
35163513
#else
3517-
if ((probedKey == empty && mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_relaxed, std::memory_order_relaxed))) {
3514+
if (mainHash->entries[index].key.compare_exchange_strong(empty, id, std::memory_order_seq_cst, std::memory_order_relaxed)) {
35183515
#endif
35193516
mainHash->entries[index].value = producer;
35203517
break;
@@ -3553,9 +3550,8 @@ class ConcurrentQueue
35533550
auto index = hashedId;
35543551
do {
35553552
index &= hash->capacity - 1;
3556-
probedKey = hash->entries[index].key.load(std::memory_order_relaxed);
3557-
if (probedKey == id) {
3558-
hash->entries[index].key.store(details::invalid_thread_id2, std::memory_order_release);
3553+
probedKey = id;
3554+
if (hash->entries[index].key.compare_exchange_strong(probedKey, details::invalid_thread_id2, std::memory_order_seq_cst, std::memory_order_relaxed)) {
35593555
break;
35603556
}
35613557
++index;

0 commit comments

Comments
 (0)