Skip to content

Commit

Permalink
Fix Deadlock Issue On AppFuseBridge
Browse files Browse the repository at this point in the history
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

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.

Signed-off-by: hyeeun.jun@samsung.com <hyeeun.jun@samsung.com>
Bug: https://issuetracker.google.com/issues/145707568
Bug: 157535024
Test: atest --test-mapping apex/blobstore

Change-Id: I0ab002da9a0b7ca2f518d50ab477a080cabe3788

Former-commit-id: 612fc47090e4bcff953806c855fe21f5c594aa7e
  • Loading branch information
hyeeun-jun authored and narayank committed Jun 17, 2020
1 parent d38fd30 commit e3690b6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
14 changes: 12 additions & 2 deletions libappfuse/FuseBridgeLoop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -328,7 +330,7 @@ bool FuseBridgeLoop::AddBridge(int mount_id, base::unique_fd dev_fd, base::uniqu

std::unique_ptr<FuseBridgeEntry> bridge(
new FuseBridgeEntry(mount_id, std::move(dev_fd), std::move(proxy_fd)));
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);
if (!opened_) {
LOG(ERROR) << "Tried to add a mount to a closed bridge";
return false;
Expand Down Expand Up @@ -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<std::mutex> lock(mutex_);
std::lock_guard<std::recursive_mutex> lock(mutex_);
if (!(wait_result && ProcessEventLocked(entries, callback))) {
for (auto it = bridges_.begin(); it != bridges_.end();) {
callback->OnClosed(it->second->mount_id());
Expand All @@ -385,5 +387,13 @@ void FuseBridgeLoop::Start(FuseBridgeLoopCallback* callback) {
}
}

void FuseBridgeLoop::Lock() {
mutex_.lock();
}

void FuseBridgeLoop::Unlock() {
mutex_.unlock();
}

} // namespace fuse
} // namespace android
6 changes: 5 additions & 1 deletion libappfuse/include/libappfuse/FuseBridgeLoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<FuseBridgeEntry*>& entries,
FuseBridgeLoopCallback* callback);
Expand All @@ -60,7 +64,7 @@ class FuseBridgeLoop final {
std::map<int, std::unique_ptr<FuseBridgeEntry>> bridges_;

// Lock for multi-threading.
std::mutex mutex_;
static std::recursive_mutex mutex_;

bool opened_;

Expand Down

0 comments on commit e3690b6

Please sign in to comment.