andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix a race in LRUCacheShard::Promote (#8717)

Browse Source 
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
This commit is contained in:
anand76 2021-08-30 19:09:43 -07:00
parent c3034fce32
commit e51db47feb
2 changed files with 9 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
HISTORY.md
View File

@ -1,4 +1,8 @@
# Rocksdb Change Log # 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) ## 6.23.3 (2021-08-09)
### Bug Fixes ### 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. * 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.

11
cache/lru_cache.cc vendored
View File

@ -358,7 +358,10 @@ Status LRUCacheShard::InsertItem(LRUHandle* e, Cache::Handle** handle,
if (handle == nullptr) { if (handle == nullptr) {
LRU_Insert(e); LRU_Insert(e);
} else { } 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); *handle = reinterpret_cast<Cache::Handle*>(e);
} }
} }
@ -396,11 +399,7 @@ void LRUCacheShard::Promote(LRUHandle* e) {
if (e->value) { if (e->value) {
Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(e); Cache::Handle* handle = reinterpret_cast<Cache::Handle*>(e);
Status s = InsertItem(e, &handle, /*free_handle_on_fail=*/false); Status s = InsertItem(e, &handle, /*free_handle_on_fail=*/false);
if (s.ok()) { 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 {
// Item is in memory, but not accounted against the cache capacity. // Item is in memory, but not accounted against the cache capacity.
// When the handle is released, the item should get deleted // When the handle is released, the item should get deleted
assert(!e->InCache()); assert(!e->InCache());