From f28d0c2020d0314e66aed240f379f2a795b6c940 Mon Sep 17 00:00:00 2001 From: Bo Wang Date: Thu, 4 Aug 2022 13:52:11 -0700 Subject: [PATCH] Fix data race reported on SetIsInSecondaryCache in LRUCache (#10472) Summary: Currently, `SetIsInSecondaryCache` is after `Promote`. After `Promote`, a handle can be accessed and its flags can be set. This causes data race. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10472 Test Plan: unit tests stress tests Reviewed By: pdillinger Differential Revision: D38403991 Pulled By: gitbw95 fbshipit-source-id: 0aaa2d2edeaf5bc799fcce605648fe49eb7119c2 --- HISTORY.md | 1 + cache/lru_cache.cc | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 8be7cbf97..c524180f4 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -20,6 +20,7 @@ * Fix data race bug in hash linked list memtable. With this bug, read request might temporarily miss an old record in the memtable in a race condition to the hash bucket. * Fix a bug that `best_efforts_recovery` may fail to open the db with mmap read. * Fixed a bug where blobs read during compaction would pollute the cache. +* Fixed a data race in LRUCache when used with a secondary_cache. ### Behavior Change * Added checksum handshake during the copying of decompressed WAL fragment. This together with #9875, #10037, #10212, #10114 and #10319 provides end-to-end integrity protection for write batch during recovery. diff --git a/cache/lru_cache.cc b/cache/lru_cache.cc index bed5e5c9f..0d7f946c4 100644 --- a/cache/lru_cache.cc +++ b/cache/lru_cache.cc @@ -460,10 +460,10 @@ Cache::Handle* LRUCacheShard::Lookup( e->sec_handle = secondary_handle.release(); e->total_charge = 0; e->Ref(); + e->SetIsInSecondaryCache(is_in_sec_cache); if (wait) { Promote(e); - e->SetIsInSecondaryCache(is_in_sec_cache); if (!e->value) { // The secondary cache returned a handle, but the lookup failed. e->Unref(); @@ -477,7 +477,6 @@ Cache::Handle* LRUCacheShard::Lookup( // If wait is false, we always return a handle and let the caller // release the handle after checking for success or failure. e->SetIncomplete(true); - e->SetIsInSecondaryCache(is_in_sec_cache); // This may be slightly inaccurate, if the lookup eventually fails. // But the probability is very low. PERF_COUNTER_ADD(secondary_cache_hit_count, 1);