From 3df1e1c471aef6a54d2c5011e0839f5fb9443922 Mon Sep 17 00:00:00 2001 From: Zhijun Fu <37800433+zhijunfu@users.noreply.github.com> Date: Sat, 29 Dec 2018 03:47:31 +0800 Subject: [PATCH] Add missing lock in FreeObjects of object buffer pool (#3647) Object manager uses multi-threading for transferring objects between different nodes, the plasma client used in object_buffer_pool_ needs to be protected by lock. We have met crashes caused by missing lock in FreeObjects() interface, this PR fixes that issue. --- src/ray/object_manager/object_buffer_pool.cc | 2 ++ src/ray/object_manager/object_buffer_pool.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ray/object_manager/object_buffer_pool.cc b/src/ray/object_manager/object_buffer_pool.cc index fa312f0c7..15e9eefac 100644 --- a/src/ray/object_manager/object_buffer_pool.cc +++ b/src/ray/object_manager/object_buffer_pool.cc @@ -192,10 +192,12 @@ void ObjectBufferPool::FreeObjects(const std::vector &object_ids) { for (const auto &id : object_ids) { plasma_ids.push_back(id.to_plasma_id()); } + std::lock_guard lock(pool_mutex_); ARROW_CHECK_OK(store_client_.Delete(plasma_ids)); } std::string ObjectBufferPool::DebugString() const { + std::lock_guard lock(pool_mutex_); std::stringstream result; result << "BufferPool:"; result << "\n- get buffer state map size: " << get_buffer_state_.size(); diff --git a/src/ray/object_manager/object_buffer_pool.h b/src/ray/object_manager/object_buffer_pool.h index e4790dfe5..d39f5b265 100644 --- a/src/ray/object_manager/object_buffer_pool.h +++ b/src/ray/object_manager/object_buffer_pool.h @@ -182,7 +182,7 @@ class ObjectBufferPool { /// Mutex on public methods for thread-safe operations on /// get_buffer_state_, create_buffer_state_, and store_client_. - std::mutex pool_mutex_; + mutable std::mutex pool_mutex_; /// Determines the maximum chunk size to be transferred by a single thread. const uint64_t default_chunk_size_; /// The state of a buffer that's currently being used.