Skip to content

Commit 709929d

Browse files
committed
fix race in promotion where releaseBackToAllocator
was being called before wakeUpWaiters.
1 parent f4e30a7 commit 709929d

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

cachelib/allocator/CacheAllocator.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2031,7 +2031,7 @@ auto& mmContainer = getMMContainer(tid, pid, cid);
20312031
}
20322032

20332033
size_t traverseAndPromoteItems(unsigned int tid, unsigned int pid, unsigned int cid, size_t batch) {
2034-
auto& mmContainer = getMMContainer(tid, pid, cid);
2034+
auto& mmContainer = getMMContainer(tid, pid, cid);
20352035
size_t promotions = 0;
20362036
std::vector<Item*> candidates;
20372037
candidates.reserve(batch);
@@ -2051,6 +2051,8 @@ auto& mmContainer = getMMContainer(tid, pid, cid);
20512051
// TODO: only allow it for read-only items?
20522052
// or implement mvcc
20532053
if (candidate->markMoving(true)) {
2054+
// promotions should rarely fail since we already marked moving
2055+
mmContainer.remove(itr);
20542056
candidates.push_back(candidate);
20552057
}
20562058

@@ -2062,15 +2064,18 @@ auto& mmContainer = getMMContainer(tid, pid, cid);
20622064
auto promoted = tryPromoteToNextMemoryTier(*candidate, true);
20632065
if (promoted) {
20642066
promotions++;
2065-
removeFromMMContainer(*candidate);
20662067
XDCHECK(!candidate->isMarkedForEviction() && !candidate->isMoving());
20672068
// it's safe to recycle the item here as there are no more
20682069
// references and the item could not been marked as moving
20692070
// by other thread since it's detached from MMContainer.
2071+
//
2072+
// but we need to wake up waiters before releasing
2073+
// since candidate's key can change after being sent
2074+
// back to allocator
2075+
wakeUpWaiters(*candidate, std::move(promoted));
20702076
auto res = releaseBackToAllocator(*candidate, RemoveContext::kEviction,
20712077
/* isNascent */ false);
20722078
XDCHECK(res == ReleaseRes::kReleased);
2073-
wakeUpWaiters(*candidate, std::move(promoted));
20742079
} else {
20752080
// we failed to allocate a new item, this item is no longer moving
20762081
auto ref = unmarkMovingAndWakeUpWaiters(*candidate, {});

0 commit comments

Comments
 (0)