From 49545ab897f0acc57b58da129280bee92139e752 Mon Sep 17 00:00:00 2001 From: "hyeeun.jun@samsung.com" Date: Wed, 29 Jan 2020 17:10:17 +0900 Subject: [PATCH] Fix Deadlock Issue On AppFuseBridge There are two locks used by AppFuseBridge. First is it's object lock, and the second is a mutex lock in app fuse library. There are two oppsite routines to get those locks. (Thread A) Got Java lock -> Try to get Native lock (Thread B) Got Native lock -> Try to get Java lock Bug : https://issuetracker.google.com/issues/145707568 Signed-off-by: hyeeun.jun@samsung.com The order must be followed to obtain two locks. If not, the dead lock will be caused. Therefore we change the routine to get the mutex lock first, and the object lock later. --- libappfuse/FuseBridgeLoop.cc | 14 ++++++++++++-- libappfuse/include/libappfuse/FuseBridgeLoop.h | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/libappfuse/FuseBridgeLoop.cc b/libappfuse/FuseBridgeLoop.cc index f71d0c341..22f381c62 100644 --- a/libappfuse/FuseBridgeLoop.cc +++ b/libappfuse/FuseBridgeLoop.cc @@ -311,6 +311,8 @@ class BridgeEpollController : private EpollController { } }; +std::recursive_mutex FuseBridgeLoop::mutex_; + FuseBridgeLoop::FuseBridgeLoop() : opened_(true) { base::unique_fd epoll_fd(epoll_create1(EPOLL_CLOEXEC)); if (epoll_fd.get() == -1) { @@ -328,7 +330,7 @@ bool FuseBridgeLoop::AddBridge(int mount_id, base::unique_fd dev_fd, base::uniqu std::unique_ptr bridge( new FuseBridgeEntry(mount_id, std::move(dev_fd), std::move(proxy_fd))); - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_); if (!opened_) { LOG(ERROR) << "Tried to add a mount to a closed bridge"; return false; @@ -372,7 +374,7 @@ void FuseBridgeLoop::Start(FuseBridgeLoopCallback* callback) { const bool wait_result = epoll_controller_->Wait(bridges_.size(), &entries); LOG(VERBOSE) << "Receive epoll events"; { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_); if (!(wait_result && ProcessEventLocked(entries, callback))) { for (auto it = bridges_.begin(); it != bridges_.end();) { callback->OnClosed(it->second->mount_id()); @@ -385,5 +387,13 @@ void FuseBridgeLoop::Start(FuseBridgeLoopCallback* callback) { } } +void FuseBridgeLoop::Lock() { + mutex_.lock(); +} + +void FuseBridgeLoop::Unlock() { + mutex_.unlock(); +} + } // namespace fuse } // namespace android diff --git a/libappfuse/include/libappfuse/FuseBridgeLoop.h b/libappfuse/include/libappfuse/FuseBridgeLoop.h index 6bfda9819..d5fc28fa4 100644 --- a/libappfuse/include/libappfuse/FuseBridgeLoop.h +++ b/libappfuse/include/libappfuse/FuseBridgeLoop.h @@ -50,6 +50,10 @@ class FuseBridgeLoop final { // thread from one which invokes |Start|. bool AddBridge(int mount_id, base::unique_fd dev_fd, base::unique_fd proxy_fd); + static void Lock(); + + static void Unlock(); + private: bool ProcessEventLocked(const std::unordered_set& entries, FuseBridgeLoopCallback* callback); @@ -60,7 +64,7 @@ class FuseBridgeLoop final { std::map> bridges_; // Lock for multi-threading. - std::mutex mutex_; + static std::recursive_mutex mutex_; bool opened_;