jens/flipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix deadlock in FlipperClient

Browse Source 
Summary:
This diff fixes deadlock that happens on mutex [FlipperClient::mutex_](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/sonar/xplat/Flipper/FlipperClient.h?commit=9eba9f832dfb648788d4c0e6ae05712e30a59a21&lines=113).

Steps that cause a deadlock:
1. Mutex is locked at [FlipperClient.cpp:170](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/sonar/xplat/Flipper/FlipperClient.cpp?commit=750ee132616cd3c470d8d091532a853e6b44f6d6&lines=170)
2. `FlipperConnectionImpl::call` executed at [FlipperClient.cpp:237](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/sonar/xplat/Flipper/FlipperClient.cpp?commit=750ee132616cd3c470d8d091532a853e6b44f6d6&lines=237)
3. `FlipperConnectionImpl::call` eventually calls `FlipperClient::getPlugin()` (see fullstacktrace below) which is trying to lock the same [FlipperClient::mutex_](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/sonar/xplat/Flipper/FlipperClient.h?commit=9eba9f832dfb648788d4c0e6ae05712e30a59a21&lines=113) as in step 1.

Full stacktrace: P420350989. Mutex is locked in frame #43 and attempted to be locked in frame #6.
More details: T92341964

Solution: unlock the mutex before calling `FlipperConnectionImpl::call` at [FlipperClient.cpp:237](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/sonar/xplat/Flipper/FlipperClient.cpp?commit=750ee132616cd3c470d8d091532a853e6b44f6d6&lines=237)

Reviewed By: jknoxville

Differential Revision: D28918093

fbshipit-source-id: 06c7841740a70e117dc5f767f9d5852149974f7f
This commit is contained in:
Aleksandr Sasha Sergeev
2021-06-07 11:19:44 -07:00
committed by Facebook GitHub Bot
parent bc647972e1
commit dbd52d1cfe
1 changed files with 7 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
xplat/Flipper/FlipperClient.cpp
View File

@@ -167,7 +167,7 @@ void FlipperClient::onMessageReceived(
// plugin, and still use it to respond with an error if we catch an exception.
std::shared_ptr<FlipperResponder> responder = std::move(uniqueResponder);
try {
std::lock_guard<std::mutex> lock(mutex_);
std::unique_lock<std::mutex> lock(mutex_);
const auto& method = message["method"];
const auto& params = message.getDefault("params");
@@ -233,7 +233,12 @@ void FlipperClient::onMessageReceived(
"name", "ConnectionNotFound"));
return;
}
const auto& conn = connections_.at(params["api"].getString());
const auto conn = connections_.at(params["api"].getString());
// conn->call(...) may call back to FlipperClient causing a deadlock (see
// T92341964). Making sure the mutex is not locked.
lock.unlock();
conn->call(
params["method"].getString(), params.getDefault("params"), responder);
return;