]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw: fix lock scope in ObjectCache::get() 43581/head
authorCasey Bodley <cbodley@redhat.com>
Mon, 18 Oct 2021 15:23:49 +0000 (11:23 -0400)
committerCasey Bodley <cbodley@redhat.com>
Mon, 18 Oct 2021 16:04:26 +0000 (12:04 -0400)
in the touch_lru() case, we promote the shared_lock to a unique_lock.
but because the unique_lock is in a nested scope, the lock drops with
its scope and we continue accessing the map without any protection

this moves the unique_lock up to function scope, where it's
constructed as unlocked with std::defer_lock. after promotion, this
lock will be held until the function returns

Fixes: https://tracker.ceph.com/issues/52800
Signed-off-by: Casey Bodley <cbodley@redhat.com>
src/rgw/rgw_cache.cc

index 3a325c81c2c2a4d4858cf2f55e1b88b9ecc7a27f..c38f19df020db2fb6b58cd6da9879a93f43cf55a 100644 (file)
@@ -14,6 +14,7 @@ int ObjectCache::get(const DoutPrefixProvider *dpp, const string& name, ObjectCa
 {
 
   std::shared_lock rl{lock};
+  std::unique_lock wl{lock, std::defer_lock}; // may be promoted to write lock
   if (!enabled) {
     return -ENOENT;
   }
@@ -30,7 +31,7 @@ int ObjectCache::get(const DoutPrefixProvider *dpp, const string& name, ObjectCa
        (ceph::coarse_mono_clock::now() - iter->second.info.time_added) > expiry) {
     ldpp_dout(dpp, 10) << "cache get: name=" << name << " : expiry miss" << dendl;
     rl.unlock();
-    std::unique_lock wl{lock};  // write lock for insertion
+    wl.lock(); // write lock for expiration
     // check that wasn't already removed by other thread
     iter = cache_map.find(name);
     if (iter != cache_map.end()) {
@@ -51,7 +52,7 @@ int ObjectCache::get(const DoutPrefixProvider *dpp, const string& name, ObjectCa
     ldpp_dout(dpp, 20) << "cache get: touching lru, lru_counter=" << lru_counter
                    << " promotion_ts=" << entry->lru_promotion_ts << dendl;
     rl.unlock();
-    std::unique_lock wl{lock};  // write lock for insertion
+    wl.lock(); // write lock for touch_lru()
     /* need to redo this because entry might have dropped off the cache */
     iter = cache_map.find(name);
     if (iter == cache_map.end()) {