Skip to content

Commit 7778ac9

Browse files
committed
Fix moveRegularItemOnEviction function
1 parent 043df5f commit 7778ac9

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
@@ -1271,35 +1271,17 @@ CacheAllocator<CacheTrait>::moveRegularItemOnEviction(
12711271
// make sure that no other thread removed it, and only then replaces it.
12721272
if (!replaceInMMContainer(oldItemPtr, *newItemHdl)) {
12731273
accessContainer_->remove(*newItemHdl);
1274-
return {};
1274+
return acquire(&oldItem);
12751275
}
12761276

12771277
// Replacing into the MM container was successful, but someone could have
12781278
// called insertOrReplace() or remove() before or after the
12791279
// replaceInMMContainer() operation, which would invalidate newItemHdl.
12801280
if (!newItemHdl->isAccessible()) {
12811281
removeFromMMContainer(*newItemHdl);
1282-
return {};
1282+
return acquire(&oldItem);
12831283
}
12841284

1285-
// no one can add or remove chained items at this point
1286-
if (oldItem.hasChainedItem()) {
1287-
// safe to acquire handle for a moving Item
1288-
auto oldHandle = acquire(&oldItem);
1289-
XDCHECK_EQ(1u, oldHandle->getRefCount()) << oldHandle->toString();
1290-
XDCHECK(!newItemHdl->hasChainedItem()) << newItemHdl->toString();
1291-
try {
1292-
auto l = chainedItemLocks_.lockExclusive(oldItem.getKey());
1293-
transferChainLocked(oldHandle, newItemHdl);
1294-
} catch (const std::exception& e) {
1295-
// this should never happen because we drained all the handles.
1296-
XLOGF(DFATAL, "{}", e.what());
1297-
throw;
1298-
}
1299-
1300-
XDCHECK(!oldItem.hasChainedItem());
1301-
XDCHECK(newItemHdl->hasChainedItem());
1302-
}
13031285
newItemHdl.unmarkNascent();
13041286
resHdl = std::move(newItemHdl); // guard will assign it to ctx under lock
13051287
return acquire(&oldItem);

cachelib/allocator/CacheAllocator.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,8 +1399,8 @@ class CacheAllocator : public CacheBase {
13991399
// @param oldItem Reference to the item being moved
14001400
// @param newItemHdl Reference to the handle of the new item being moved into
14011401
//
1402-
// @return true If the move was completed, and the containers were updated
1403-
// successfully.
1402+
// @return the handle to the oldItem if the move was completed
1403+
// and the oldItem can be recycled.
14041404
template <typename ItemPtr>
14051405
ItemHandle moveRegularItemOnEviction(ItemPtr& oldItem, ItemHandle& newItemHdl);
14061406

0 commit comments

Comments
 (0)