From dbd52d1cfe2237fdb87c3d8a3716f31b4f300e59 Mon Sep 17 00:00:00 2001
From: Aleksandr Sasha Sergeev <alxr@fb.com>
Date: Mon, 7 Jun 2021 11:19:44 -0700
Subject: [PATCH] Fix deadlock in FlipperClient

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
---
 xplat/Flipper/FlipperClient.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xplat/Flipper/FlipperClient.cpp b/xplat/Flipper/FlipperClient.cpp
index 44659fc40..8a926b3b2 100644
--- a/xplat/Flipper/FlipperClient.cpp
+++ b/xplat/Flipper/FlipperClient.cpp
@@ -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;