From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 17 Mar 2023 14:50:15 +0000 (+0000)
Subject: Fix rock/RockSwapDir.cc "slot->sameKey()" assertion (#1310)
X-Git-Tag: SQUID_7_0_1~460
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=682b7fcd1a80df0ebfaca7c410e4210cdb7d059e;p=thirdparty%2Fsquid.git

Fix rock/RockSwapDir.cc "slot->sameKey()" assertion (#1310)

Introduced in 2014 commit fa2301d, the assertion was too strict: Due to
store_client idiosyncrasies, openStoreIO() may be called long after the
initial cache hit is detected and locked for reading. It is possible
that the StoreEntry object has gone private since that detection and
locking. Private StoreEntry objects lose their public keys. The existing
anchor lock assures that we still point to the right disk entry, but we
cannot check that invariant using the now-private StoreEntry key.

There are no known sightings of this assertion in the wild, but we can
now reproduce it using high-load Web Polygraph tests. It is not clear
whether some other Squid changes made the triggering sequence of events
a bit more likely, or we were just lucky to discover the right workload.
---

diff --git a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc
index 758b49ddf4..e14487b43f 100644
--- a/src/fs/rock/RockSwapDir.cc
+++ b/src/fs/rock/RockSwapDir.cc
@@ -791,7 +791,17 @@ Rock::SwapDir::openStoreIO(StoreEntry &e, StoreIOState::STIOCB * const cbIo, voi
            std::setfill('0') << std::hex << std::uppercase << std::setw(8) <<
            sio->swap_filen);
 
-    assert(slot->sameKey(static_cast<const cache_key*>(e.key)));
+    // When StoreEntry::swap_filen for e was set by our anchorEntry(), e had a
+    // public key, but it could have gone private since then (while keeping the
+    // anchor lock). The stale anchor key is not (and cannot be) erased (until
+    // the marked-for-deletion/release anchor/entry is unlocked is recycled).
+    const auto ourAnchor = [&]() {
+        if (const auto publicKey = e.publicKey())
+            return slot->sameKey(publicKey);
+        return true; // cannot check
+    };
+    assert(ourAnchor());
+
     // For collapsed disk hits: e.swap_file_sz and slot->basics.swap_file_sz
     // may still be zero and basics.swap_file_sz may grow.
     assert(slot->basics.swap_file_sz >= e.swap_file_sz);