(doris) branch master updated: [Bugfix] Fix nullptr cache object because init file cache concurrently (#35722)

[dataroaring]

This is an automated email from the ASF dual-hosted git repository.

dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new e71e4bfdb45 [Bugfix] Fix nullptr cache object because init file cache 
concurrently (#35722)
e71e4bfdb45 is described below

commit e71e4bfdb45c129bfeeba282cf38a0d3dd5eec96
Author: Lightman <31928846+lchangli...@users.noreply.github.com>
AuthorDate: Tue Jun 4 22:50:32 2024 +0800

    [Bugfix] Fix nullptr cache object because init file cache concurrently 
(#35722)
    
    The file caches are inited in
    be/src/runtime/exec_env_init.cpp:init_file_cache_factory concurrently.
    The field `_path_to_cache` and `_caches` is not protected by lock. So BE
    may coredump because the cache object is nullptr.
---
 be/src/io/cache/block_file_cache_factory.cpp | 10 +++++++---
 be/src/io/cache/block_file_cache_factory.h   |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/be/src/io/cache/block_file_cache_factory.cpp 
b/be/src/io/cache/block_file_cache_factory.cpp
index 93029871e61..a6df98c686d 100644
--- a/be/src/io/cache/block_file_cache_factory.cpp
+++ b/be/src/io/cache/block_file_cache_factory.cpp
@@ -92,16 +92,20 @@ Status FileCacheFactory::create_file_cache(const 
std::string& cache_base_path,
     }
     auto cache = std::make_unique<BlockFileCache>(cache_base_path, 
file_cache_settings);
     RETURN_IF_ERROR(cache->initialize());
-    _path_to_cache[cache_base_path] = cache.get();
-    _caches.push_back(std::move(cache));
+    {
+        std::lock_guard lock(_mtx);
+        _path_to_cache[cache_base_path] = cache.get();
+        _caches.push_back(std::move(cache));
+        _capacity += file_cache_settings.capacity;
+    }
     LOG(INFO) << "[FileCache] path: " << cache_base_path
               << " total_size: " << file_cache_settings.capacity
               << " disk_total_size: " << disk_capacity;
-    _capacity += file_cache_settings.capacity;
     return Status::OK();
 }
 
 BlockFileCache* FileCacheFactory::get_by_path(const UInt128Wrapper& key) {
+    // dont need lock mutex because _caches is immutable after 
create_file_cache
     return _caches[KeyHash()(key) % _caches.size()].get();
 }
 
diff --git a/be/src/io/cache/block_file_cache_factory.h 
b/be/src/io/cache/block_file_cache_factory.h
index 2d0dc27a036..696dae6fdc5 100644
--- a/be/src/io/cache/block_file_cache_factory.h
+++ b/be/src/io/cache/block_file_cache_factory.h
@@ -75,6 +75,7 @@ public:
     FileCacheFactory(const FileCacheFactory&) = delete;
 
 private:
+    std::mutex _mtx;
     std::vector<std::unique_ptr<BlockFileCache>> _caches;
     std::unordered_map<std::string, BlockFileCache*> _path_to_cache;
     size_t _capacity = 0;


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org