From 079a9cb3e610ed7ab26f7bbcd8219a0111e851d3 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Mon, 18 Oct 2021 11:23:49 -0400 Subject: [PATCH] rgw: fix lock scope in ObjectCache::get() 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 (cherry picked from commit 1aee987ec8f817541aab9cd21480b5351df38f69) --- src/rgw/rgw_cache.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rgw/rgw_cache.cc b/src/rgw/rgw_cache.cc index e82c142eb0240..346b86193c99a 100644 --- a/src/rgw/rgw_cache.cc +++ b/src/rgw/rgw_cache.cc @@ -13,6 +13,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; } @@ -29,7 +30,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()) { @@ -50,7 +51,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()) { -- 2.39.5