github-actions[bot] commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-4010127793
## Code Review Summary
**PR #60533: [fix](filecache) avoid SIGSEGV in background LRU update when
clear cache**
### Overview
This PR fixes a use-after-free / dangling pointer crash in the background
LRU update thread. The root cause: `update_block_lru()` previously dereferenced
`block->cell` — a raw pointer back into the `_files` map — which becomes
dangling when `clear_file_cache_directly()` destroys all cells.
### Review Checklist
**Correctness**: PASS
The fix replaces direct `block->cell` dereference with a safe `get_cell()`
lookup into `_files` under the cache lock. If the cell was removed,
`get_cell()` returns `nullptr` and the function returns early. The identity
check (`cell->file_block.get() != block.get()`) correctly guards against
ABA-style reuse of the same hash+offset slot by a different block.
**Thread Safety**: PASS
`update_block_lru()` is called only from
`run_background_block_lru_update()`, which holds `_mutex` via
`SCOPED_CACHE_LOCK`. Both cache clearing paths (`clear_file_cache_async` and
`clear_file_cache_directly`) also hold `_mutex`. The `get_cell()` lookup is
therefore safe under the same lock.
**Initialization**: PASS
`FileBlockCell* cell` was previously uninitialized (`FileBlockCell* cell;`)
— reading it before assignment was undefined behavior. The `{nullptr}`
initializer is the correct fix.
**Behavioral Change**: Minor, acceptable
`cell->update_atime()` is now called even when `cell->queue_iterator` is not
set (but only after confirming the cell exists and matches). Previously it was
inside the `if (cell)` block but also conditional on cell existing. This is a
benign change — updating atime even without a queue iterator is reasonable.
**Test Coverage**: PASS
The new test covers both the null-block path and the stale/corrupted cell
pointer path. The test directly accesses `mgr._mutex` (via friend
relationship), which is acceptable for this kind of internal unit test.
**No issues found.** The fix is well-targeted and correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
