Clear handlers on disconnect

Summary:
A few changes that should make our connect/disconnect more robust:

* Certificate provider handler should be set directly to and managed by the policy.
* Instantiate the socket once, synchronously on the connect method. Explicit deallocation once, synchronously on the disconnect method.
* Clear handlers on disconnect after clearing the delegate.
* Wait for the operation queue to drain before returning.

Reviewed By: passy

Differential Revision: D42664724

fbshipit-source-id: bd482acbb64a9bc9e36fb3418d4c81afa2109305
This commit is contained in:
Lorenzo Blasa
2023-01-23 03:45:59 -08:00
committed by Facebook GitHub Bot
parent b31f8c8755
commit 764e94503e
3 changed files with 33 additions and 33 deletions

View File

@@ -23,15 +23,15 @@ NS_ASSUME_NONNULL_BEGIN
/// A message handler used to dispatch messages received from the server. /// A message handler used to dispatch messages received from the server.
@property(nonatomic) facebook::flipper::SocketMessageHandler messageHandler; @property(nonatomic) facebook::flipper::SocketMessageHandler messageHandler;
/// A certificate provider used to obtain the client certificate used for
/// authentication.
@property(nonatomic)
facebook::flipper::SocketCertificateProvider certificateProvider;
/// Initializes an instance of FliperWebSocketTransport with an endpoint URL. /// Initializes an instance of FliperWebSocketTransport with an endpoint URL.
/// @param url Endpoint URL used to establish the connection. /// @param url Endpoint URL used to establish the connection.
- (instancetype)initWithURL:(NSURL* _Nonnull)url; - (instancetype)initWithURL:(NSURL* _Nonnull)url;
/// A certificate provider used to obtain the client certificate used for
/// authentication.
- (void)setCertificateProvider:
(facebook::flipper::SocketCertificateProvider)certificateProvider;
/// Connect to the endpoint. /// Connect to the endpoint.
- (void)connect; - (void)connect;

View File

@@ -144,6 +144,10 @@ static constexpr int connectionKeepaliveSeconds = 10;
return; return;
} }
_socket = [[SRWebSocket alloc] initWithURL:self->_url
securityPolicy:self->_policy];
_socket.delegate = self;
__weak auto weakSelf = self; __weak auto weakSelf = self;
[_dispatchQueue addOperationWithBlock:^{ [_dispatchQueue addOperationWithBlock:^{
__strong auto strongSelf = weakSelf; __strong auto strongSelf = weakSelf;
@@ -158,33 +162,32 @@ static constexpr int connectionKeepaliveSeconds = 10;
return; return;
} }
strongSelf->_socket = [[SRWebSocket alloc] initWithURL:self->_url
securityPolicy:self->_policy];
strongSelf->_socket.delegate = self;
[strongSelf->_socket open]; [strongSelf->_socket open];
}]; }];
} }
- (void)disconnect { - (void)disconnect {
_socket.delegate = nil;
// 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. Furthermore,
// we are clearing the delegate above.
_eventHandler(facebook::flipper::SocketEvent::CLOSE);
_eventHandler = [](facebook::flipper::SocketEvent) {};
_messageHandler = ^(const std::string&) {
};
_policy.certificateProvider = [](char* _Nonnull, size_t) { return ""; };
[_dispatchQueue cancelAllOperations]; [_dispatchQueue cancelAllOperations];
[_dispatchQueue waitUntilAllOperationsAreFinished];
if ([_keepAlive isValid]) { if ([_keepAlive isValid]) {
[_keepAlive invalidate]; [_keepAlive invalidate];
} }
_keepAlive = nil; _keepAlive = nil;
// 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; _socket = nil;
};
} }
- (void)send:(NSString*)message - (void)send:(NSString*)message
@@ -202,7 +205,6 @@ static constexpr int connectionKeepaliveSeconds = 10;
- (void)setCertificateProvider: - (void)setCertificateProvider:
(facebook::flipper::SocketCertificateProvider)certificateProvider { (facebook::flipper::SocketCertificateProvider)certificateProvider {
_certificateProvider = certificateProvider;
_policy.certificateProvider = certificateProvider; _policy.certificateProvider = certificateProvider;
} }
@@ -241,7 +243,6 @@ static constexpr int connectionKeepaliveSeconds = 10;
} else { } else {
_eventHandler(facebook::flipper::SocketEvent::ERROR); _eventHandler(facebook::flipper::SocketEvent::ERROR);
} }
_socket = nil;
} }
- (void)_webSocketDidClose { - (void)_webSocketDidClose {
@@ -251,7 +252,6 @@ static constexpr int connectionKeepaliveSeconds = 10;
_keepAlive = nil; _keepAlive = nil;
_eventHandler(facebook::flipper::SocketEvent::CLOSE); _eventHandler(facebook::flipper::SocketEvent::CLOSE);
_socket = nil;
} }
- (void)_webSocketDidReceiveMessage:(id)message { - (void)_webSocketDidReceiveMessage:(id)message {

View File

@@ -112,15 +112,15 @@ bool FlipperWebSocket::connect(FlipperConnectionManager* manager) {
}; };
if (endpoint_.secure) { if (endpoint_.secure) {
socket_.certificateProvider = [this]( [socket_
char* _Nonnull password, size_t length) { setCertificateProvider:[this](char* _Nonnull password, size_t length) {
auto pkcs12 = connectionContextStore_->getCertificate(); auto pkcs12 = connectionContextStore_->getCertificate();
if (pkcs12.first.length() == 0) { if (pkcs12.first.length() == 0) {
return std::string(""); return std::string("");
} }
strncpy(password, pkcs12.second.c_str(), length); strncpy(password, pkcs12.second.c_str(), length);
return pkcs12.first; return pkcs12.first;
}; }];
} }
[socket_ connect]; [socket_ connect];