From 10e97a7e985bbf824b7677a0bdfadd71f18b8965 Mon Sep 17 00:00:00 2001
From: Lorenzo Blasa <lblasa@fb.com>
Date: Fri, 17 Sep 2021 07:33:03 -0700
Subject: [PATCH] Fixes an error with the handled flag for certificate exchange

Summary:
This change fixes a bug with the handled flag during the certificate exchange process.

Explanation:
handled was passed by reference as &handled
Once the function goes out of scope then the reference, well, it just becomes invalid (undefined behaviour)
In some cases, it appears as 'handled' because the reference is invalid and it happens to be 'true'.

Changelog: Fixed an issue where clients would randomly not connect to Flipper. Please update FlipperKit to 0.110.0 to apply the fix: https://fbflipper.com/docs/getting-started/react-native#using-the-latest-flipper-sdk

Reviewed By: mweststrate

Differential Revision: D31017592

fbshipit-source-id: c087a769fa23de1acfd3c198b4db4d6ccdb2be90
---
 xplat/Flipper/FlipperConnectionManagerImpl.cpp | 11 ++++++-----
 xplat/Flipper/FlipperConnectionManagerImpl.h   |  2 ++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp
index 55594368f..6d71c46d9 100644
--- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp
+++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp
@@ -366,12 +366,13 @@ void FlipperConnectionManagerImpl::requestSignedCertFromFlipper() {
       "destination",
       contextStore_->getCertificateDirectoryPath().c_str())("medium", medium);
   auto gettingCert = flipperState_->start("Getting cert from desktop");
-  bool handled = false;
 
-  flipperEventBase_->add([this, &handled, message, gettingCert]() {
+  certificateExchangeCompleted_ = false;
+
+  flipperEventBase_->add([this, message, gettingCert]() {
     client_->sendExpectResponse(
         folly::toJson(message),
-        [this, &handled, message, gettingCert](
+        [this, message, gettingCert](
             const std::string& response, bool isError) {
           /**
             Need to keep track of whether the response has been handled.
@@ -382,9 +383,9 @@ void FlipperConnectionManagerImpl::requestSignedCertFromFlipper() {
             interrupted because we are effectively still handing the response
             read. So, if already handled, ignore and return;
           */
-          if (handled)
+          if (certificateExchangeCompleted_)
             return;
-          handled = true;
+          certificateExchangeCompleted_ = true;
           if (isError) {
             if (response.compare("not implemented")) {
               auto error =
diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.h b/xplat/Flipper/FlipperConnectionManagerImpl.h
index 3e0cd240b..78807a6a7 100644
--- a/xplat/Flipper/FlipperConnectionManagerImpl.h
+++ b/xplat/Flipper/FlipperConnectionManagerImpl.h
@@ -68,6 +68,8 @@ class FlipperConnectionManagerImpl : public FlipperConnectionManager {
   std::unique_ptr<FlipperSocket> client_;
 
   bool connectionIsTrusted_;
+  bool certificateExchangeCompleted_ = false;
+
   int failedConnectionAttempts_ = 0;
   std::shared_ptr<ConnectionContextStore> contextStore_;
   std::shared_ptr<FlipperConnectionManagerWrapper> implWrapper_;