Skip to content

Commit 66b7ba1

Browse files
committed
Fix moveRegularItemOnEviction function
1 parent b06846a commit 66b7ba1

File tree

2 files changed

+4
-22
lines changed

2 files changed

+4
-22
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,35 +1246,17 @@ CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
12461246
// make sure that no other thread removed it, and only then replaces it.
12471247
if (!replaceInMMContainer(oldItemPtr, *newItemHdl)) {
12481248
accessContainer_->remove(*newItemHdl);
1249-
return {};
1249+
return acquire(&oldItem);
12501250
}
12511251

12521252
// Replacing into the MM container was successful, but someone could have
12531253
// called insertOrReplace() or remove() before or after the
12541254
// replaceInMMContainer() operation, which would invalidate newItemHdl.
12551255
if (!newItemHdl->isAccessible()) {
12561256
removeFromMMContainer(*newItemHdl);
1257-
return {};
1257+
return acquire(&oldItem);
12581258
}
12591259

1260-
// no one can add or remove chained items at this point
1261-
if (oldItem.hasChainedItem()) {
1262-
// safe to acquire handle for a moving Item
1263-
auto oldHandle = acquire(&oldItem);
1264-
XDCHECK_EQ(1u, oldHandle->getRefCount()) << oldHandle->toString();
1265-
XDCHECK(!newItemHdl->hasChainedItem()) << newItemHdl->toString();
1266-
try {
1267-
auto l = chainedItemLocks_.lockExclusive(oldItem.getKey());
1268-
transferChainLocked(oldHandle, newItemHdl);
1269-
} catch (const std::exception& e) {
1270-
// this should never happen because we drained all the handles.
1271-
XLOGF(DFATAL, "{}", e.what());
1272-
throw;
1273-
}
1274-
1275-
XDCHECK(!oldItem.hasChainedItem());
1276-
XDCHECK(newItemHdl->hasChainedItem());
1277-
}
12781260
newItemHdl.unmarkNascent();
12791261
resHdl = std::move(newItemHdl); // guard will assign it to ctx under lock
12801262
return acquire(&oldItem);

cachelib/allocator/CacheAllocator.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,8 +1281,8 @@ class CacheAllocator : public CacheBase {
12811281
// @param oldItem Reference to the item being moved
12821282
// @param newItemHdl Reference to the handle of the new item being moved into
12831283
//
1284-
// @return true If the move was completed, and the containers were updated
1285-
// successfully.
1284+
// @return the handle to the oldItem if the move was completed
1285+
// and the oldItem can be recycled.
12861286
template <typename ItemPtr>
12871287
ItemHandle moveRegularItemOnEviction(ItemPtr& oldItem, ItemHandle& newItemHdl);
12881288

0 commit comments

Comments
 (0)