Skip to content

Commit ea3191a

Browse files
byrnedjvinser52
authored andcommitted
fix race in promotion where releaseBackToAllocator
was being called before wakeUpWaiters.
1 parent eaadf9d commit ea3191a

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
@@ -2061,7 +2061,7 @@ auto& mmContainer = getMMContainer(tid, pid, cid);
20612061
}
20622062

20632063
size_t traverseAndPromoteItems(unsigned int tid, unsigned int pid, unsigned int cid, size_t batch) {
2064-
auto& mmContainer = getMMContainer(tid, pid, cid);
2064+
auto& mmContainer = getMMContainer(tid, pid, cid);
20652065
size_t promotions = 0;
20662066
std::vector<Item*> candidates;
20672067
candidates.reserve(batch);
@@ -2081,6 +2081,8 @@ auto& mmContainer = getMMContainer(tid, pid, cid);
20812081
// TODO: only allow it for read-only items?
20822082
// or implement mvcc
20832083
if (candidate->markMoving(true)) {
2084+
// promotions should rarely fail since we already marked moving
2085+
mmContainer.remove(itr);
20842086
candidates.push_back(candidate);
20852087
}
20862088

@@ -2092,15 +2094,18 @@ auto& mmContainer = getMMContainer(tid, pid, cid);
20922094
auto promoted = tryPromoteToNextMemoryTier(*candidate, true);
20932095
if (promoted) {
20942096
promotions++;
2095-
removeFromMMContainer(*candidate);
20962097
XDCHECK(!candidate->isMarkedForEviction() && !candidate->isMoving());
20972098
// it's safe to recycle the item here as there are no more
20982099
// references and the item could not been marked as moving
20992100
// by other thread since it's detached from MMContainer.
2101+
//
2102+
// but we need to wake up waiters before releasing
2103+
// since candidate's key can change after being sent
2104+
// back to allocator
2105+
wakeUpWaiters(*candidate, std::move(promoted));
21002106
auto res = releaseBackToAllocator(*candidate, RemoveContext::kEviction,
21012107
/* isNascent */ false);
21022108
XDCHECK(res == ReleaseRes::kReleased);
2103-
wakeUpWaiters(*candidate, std::move(promoted));
21042109
} else {
21052110
// we failed to allocate a new item, this item is no longer moving
21062111
auto ref = unmarkMovingAndWakeUpWaiters(*candidate, {});

0 commit comments

Comments
 (0)