Skip to content

Commit

Permalink
Fix threading issue in IPC connection
Browse files Browse the repository at this point in the history
This change addresses the possibility of
`DTXIPCConnection._slaveDidConnectWithName` being called from non-main
thread by dispatching the work to main thread when it is being called
from non main thread. This is necessary so that `_otherConnection`
gets assigned on main thread.

`SBTUITestTunnelClient.serverDidConnect` is updated to wait for
`ipcConnection` to become valid. This is needed because
`DTXIPCConnection._slaveDidConnectWithName` and `SBTUITestTunnelClient.serverDidConnect`
can be called from different threads upon which we dispatch both to
main. But `SBTUITestTunnelClient.serverDidConnect` work can start
first at which point it can lead to assertion of `ipcConnection` not
being valid as `ipcConnection._otherConnection` has not yet been
assigned.
  • Loading branch information
alvar-bolt committed May 16, 2024
1 parent 8a480d7 commit 360d53c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
21 changes: 16 additions & 5 deletions Sources/SBTUITestTunnelClient/SBTUITestTunnelClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,26 @@ - (void)serverDidConnect:(id)sender

NSLog(@"[SBTUITestTunnel] IPC tunnel did connect after, %fs", CFAbsoluteTimeGetCurrent() - weakSelf.launchStart);

if (weakSelf.startupBlock) {
weakSelf.startupBlock();
[weakSelf checkConnectionAndProceed];
});
}

- (void)checkConnectionAndProceed
{
if (self.ipcConnection.isValid) {
if (_startupBlock) {
_startupBlock();
NSLog(@"[SBTUITestTunnel] Did perform startupBlock");
}

weakSelf.startupCompleted = [[weakSelf sendSynchronousRequestWithPath:SBTUITunneledApplicationCommandStartupCommandsCompleted params:@{}] isEqualToString:@"YES"];
_startupCompleted = [[self sendSynchronousRequestWithPath:SBTUITunneledApplicationCommandStartupCommandsCompleted params:@{}] isEqualToString:@"YES"];

NSLog(@"[SBTUITestTunnel] Tunnel ready after %fs", CFAbsoluteTimeGetCurrent() - weakSelf.launchStart);
});
NSLog(@"[SBTUITestTunnel] Tunnel ready after %fs", CFAbsoluteTimeGetCurrent() - _launchStart);
} else {
dispatch_async(dispatch_get_main_queue(), ^{
[self checkConnectionAndProceed];
});
}
}

- (void)performCommandWithParameters:(NSDictionary *)parameters block:(void (^)(NSDictionary *))block {}
Expand Down
12 changes: 10 additions & 2 deletions Sources/SBTUITestTunnelCommon/DetoxIPC/DTXIPCConnection.m
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,16 @@ - (void)setExportedObject:(id)exportedObject

- (oneway void)_slaveDidConnectWithName:(NSString*)slaveServiceName
{
_otherConnection = [NSConnection connectionWithRegisteredName:slaveServiceName host:nil];
[NSNotificationCenter.defaultCenter addObserver:self selector:@selector(_otherConnectionDidDie:) name:NSConnectionDidDieNotification object:_otherConnection];
dispatch_block_t block = ^{
self->_otherConnection = [NSConnection connectionWithRegisteredName:slaveServiceName host:nil];
[NSNotificationCenter.defaultCenter addObserver:self selector:@selector(_otherConnectionDidDie:) name:NSConnectionDidDieNotification object:self->_otherConnection];
};

if ([NSThread isMainThread]) {
block();
} else {
dispatch_async(dispatch_get_main_queue(), block);
}
}

- (oneway void)_invokeFromRemote:(NSDictionary*)serializedInvocation
Expand Down

0 comments on commit 360d53c

Please sign in to comment.