diff --git a/be/src/io/cache/block_file_cache.cpp b/be/src/io/cache/block_file_cache.cpp index cd5f2d574796f6..407101449d8056 100644 --- a/be/src/io/cache/block_file_cache.cpp +++ b/be/src/io/cache/block_file_cache.cpp @@ -485,17 +485,23 @@ Status BlockFileCache::initialize_unlocked(std::lock_guard& cache_lo void BlockFileCache::update_block_lru(FileBlockSPtr block, std::lock_guard& cache_lock) { - FileBlockCell* cell = block->cell; - if (cell) { - if (cell->queue_iterator) { - auto& queue = get_queue(block->cache_type()); - queue.move_to_end(*cell->queue_iterator, cache_lock); - _lru_recorder->record_queue_event(block->cache_type(), CacheLRULogType::MOVETOBACK, - block->_key.hash, block->_key.offset, - block->_block_range.size()); - } - cell->update_atime(); + if (!block) { + return; + } + + FileBlockCell* cell = get_cell(block->get_hash_value(), block->offset(), cache_lock); + if (!cell || cell->file_block.get() != block.get()) { + return; + } + + if (cell->queue_iterator) { + auto& queue = get_queue(block->cache_type()); + queue.move_to_end(*cell->queue_iterator, cache_lock); + _lru_recorder->record_queue_event(block->cache_type(), CacheLRULogType::MOVETOBACK, + block->_key.hash, block->_key.offset, + block->_block_range.size()); } + cell->update_atime(); } void BlockFileCache::use_cell(const FileBlockCell& cell, FileBlocks* result, bool move_iter_flag, diff --git a/be/src/io/cache/file_block.h b/be/src/io/cache/file_block.h index eab74e827a800a..10263c0e273cb9 100644 --- a/be/src/io/cache/file_block.h +++ b/be/src/io/cache/file_block.h @@ -169,7 +169,7 @@ class FileBlock { size_t _downloaded_size {0}; bool _is_deleting {false}; - FileBlockCell* cell; + FileBlockCell* cell {nullptr}; }; extern std::ostream& operator<<(std::ostream& os, const FileBlock::State& value); diff --git a/be/test/io/cache/need_update_lru_blocks_test.cpp b/be/test/io/cache/need_update_lru_blocks_test.cpp index 31b6eccab25a0d..2e20cc90dda71b 100644 --- a/be/test/io/cache/need_update_lru_blocks_test.cpp +++ b/be/test/io/cache/need_update_lru_blocks_test.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include "io/cache/block_file_cache.h" @@ -108,4 +109,42 @@ TEST(NeedUpdateLRUBlocksTest, ClearIsIdempotent) { EXPECT_EQ(0u, pending.drain(4, &drained)); } +TEST(NeedUpdateLRUBlocksTest, UpdateBlockLRUIgnoresNullAndCorruptedCellPointer) { + io::FileCacheSettings settings; + settings.capacity = 1024 * 1024; + settings.query_queue_size = 1024 * 1024; + settings.query_queue_elements = 10; + settings.max_file_block_size = 1024; + settings.max_query_cache_size = 1024 * 1024; + settings.storage = "memory"; + + io::BlockFileCache mgr("memory", settings); + + { + std::lock_guard cache_lock(mgr._mutex); + FileBlockSPtr null_block; + mgr.update_block_lru(null_block, cache_lock); + } + + FileCacheKey key; + key.hash = io::BlockFileCache::hash("update_block_lru_corrupted_cell"); + key.offset = 0; + key.meta.expiration_time = 0; + key.meta.type = FileCacheType::NORMAL; + key.meta.tablet_id = 0; + + auto block = + std::make_shared(key, /*size*/ 1, /*mgr*/ &mgr, FileBlock::State::EMPTY); + EXPECT_EQ(nullptr, block->cell); + + // Simulate a corrupted/stale cell pointer. Previously update_block_lru() + // dereferenced block->cell directly and could crash. + block->cell = reinterpret_cast(0x1); + + { + std::lock_guard cache_lock(mgr._mutex); + mgr.update_block_lru(block, cache_lock); + } +} + } // namespace doris::io