Fix use-after-unmap in markMovingForSlabRelease

Summary:
I noticed that in coredumper there have been crashes in fookeep for some time which should be fixed with this diff.

Fix a race condition where `toString()` accesses slab memory after
`abortSlabRelease()` has restored the slab to normal operation. Another
thread (e.g. MemoryMonitor) can pick up the same slab, release it, and
call `adviseSlab()` which does `madvise(MADV_REMOVE)`, unmapping the
pages before the first thread reads from them — causing SIGSEGV.

The fix captures the item description before aborting, while the slab is
still marked for release and protected from being picked by other threads.

I didn't test if this actually fixes the issue because I am not sure how to reproduce it. We can check coredumper in a few weeks to see if the issue is really fixed.

Reviewed By: pbhandar2, rlyerly

Differential Revision: D97321920

fbshipit-source-id: 05c2929389393ca5355037e9325f3d6c2f45e3db

Author: AlnisM

cachelib/allocator/CacheAllocator.h

@@ -5432,6 +5432,31 @@ bool CacheAllocator<CacheTrait>::markMovingForSlabRelease(
     });
   };
 
+  // Snapshot the item string under processAllocForRelease() protection.
+  // Reading alloc directly here races with concurrent free(), while doing it
+  // after abortSlabRelease() can race with the slab being advised away.
+  auto captureItemStringForAbort = [&]() {
+    std::string itemStr = "item <already freed before abort>";
+    try {
+      allocator_->processAllocForRelease(ctx, alloc, [&](void* memory) {
+        itemStr = static_cast<Item*>(memory)->toString();
+      });
+    } catch (const std::exception& e) {
+      itemStr =
+          folly::sformat("<failed to capture item for abort: {}>", e.what());
+    }
+    return itemStr;
+  };
+
+  auto abortWithMessage = [&](const std::string& reason) {
+    auto itemStr = captureItemStringForAbort();
+    allocator_->abortSlabRelease(ctx);
+    throw exception::SlabReleaseAborted(
+        folly::sformat("Slab Release aborted {} while still trying to mark"
+                       " as moving for Item: {}. Pool: {}, Class: {}.",
+                       reason, itemStr, ctx.getPoolId(), ctx.getClassId()));
+  };
+
   auto startTime = util::getCurrentTimeMs();
   while (true) {
     allocator_->processAllocForRelease(ctx, alloc, fn);
@@ -5449,25 +5474,14 @@ bool CacheAllocator<CacheTrait>::markMovingForSlabRelease(
     itemFreed = true;
 
     if (isShutdownInProgress()) {
-      allocator_->abortSlabRelease(ctx);
-      throw exception::SlabReleaseAborted(
-          folly::sformat("Slab Release aborted while still trying to mark"
-                         " as moving for Item: {}. Pool: {}, Class: {}.",
-                         static_cast<Item*>(alloc)->toString(), ctx.getPoolId(),
-                         ctx.getClassId()));
+      abortWithMessage("due to shutdown");
     }
 
     if (config_.slabRebalanceTimeout.count() > 0) {
       auto elapsedTime = util::getCurrentTimeMs() - startTime;
       if (elapsedTime >
           static_cast<uint64_t>(config_.slabRebalanceTimeout.count())) {
-        allocator_->abortSlabRelease(ctx);
-        throw exception::SlabReleaseAborted(
-            folly::sformat("Slab Release aborted after {} ms while still"
-                           " trying to mark as moving for Item: {}. Pool: {},"
-                           " Class: {}.",
-                           elapsedTime, static_cast<Item*>(alloc)->toString(),
-                           ctx.getPoolId(), ctx.getClassId()));
+        abortWithMessage(folly::sformat("after {} ms", elapsedTime));
       }
     }