From e51db47febd5ac898532a3c8fd6127165f401b7b Mon Sep 17 00:00:00 2001
From: anand76 <anand76@devvm4702.ftw0.facebook.com>
Date: Mon, 30 Aug 2021 19:09:43 -0700
Subject: [PATCH] Fix a race in LRUCacheShard::Promote (#8717)

Summary:
In ```LRUCacheShard::Promote```, a reference is released outside the LRU mutex. Fix the race condition.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8717

Reviewed By: zhichao-cao

Differential Revision: D30649206

Pulled By: anand1976

fbshipit-source-id: 09c0af05b2294a7fe2c02876a61b0bad6e3ada61
---
 HISTORY.md         |  4 ++++
 cache/lru_cache.cc | 11 +++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/HISTORY.md b/HISTORY.md
index c29f8d51e..8fb5ffa62 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,4 +1,8 @@
 # Rocksdb Change Log
+## 6.23.4 (2021-08-31)
+### Bug Fixes
+* Fix a race in item ref counting in LRUCache when promoting an item from the SecondaryCache.
+
 ## 6.23.3 (2021-08-09)
 ### Bug Fixes
 * Removed a call to `RenameFile()` on a non-existent info log file ("LOG") when opening a new DB. Such a call was guaranteed to fail though did not impact applications since we swallowed the error. Now we also stopped swallowing errors in renaming "LOG" file.
diff --git a/cache/lru_cache.cc b/cache/lru_cache.cc
index db60828c0..e43e8e5a0 100644
--- a/cache/lru_cache.cc
+++ b/cache/lru_cache.cc
@@ -358,7 +358,10 @@ Status LRUCacheShard::InsertItem(LRUHandle* e, Cache::Handle** handle,
       if (handle == nullptr) {
         LRU_Insert(e);
       } else {
-        e->Ref();
+        // If caller already holds a ref, no need to take one here
+        if (!e->HasRefs()) {
+          e->Ref();
+        }
         *handle = reinterpret_cast<Cache::Handle*>(e);
       }
     }
@@ -396,11 +399,7 @@ void LRUCacheShard::Promote(LRUHandle* e) {
   if (e->value) {
     Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(e);
     Status s = InsertItem(e, &handle, /*free_handle_on_fail=*/false);
-    if (s.ok()) {
-      // InsertItem would have taken a reference on the item, so decrement it
-      // here as we expect the caller to already hold a reference
-      e->Unref();
-    } else {
+    if (!s.ok()) {
       // Item is in memory, but not accounted against the cache capacity.
       // When the handle is released, the item should get deleted
       assert(!e->InCache());