[fix](core)be core when BeConfDataDirReader::get_data_dir_by_file_path #59204
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
koarz
changed the title
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run performance |
TPC-H: Total hot run time: 35023 ms |
TPC-DS: Total hot run time: 179761 ms |
ClickBench: Total hot run time: 27.22 s |
koarz
changed the title
gavinchou
previously approved these changes
Dec 24, 2025
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
Author
|
run buildall |
github-actions
bot
removed
the
approved
TPC-H: Total hot run time: 34965 ms |
TPC-DS: Total hot run time: 179596 ms |
ClickBench: Total hot run time: 27.01 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run nonConcurrent |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
gavinchou
approved these changes
Dec 30, 2025
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions bot
pushed a commit
that referenced
this pull request
Dec 30, 2025
#59204) During the execution of `init_file_cache_factory`, the following call path is triggered: ```txt init_file_cache_factory -> FileCacheFactory::create_file_cache -> cache->initialize() -> initialize_unlocked -> _storage->init(this) -> FSFileCacheStorage::init() ``` (At this point, a thread named `_cache_background_load_thread` is created, and the remaining operations run within this thread) `-> upgrade_cache_dir_if_necessary -> read_file_cache_version -> FileSystem::open_file -> open_file_impl -> LocalFileReader::LocalFileReader -> BeConfDataDirReader::get_data_dir_by_file_path` After `FSFileCacheStorage::init` completes (spawning the `_cache_background_load_thread`), `ExecEnv::_init` continues to execute `doris::io::BeConfDataDirReader::init_be_conf_data_dir`. This function performs push operations on `be_config_data_dir_list`. Simultaneously, `BeConfDataDirReader::get_data_dir_by_file_path` (running in the background thread) iterates over this same `be_config_data_dir_list`. This leads to a race condition: if `doris::io::BeConfDataDirReader::init_be_conf_data_dir` is inserting data while the vector is being read, two issues arise: 1. Modifying `be_config_data_dir_list` while iterating over it via a range-based for loop results in **Undefined Behavior (UB)**. 2. If `be_config_data_dir_list` triggers a reallocation (expansion) during the insertion, concurrent read operations on its elements will access dangling references, triggering a **heap-use-after-free** error. Since `init_be_conf_data_dir` depends on `cache_paths` derived from `init_file_cache_factory`, we must carefully manage the synchronization sequence to prevent these errors.
github-actions bot
pushed a commit
that referenced
this pull request
Dec 30, 2025
#59204) During the execution of `init_file_cache_factory`, the following call path is triggered: ```txt init_file_cache_factory -> FileCacheFactory::create_file_cache -> cache->initialize() -> initialize_unlocked -> _storage->init(this) -> FSFileCacheStorage::init() ``` (At this point, a thread named `_cache_background_load_thread` is created, and the remaining operations run within this thread) `-> upgrade_cache_dir_if_necessary -> read_file_cache_version -> FileSystem::open_file -> open_file_impl -> LocalFileReader::LocalFileReader -> BeConfDataDirReader::get_data_dir_by_file_path` After `FSFileCacheStorage::init` completes (spawning the `_cache_background_load_thread`), `ExecEnv::_init` continues to execute `doris::io::BeConfDataDirReader::init_be_conf_data_dir`. This function performs push operations on `be_config_data_dir_list`. Simultaneously, `BeConfDataDirReader::get_data_dir_by_file_path` (running in the background thread) iterates over this same `be_config_data_dir_list`. This leads to a race condition: if `doris::io::BeConfDataDirReader::init_be_conf_data_dir` is inserting data while the vector is being read, two issues arise: 1. Modifying `be_config_data_dir_list` while iterating over it via a range-based for loop results in **Undefined Behavior (UB)**. 2. If `be_config_data_dir_list` triggers a reallocation (expansion) during the insertion, concurrent read operations on its elements will access dangling references, triggering a **heap-use-after-free** error. Since `init_be_conf_data_dir` depends on `cache_paths` derived from `init_file_cache_factory`, we must carefully manage the synchronization sequence to prevent these errors.
yiguolei
pushed a commit
that referenced
this pull request
Dec 30, 2025
…_by_file_path #59204 (#59472) Cherry-picked from #59204 Co-authored-by: koarz <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the execution of
init_file_cache_factory, the following call path is triggered:(At this point, a thread named
_cache_background_load_threadis created, and the remaining operations run within this thread)-> upgrade_cache_dir_if_necessary -> read_file_cache_version -> FileSystem::open_file -> open_file_impl -> LocalFileReader::LocalFileReader -> BeConfDataDirReader::get_data_dir_by_file_pathAfter
FSFileCacheStorage::initcompletes (spawning the_cache_background_load_thread),ExecEnv::_initcontinues to executedoris::io::BeConfDataDirReader::init_be_conf_data_dir. This function performs push operations onbe_config_data_dir_list.Simultaneously,
BeConfDataDirReader::get_data_dir_by_file_path(running in the background thread) iterates over this samebe_config_data_dir_list. This leads to a race condition: ifdoris::io::BeConfDataDirReader::init_be_conf_data_diris inserting data while the vector is being read, two issues arise:be_config_data_dir_listwhile iterating over it via a range-based for loop results in Undefined Behavior (UB).be_config_data_dir_listtriggers a reallocation (expansion) during the insertion, concurrent read operations on its elements will access dangling references, triggering a heap-use-after-free error.Since
init_be_conf_data_dirdepends oncache_pathsderived frominit_file_cache_factory, we must carefully manage the synchronization sequence to prevent these errors.