From e9c0a459dd4349ab2ee56406536a5e568aac4b29 Mon Sep 17 00:00:00 2001 From: Lorenzo Blasa Date: Tue, 7 Feb 2023 05:55:11 -0800 Subject: [PATCH] Remove connection timeout Summary: This change mainly removes the connection timeout period as it may introduce a race condition in which slow server connections get terminated potentially creating connection loops. Also, suspend and wait for all operations to complete once a socket is disconnected. Reviewed By: ivanmisuno Differential Revision: D43048252 fbshipit-source-id: 64c28a3d3d2fd4e065084d5f55a17444385c07e0 --- iOS/FlipperKit/FlipperPlatformWebSocket.mm | 36 ++++++++++++---------- iOS/FlipperKit/FlipperWebSocket.mm | 8 +---- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/iOS/FlipperKit/FlipperPlatformWebSocket.mm b/iOS/FlipperKit/FlipperPlatformWebSocket.mm index a51c5ac27..76df258f2 100644 --- a/iOS/FlipperKit/FlipperPlatformWebSocket.mm +++ b/iOS/FlipperKit/FlipperPlatformWebSocket.mm @@ -166,25 +166,26 @@ static constexpr int connectionKeepaliveSeconds = 10; } - (void)disconnect { - [_dispatchQueue cancelAllOperations]; - if ([_keepAlive isValid]) { [_keepAlive invalidate]; } _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; + }; + + [_dispatchQueue cancelAllOperations]; + [_dispatchQueue waitUntilAllOperationsAreFinished]; + // Manually trigger a 'close' event as SocketRocket close method will // not notify the delegate. SocketRocket only triggers the close event // when the connection is closed from the server. _eventHandler(facebook::flipper::SocketEvent::CLOSE); - - if (_socket) { - // Clear the socket delegate before close. Ensures that we won't get - // any messages after the disconnect takes place. - _socket.delegate = nil; - [_socket close]; - _socket = nil; - }; } - (void)send:(NSString*)message @@ -192,10 +193,12 @@ static constexpr int connectionKeepaliveSeconds = 10; __weak auto weakSelf = self; [_dispatchQueue addOperationWithBlock:^{ __strong auto strongSelf = weakSelf; - NSError* error = nil; - [strongSelf->_socket sendString:message error:&error]; - if (completionHandler) { - completionHandler(error); + if (strongSelf) { + NSError* error = nil; + [strongSelf->_socket sendString:message error:&error]; + if (completionHandler) { + completionHandler(error); + } } }]; } @@ -210,7 +213,9 @@ static constexpr int connectionKeepaliveSeconds = 10; __weak auto weakSelf = self; [_dispatchQueue addOperationWithBlock:^{ __strong auto strongSelf = weakSelf; - [strongSelf->_socket sendPing:nil error:nil]; + if (strongSelf) { + [strongSelf->_socket sendPing:nil error:nil]; + } }]; } @@ -251,7 +256,6 @@ static constexpr int connectionKeepaliveSeconds = 10; _keepAlive = nil; _eventHandler(facebook::flipper::SocketEvent::CLOSE); - _socket = nil; } - (void)_webSocketDidReceiveMessage:(id)message { diff --git a/iOS/FlipperKit/FlipperWebSocket.mm b/iOS/FlipperKit/FlipperWebSocket.mm index 3389d7791..d5dd99917 100644 --- a/iOS/FlipperKit/FlipperWebSocket.mm +++ b/iOS/FlipperKit/FlipperWebSocket.mm @@ -125,13 +125,7 @@ bool FlipperWebSocket::connect(FlipperConnectionManager* manager) { [socket_ connect]; - auto state = connected.wait_for(std::chrono::seconds(10)); - if (state == std::future_status::ready) { - return connected.get(); - } - - disconnect(); - return false; + return connected.get(); } void FlipperWebSocket::disconnect() {