From 4ac755370d69085ddde0a013fc455609468a8bb6 Mon Sep 17 00:00:00 2001
From: Lorenzo Blasa <lblasa@meta.com>
Date: Thu, 29 Jun 2023 12:40:09 -0700
Subject: [PATCH] Move socket clean inside operation queue

Summary: Set delegate and close inside the operation's queue as to make it safer i.e. all socket related operations are done inside the queue.

Reviewed By: ivanmisuno

Differential Revision: D47124235

fbshipit-source-id: 48b53db1cd47d017a26186a156046ba68fe358b7
---
 iOS/FlipperKit/FlipperPlatformWebSocket.mm    | 23 +++++++++++--------
 .../Flipper/FlipperConnectionManagerImpl.cpp  |  4 ----
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/iOS/FlipperKit/FlipperPlatformWebSocket.mm b/iOS/FlipperKit/FlipperPlatformWebSocket.mm
index d40b13299..bcd8eaa65 100644
--- a/iOS/FlipperKit/FlipperPlatformWebSocket.mm
+++ b/iOS/FlipperKit/FlipperPlatformWebSocket.mm
@@ -171,16 +171,21 @@ static constexpr int connectionKeepaliveSeconds = 10;
   }
   _keepAlive = nil;
 
-  if (_socket) {
-    // Clear the socket delegate before close. Ensures that we won't get
-    // any messages after the disconnect takes place.
-    [_socket setDelegate:nil];
-    [_socket close];
-    _socket = nil;
-  };
+  __weak auto weakSelf = self;
+  NSBlockOperation* disconnectOperation =
+      [NSBlockOperation blockOperationWithBlock:^{
+        __strong auto strongSelf = weakSelf;
+        // Clear the socket delegate before close. Ensures that we won't get
+        // any messages after the disconnect takes place.
+        if (strongSelf->_socket) {
+          [strongSelf->_socket setDelegate:nil];
+          [strongSelf->_socket close];
 
-  [_dispatchQueue cancelAllOperations];
-  [_dispatchQueue waitUntilAllOperationsAreFinished];
+          strongSelf->_socket = nil;
+        }
+      }];
+
+  [_dispatchQueue addOperations:@[ disconnectOperation ] waitUntilFinished:YES];
 }
 
 - (void)send:(NSString*)message
diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp
index 53adf63f7..878f89604 100644
--- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp
+++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp
@@ -107,13 +107,11 @@ void FlipperConnectionManagerImpl::handleSocketEvent(SocketEvent event) {
         }
         break;
       case SocketEvent::SSL_ERROR:
-        log("[conn] handleSocketEvent(SSL_ERROR)");
         failedConnectionAttempts_++;
         reconnect();
         break;
       case SocketEvent::CLOSE:
       case SocketEvent::ERROR:
-        log("[conn] handleSocketEvent(CLOSE_ERROR)");
         if (!isConnected_) {
           reconnect();
           return;
@@ -182,7 +180,6 @@ void FlipperConnectionManagerImpl::startSync() {
 }
 
 void FlipperConnectionManagerImpl::connectAndExchangeCertificate() {
-  log("[conn] connectAndExchangeCertificate()");
   auto port = insecurePort;
   auto endpoint = FlipperConnectionEndpoint(deviceData_.host, port, false);
 
@@ -265,7 +262,6 @@ void FlipperConnectionManagerImpl::reconnect() {
     log("Not started");
     return;
   }
-  log("[conn] reconnect()");
   scheduler_->scheduleAfter(
       [this]() { startSync(); }, RECONNECT_INTERVAL_SECONDS * 1000.0f);
 }