Commit d29d641
Synchronize Region::isBeingReclaimed() - read flags_ under lock_
Summary:
navy::Region::flags_ is a non-atomic uint16_t guarded by the per-Region lock_
(a non-recursive TimedMutex). All mutators write it under lock_, and the public
status getters take lock_ themselves (getActiveWriters(), getActiveInMemReaders(),
hasBuffer()); the *Locked-suffixed helpers (isFlushedLocked(), isCleanedupLocked())
assume the caller already holds it. isBeingReclaimed() was the lone accessor
reading flags_ with no lock and no Locked suffix.
This is a genuine data race. On the lookup/remove path,
BlockCache::remove -> lookup -> RegionManager::read calls
region.isBeingReclaimed() (a best-effort readDuringReclaimCount_ stat) on a Navy
worker thread, concurrently with the flush path
RegionManager::doFlush -> Region::flushBuffer, which writes flags_ |= kFlushed
under lock_ on a Navy fiber. ThreadSanitizer reports the unsynchronized read of
the lock_-guarded flags_ word: write at Region.cpp:116 vs read at Region.h:195,
on a live Region (no use-after-free; both threads operate while the cache is up).
Fix: isBeingReclaimed() acquires lock_ before reading flags_, matching its
sibling getters. The sole caller (RegionManager::read) does not hold lock_ at the
call site, so acquiring it here is deadlock-safe (lock_ is non-recursive).
Reviewed By: pbhandar2
Differential Revision: D110222510
fbshipit-source-id: ebeaae18dc2b2b3be8d5126241e6f84e405a8d7f1 parent ee63cea commit d29d641
1 file changed
Lines changed: 4 additions & 1 deletion
File tree
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
192 | 192 | | |
193 | 193 | | |
194 | 194 | | |
195 | | - | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
196 | 199 | | |
197 | 200 | | |
198 | 201 | | |
| |||
0 commit comments