From: Matan Breizman <mbreizma@redhat.com>
Date: Tue, 24 Dec 2024 11:22:58 +0000 (+0000)
Subject: crimson/common/shared_lru: invalidate Deleter's cache
X-Git-Tag: v20.0.0~444^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c21e0e6d1488fdf099962531a598f80b0f2d54dd;p=ceph.git

crimson/common/shared_lru: invalidate Deleter's cache

Once we destruct SharedLRU, SharedLRU::weak_refs map is destroyed.
As a weak refernce might outlive the SharedLRU itself, when destroying
the object via the custom Deleter, we try to access the already
destroyed SharedLRU instance's weak ref map.

Instead, invalidate the custom Deleter (Deleter::cache), when
destructing the SharedLRU.

Fixes: https://tracker.ceph.com/issues/66478

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
---

diff --git a/src/crimson/common/shared_lru.h b/src/crimson/common/shared_lru.h
index 92d99d332c44..c08f033e0ea7 100644
--- a/src/crimson/common/shared_lru.h
+++ b/src/crimson/common/shared_lru.h
@@ -25,11 +25,16 @@ class SharedLRU {
   SimpleLRU<K, shared_ptr_t, false> cache;
   std::map<K, std::pair<weak_ptr_t, V*>> weak_refs;
 
+  // Once all of the shared pointers are destoryed,
+  // erase the tracked object from the weak_ref map
+  // before actually destorying it
   struct Deleter {
     SharedLRU<K,V>* cache;
     const K key;
     void operator()(V* ptr) {
-      cache->_erase_weak(key);
+      if (cache) {
+        cache->_erase_weak(key);
+      }
       delete ptr;
     }
   };
@@ -42,9 +47,19 @@ public:
   {}
   ~SharedLRU() {
     cache.clear();
+
     // initially, we were assuming that no pointer obtained from SharedLRU
     // can outlive the lru itself. However, since going with the interruption
     // concept for handling shutdowns, this is no longer valid.
+    // Moreover, before clearing weak_refs, invalidate each deleter
+    // cache pointer as this SharedLRU is being destoryed.
+    for (const auto& [key, value] : weak_refs) {
+      shared_ptr_t val;
+      val = value.first.lock();
+      auto this_deleter = get_deleter<Deleter>(val);
+      this_deleter->cache = nullptr;
+    }
+
     weak_refs.clear();
   }
   /**