diff --git a/README.md b/README.md index a367763f7..345e1d157 100644 --- a/README.md +++ b/README.md @@ -42,15 +42,5 @@ Then, you find `WebDriverAgentRunner-Runner-sim-.zip` for iOS and `Web [`WebDriverAgent` is BSD-licensed](LICENSE). -## Third Party Sources - -WebDriverAgent depends on the following third-party frameworks: -- [CocoaHTTPServer](https://github.com/robbiehanson/CocoaHTTPServer) -- [RoutingHTTPServer](https://github.com/mattstevens/RoutingHTTPServer) - -These projects haven't been maintained in a while. That's why the source code of these -projects has been integrated directly in the WebDriverAgent source tree. - -You can find the source files and their licenses in the `WebDriverAgentLib/Vendor` directory. Have fun! diff --git a/WebDriverAgentLib/Commands/FBScreenshotCommands.m b/WebDriverAgentLib/Commands/FBScreenshotCommands.m index e2b090722..9586a197f 100644 --- a/WebDriverAgentLib/Commands/FBScreenshotCommands.m +++ b/WebDriverAgentLib/Commands/FBScreenshotCommands.m @@ -18,8 +18,8 @@ + (NSArray *)routes { return @[ - [[FBRoute GET:@"/screenshot"].withoutSession respondWithTarget:self action:@selector(handleGetScreenshot:)], - [[FBRoute GET:@"/screenshot"] respondWithTarget:self action:@selector(handleGetScreenshot:)], + [[FBRoute GET:@"/screenshot"].withoutSession.standalone respondWithTarget:self action:@selector(handleGetScreenshot:)], + [[FBRoute GET:@"/screenshot"].standalone respondWithTarget:self action:@selector(handleGetScreenshot:)], ]; } diff --git a/WebDriverAgentLib/Commands/FBSessionCommands.m b/WebDriverAgentLib/Commands/FBSessionCommands.m index c03cdd552..e8b886ac7 100644 --- a/WebDriverAgentLib/Commands/FBSessionCommands.m +++ b/WebDriverAgentLib/Commands/FBSessionCommands.m @@ -47,8 +47,8 @@ + (NSArray *)routes [[FBRoute POST:@"/wda/apps/state"] respondWithTarget:self action:@selector(handleSessionAppState:)], [[FBRoute GET:@"/wda/apps/list"] respondWithTarget:self action:@selector(handleGetActiveAppsList:)], [[FBRoute GET:@""] respondWithTarget:self action:@selector(handleGetActiveSession:)], - [[FBRoute DELETE:@""] respondWithTarget:self action:@selector(handleDeleteSession:)], - [[FBRoute GET:@"/status"].withoutSession respondWithTarget:self action:@selector(handleGetStatus:)], + [[FBRoute DELETE:@""].standalone respondWithTarget:self action:@selector(handleDeleteSession:)], + [[FBRoute GET:@"/status"].withoutSession.standalone respondWithTarget:self action:@selector(handleGetStatus:)], // Health check might modify simulator state so it should only be called in-between testing sessions [[FBRoute GET:@"/wda/healthcheck"].withoutSession respondWithTarget:self action:@selector(handleGetHealthCheck:)], @@ -89,9 +89,7 @@ + (NSArray *)routes + (id)handleCreateSession:(FBRouteRequest *)request { - if (nil != FBSession.activeSession) { - [FBSession.activeSession kill]; - } + [FBSession killActiveSessionAndWaitForTeardown]; NSDictionary *capabilities; id errorResponse = [self capabilitiesFromCreateSessionRequest:request diff --git a/WebDriverAgentLib/Routing/FBHTTPServer.h b/WebDriverAgentLib/Routing/FBHTTPServer.h index 42db7f115..75ea80ccd 100644 --- a/WebDriverAgentLib/Routing/FBHTTPServer.h +++ b/WebDriverAgentLib/Routing/FBHTTPServer.h @@ -47,17 +47,41 @@ NS_ASSUME_NONNULL_BEGIN /** Registers a route handler for the given HTTP method and path pattern (":param" segments are - captured into the request's `params`). + captured into the request's `params`). Equivalent to -handleMethod:withPath:standalone:block: + with standalone:NO. */ - (void)handleMethod:(NSString *)method withPath:(NSString *)path block:(void (^)(RouteRequest *request, RouteResponse *response))block; +/** + Registers a route handler that, when `standalone` is YES, bypasses -routeQueue entirely so a + handler stuck on that queue can never block it. Concurrent requests to the same method+path are + coalesced into a single in-flight execution, whose response is delivered to all of them; anything + else runs on its own queue, so distinct standalone endpoints always execute in parallel with each + other and with whatever is stuck on -routeQueue. + */ +- (void)handleMethod:(NSString *)method + withPath:(NSString *)path + standalone:(BOOL)standalone + block:(void (^)(RouteRequest *request, RouteResponse *response))block; + /** Convenience for -handleMethod:@"GET" withPath:path block:block. */ - (void)get:(NSString *)path withBlock:(void (^)(RouteRequest *request, RouteResponse *response))block; +/** + Immediately sends `response` to every non-standalone request currently pending for the given + "sessionID" path param - whether still queued on -routeQueue or already executing - instead of + leaving their HTTP clients waiting on a session that no longer exists. A request that has already + started executing keeps running to completion in the background regardless (GCD gives no way to + abort a block once it starts), but its eventual result is discarded rather than ever reaching a + client. `response` is written as-is to every pending client, so the caller is expected to supply + a fully-populated, protocol-correct error response (e.g. a W3C-shaped JSON body). + */ +- (void)abandonPendingRequestsForSessionID:(NSString *)sessionID withResponse:(RouteResponse *)response; + /** Starts listening on `port`. */ diff --git a/WebDriverAgentLib/Routing/FBHTTPServer.m b/WebDriverAgentLib/Routing/FBHTTPServer.m index a8cdd3e36..1532086a9 100644 --- a/WebDriverAgentLib/Routing/FBHTTPServer.m +++ b/WebDriverAgentLib/Routing/FBHTTPServer.m @@ -35,6 +35,7 @@ @interface FBHTTPRoute : NSObject @property (nonatomic, strong) NSRegularExpression *regex; @property (nonatomic, copy, nullable) NSArray *keys; @property (nonatomic, copy) void (^block)(RouteRequest *request, RouteResponse *response); +@property (nonatomic, assign) BOOL isStandalone; @end @implementation FBHTTPRoute @@ -55,6 +56,25 @@ @implementation FBPendingHTTPRequestHeader @end +// One dispatched-but-not-yet-answered request. Default (pointer) identity, so two pipelined +// requests sharing a connection are never conflated into a single tracked entry. +@interface FBPendingRequest : NSObject +@property (nonatomic, strong, readonly) nw_connection_t client; +@end + +@implementation FBPendingRequest + +- (instancetype)initWithClient:(nw_connection_t)client +{ + if ((self = [super init])) { + _client = client; + } + return self; +} + +@end + + @interface FBHTTPServer () @property (nonatomic, nullable, strong) FBTCPSocket *socket; @@ -67,6 +87,20 @@ @interface FBHTTPServer () // Per-client cache of the already-parsed request line + headers while its body is still // arriving; nil while a client's next unread bytes start with an unparsed header block. @property (nonatomic, strong) NSMapTable *pendingRequestHeaders; +// All buffer access - appending new data and -processBufferForClient:'s unlocked parse - is +// funneled through this one serial queue, so appends can never race a parse. +@property (nonatomic, strong) dispatch_queue_t bufferProcessingQueue; +// Connections with a request parsed off the buffer but not yet answered. Blocks +// -processBufferForClient: from starting the next pipelined request, so responses on one +// connection can't be written out of order. Guarded by @synchronized(self.connectionBuffers). +@property (nonatomic, strong) NSMutableSet *connectionsAwaitingResponse; +// Keyed by "METHOD path" - requests waiting on an already in-flight standalone request for that +// endpoint. Guarded by @synchronized(self.standaloneWaiters). +@property (nonatomic, strong) NSMutableDictionary *> *standaloneWaiters; +// Keyed by the "sessionID" path param - requests currently queued or executing for that session, +// standalone or not (except DELETE /session itself - see -dispatchMethod:). See +// -abandonPendingRequestsForSessionID:. Guarded by @synchronized(self.pendingSessionRequests). +@property (nonatomic, strong) NSMutableDictionary *> *pendingSessionRequests; @end @@ -81,6 +115,10 @@ - (instancetype)init valueOptions:(NSPointerFunctionsOptions)NSMapTableStrongMemory]; _pendingRequestHeaders = [NSMapTable mapTableWithKeyOptions:(NSPointerFunctionsOptions)(NSMapTableObjectPointerPersonality | NSMapTableStrongMemory) valueOptions:(NSPointerFunctionsOptions)NSMapTableStrongMemory]; + _bufferProcessingQueue = dispatch_queue_create("com.facebook.wda.http.bufferProcessing", DISPATCH_QUEUE_SERIAL); + _connectionsAwaitingResponse = [NSMutableSet set]; + _standaloneWaiters = [NSMutableDictionary dictionary]; + _pendingSessionRequests = [NSMutableDictionary dictionary]; } return self; } @@ -107,8 +145,7 @@ - (FBHTTPRoute *)compiledRouteWithPath:(NSString *)path FBHTTPRoute *route = [FBHTTPRoute new]; NSMutableArray *keys = [NSMutableArray array]; - // Escape regex-significant characters before substituting :param placeholders, like - // RoutingHTTPServer.m used to. + // Escape regex-significant characters before substituting :param placeholders. NSRegularExpression *escapeRegex = [NSRegularExpression regularExpressionWithPattern:@"[.+()]" options:(NSRegularExpressionOptions)0 error:nil]; @@ -157,10 +194,19 @@ - (FBHTTPRoute *)compiledRouteWithPath:(NSString *)path - (void)handleMethod:(NSString *)method withPath:(NSString *)path block:(void (^)(RouteRequest *request, RouteResponse *response))block +{ + [self handleMethod:method withPath:path standalone:NO block:block]; +} + +- (void)handleMethod:(NSString *)method + withPath:(NSString *)path + standalone:(BOOL)standalone + block:(void (^)(RouteRequest *request, RouteResponse *response))block { FBHTTPRoute *route = [self compiledRouteWithPath:path]; route.verb = method.uppercaseString; route.block = block; + route.isStandalone = standalone; [self.routes addObject:route]; } @@ -191,6 +237,7 @@ - (void)stop:(BOOL)immediately @synchronized (self.connectionBuffers) { [self.connectionBuffers removeAllObjects]; [self.pendingRequestHeaders removeAllObjects]; + [self.connectionsAwaitingResponse removeAllObjects]; } _isRunning = NO; } @@ -209,123 +256,136 @@ - (void)didClientDisconnect:(nw_connection_t)client @synchronized (self.connectionBuffers) { [self.connectionBuffers removeObjectForKey:client]; [self.pendingRequestHeaders removeObjectForKey:client]; + [self.connectionsAwaitingResponse removeObject:client]; } } - (void)client:(nw_connection_t)client didReceiveData:(NSData *)data { - NSMutableData *buffer; - @synchronized (self.connectionBuffers) { - buffer = [self.connectionBuffers objectForKey:client]; - if (nil == buffer) { + // The append itself, not just the parse, must run on bufferProcessingQueue: otherwise a receive + // callback here could still mutate the buffer while -processBufferForClient: is reading it + // unlocked on that queue. + __weak typeof(self) weakSelf = self; + dispatch_async(self.bufferProcessingQueue, ^{ + __strong typeof(weakSelf) strongSelf = weakSelf; + if (nil == strongSelf) { return; } - [buffer appendData:data]; - } - [self processBufferForClient:client]; + @synchronized (strongSelf.connectionBuffers) { + NSMutableData *buffer = [strongSelf.connectionBuffers objectForKey:client]; + if (nil == buffer) { + return; + } + [buffer appendData:data]; + } + [strongSelf processBufferForClient:client]; + }); } #pragma mark - HTTP parsing +// Parses and dispatches at most one request per call; a connection with one already in flight is +// left alone (see -connectionsAwaitingResponse) until its response is written. - (void)processBufferForClient:(nw_connection_t)client { - while (YES) { - NSMutableData *buffer; - FBPendingHTTPRequestHeader *pending; - @synchronized (self.connectionBuffers) { - buffer = [self.connectionBuffers objectForKey:client]; - if (nil == buffer) { - return; - } - pending = [self.pendingRequestHeaders objectForKey:client]; + NSMutableData *buffer; + FBPendingHTTPRequestHeader *pending; + @synchronized (self.connectionBuffers) { + if ([self.connectionsAwaitingResponse containsObject:client]) { + return; } + buffer = [self.connectionBuffers objectForKey:client]; + if (nil == buffer) { + return; + } + pending = [self.pendingRequestHeaders objectForKey:client]; + } - if (nil == pending) { - NSRange headerEndRange = [buffer rangeOfData:FBCRLFCRLFData() options:(NSDataSearchOptions)0 range:NSMakeRange(0, buffer.length)]; - if (NSNotFound == headerEndRange.location) { - // Wait for the rest of the header block to arrive. - return; - } + if (nil == pending) { + NSRange headerEndRange = [buffer rangeOfData:FBCRLFCRLFData() options:(NSDataSearchOptions)0 range:NSMakeRange(0, buffer.length)]; + if (NSNotFound == headerEndRange.location) { + // Wait for the rest of the header block to arrive. + return; + } - NSData *headerData = [buffer subdataWithRange:NSMakeRange(0, headerEndRange.location)]; - NSString *headerString = [[NSString alloc] initWithData:headerData encoding:NSUTF8StringEncoding]; - NSArray *lines = [headerString componentsSeparatedByString:@"\r\n"]; - if (lines.count < 1) { - [self respondBadRequestToClient:client]; - return; - } + NSData *headerData = [buffer subdataWithRange:NSMakeRange(0, headerEndRange.location)]; + NSString *headerString = [[NSString alloc] initWithData:headerData encoding:NSUTF8StringEncoding]; + NSArray *lines = [headerString componentsSeparatedByString:@"\r\n"]; + if (lines.count < 1) { + [self respondBadRequestToClient:client]; + return; + } - NSArray *requestLineParts = [lines.firstObject componentsSeparatedByString:@" "]; - if (requestLineParts.count < 2) { - [self respondBadRequestToClient:client]; - return; - } + NSArray *requestLineParts = [lines.firstObject componentsSeparatedByString:@" "]; + if (requestLineParts.count < 2) { + [self respondBadRequestToClient:client]; + return; + } - NSMutableDictionary *requestHeaders = [NSMutableDictionary dictionary]; - for (NSUInteger i = 1; i < lines.count; i++) { - NSString *line = lines[i]; - NSRange colonRange = [line rangeOfString:@":"]; - if (NSNotFound == colonRange.location) { - continue; - } - NSString *name = [line substringToIndex:colonRange.location]; - NSString *value = [[line substringFromIndex:colonRange.location + 1] - stringByTrimmingCharactersInSet:NSCharacterSet.whitespaceCharacterSet]; - requestHeaders[name.lowercaseString] = value; + NSMutableDictionary *requestHeaders = [NSMutableDictionary dictionary]; + for (NSUInteger i = 1; i < lines.count; i++) { + NSString *line = lines[i]; + NSRange colonRange = [line rangeOfString:@":"]; + if (NSNotFound == colonRange.location) { + continue; } + NSString *name = [line substringToIndex:colonRange.location]; + NSString *value = [[line substringFromIndex:colonRange.location + 1] + stringByTrimmingCharactersInSet:NSCharacterSet.whitespaceCharacterSet]; + requestHeaders[name.lowercaseString] = value; + } - NSString *transferEncoding = requestHeaders[@"transfer-encoding"]; - if (transferEncoding.length > 0) { - // No transfer decoder is implemented at all, so any encoding (chunked or otherwise - - // including a value only introduced by a duplicate header overwriting "chunked" above) - // is rejected rather than risking the body being misread as empty and desyncing the rest - // of the connection's request stream. - RouteResponse *notImplemented = [RouteResponse new]; - id notImplementedPayload = FBResponseWithStatus([FBCommandStatus unknownCommandErrorWithMessage:@"Transfer-Encoding is not supported" + NSString *transferEncoding = requestHeaders[@"transfer-encoding"]; + if (transferEncoding.length > 0) { + // No transfer decoder is implemented at all, so any encoding (chunked or otherwise - + // including a value only introduced by a duplicate header overwriting "chunked" above) + // is rejected rather than risking the body being misread as empty and desyncing the rest + // of the connection's request stream. + RouteResponse *notImplemented = [RouteResponse new]; + id notImplementedPayload = FBResponseWithStatus([FBCommandStatus invalidArgumentErrorWithMessage:@"Transfer-Encoding is not supported" traceback:nil]); - [notImplementedPayload dispatchWithResponse:notImplemented]; - [self failClient:client withResponse:notImplemented]; - return; - } - - NSUInteger contentLength = (NSUInteger)requestHeaders[@"content-length"].integerValue; - if (contentLength > FBConfiguration.sharedInstance.httpRequestBodySizeLimit) { - // Mirrors CocoaHTTPServer's maxRequestBodySize enforcement. Closes the connection after - // responding, since the rest of the oversized body is still incoming. - RouteResponse *tooLarge = [RouteResponse new]; - id tooLargePayload = FBResponseWithStatus([FBCommandStatus unknownCommandErrorWithMessage:@"Request Entity Too Large" - traceback:nil]); - [tooLargePayload dispatchWithResponse:tooLarge]; - [self failClient:client withResponse:tooLarge]; - return; - } - - pending = [FBPendingHTTPRequestHeader new]; - pending.method = requestLineParts[0].uppercaseString; - pending.pathAndQuery = requestLineParts[1]; - pending.bodyStart = headerEndRange.location + headerEndRange.length; - pending.contentLength = contentLength; - @synchronized (self.connectionBuffers) { - [self.pendingRequestHeaders setObject:pending forKey:client]; - } + [notImplementedPayload dispatchWithResponse:notImplemented]; + [self failClient:client withResponse:notImplemented]; + return; } - NSUInteger totalRequestLength = pending.bodyStart + pending.contentLength; - if (buffer.length < totalRequestLength) { - // Wait for the rest of the body to arrive - the parsed header stays cached above, so this - // doesn't re-scan/re-parse the header block on every subsequently arriving chunk. + NSUInteger contentLength = (NSUInteger)requestHeaders[@"content-length"].integerValue; + if (contentLength > FBConfiguration.sharedInstance.httpRequestBodySizeLimit) { + // Closes the connection after responding, since the rest of the oversized body is still incoming. + RouteResponse *tooLarge = [RouteResponse new]; + id tooLargePayload = FBResponseWithStatus([FBCommandStatus invalidArgumentErrorWithMessage:@"The request body exceeds the configured size limit" + traceback:nil]); + [tooLargePayload dispatchWithResponse:tooLarge]; + [self failClient:client withResponse:tooLarge]; return; } - NSData *body = pending.contentLength > 0 ? [buffer subdataWithRange:NSMakeRange(pending.bodyStart, pending.contentLength)] : [NSData data]; - + pending = [FBPendingHTTPRequestHeader new]; + pending.method = requestLineParts[0].uppercaseString; + pending.pathAndQuery = requestLineParts[1]; + pending.bodyStart = headerEndRange.location + headerEndRange.length; + pending.contentLength = contentLength; @synchronized (self.connectionBuffers) { - [buffer replaceBytesInRange:NSMakeRange(0, totalRequestLength) withBytes:NULL length:0]; - [self.pendingRequestHeaders removeObjectForKey:client]; + [self.pendingRequestHeaders setObject:pending forKey:client]; } + } - [self dispatchMethod:pending.method pathAndQuery:pending.pathAndQuery body:body client:client]; + NSUInteger totalRequestLength = pending.bodyStart + pending.contentLength; + if (buffer.length < totalRequestLength) { + // Wait for the rest of the body to arrive - the parsed header stays cached above, so this + // doesn't re-scan/re-parse the header block on every subsequently arriving chunk. + return; } + + NSData *body = pending.contentLength > 0 ? [buffer subdataWithRange:NSMakeRange(pending.bodyStart, pending.contentLength)] : [NSData data]; + + @synchronized (self.connectionBuffers) { + [buffer replaceBytesInRange:NSMakeRange(0, totalRequestLength) withBytes:NULL length:0]; + [self.pendingRequestHeaders removeObjectForKey:client]; + [self.connectionsAwaitingResponse addObject:client]; + } + + [self dispatchMethod:pending.method pathAndQuery:pending.pathAndQuery body:body client:client]; } // Removes the client's buffered state and responds with a closing error response. Removing the @@ -344,8 +404,8 @@ - (void)failClient:(nw_connection_t)client withResponse:(RouteResponse *)respons - (void)respondBadRequestToClient:(nw_connection_t)client { RouteResponse *badRequest = [RouteResponse new]; - id payload = FBResponseWithStatus([FBCommandStatus unknownCommandErrorWithMessage:@"The request could not be parsed as valid HTTP" - traceback:nil]); + id payload = FBResponseWithStatus([FBCommandStatus invalidArgumentErrorWithMessage:@"The request could not be parsed as valid HTTP" + traceback:nil]); [payload dispatchWithResponse:badRequest]; [self failClient:client withResponse:badRequest]; } @@ -388,9 +448,29 @@ - (void)dispatchMethod:(NSString *)method pathAndQuery:(NSString *)pathAndQuery RouteResponse *response = [RouteResponse new]; [self applyDefaultHeadersToResponse:response]; + NSString *sessionID = params[@"sessionID"]; + if (route.isStandalone) { + // DELETE triggers -abandonPendingRequestsForSessionID: itself; tracking its own request + // would make it abandon itself and write a response twice. + NSString *trackedSessionID = [route.verb isEqualToString:@"DELETE"] ? nil : sessionID; + [self dispatchStandaloneRoute:route request:request response:response client:client method:method pathAndQuery:pathAndQuery sessionID:trackedSessionID]; + return; + } + + FBPendingRequest *pendingRequest = nil; + if (nil != sessionID) { + pendingRequest = [[FBPendingRequest alloc] initWithClient:client]; + [self trackPendingRequest:pendingRequest forSessionID:sessionID]; + } + void (^invoke)(void) = ^{ route.block(request, response); - [self writeResponse:response toClient:client]; + // Whoever untracks `pendingRequest` first "wins" and gets to respond - either this normal + // completion, or -abandonPendingRequestsForSessionID: on another thread. + BOOL shouldRespond = (nil == pendingRequest) || [self untrackPendingRequest:pendingRequest forSessionID:sessionID]; + if (shouldRespond) { + [self writeResponse:response toClient:client]; + } }; dispatch_queue_t routeQueue = self.routeQueue; if (routeQueue) { @@ -409,6 +489,103 @@ - (void)dispatchMethod:(NSString *)method pathAndQuery:(NSString *)pathAndQuery [self writeResponse:notFound toClient:client]; } +#pragma mark - Session-scoped request cancellation + +- (void)trackPendingRequest:(FBPendingRequest *)pendingRequest forSessionID:(NSString *)sessionID +{ + @synchronized (self.pendingSessionRequests) { + NSMutableSet *pendingRequests = self.pendingSessionRequests[sessionID]; + if (nil == pendingRequests) { + pendingRequests = [NSMutableSet set]; + self.pendingSessionRequests[sessionID] = pendingRequests; + } + [pendingRequests addObject:pendingRequest]; + } +} + +// Returns YES if this caller won the race to respond, vs. -abandonPendingRequestsForSessionID: +// already having claimed `pendingRequest` on another thread. +- (BOOL)untrackPendingRequest:(FBPendingRequest *)pendingRequest forSessionID:(NSString *)sessionID +{ + @synchronized (self.pendingSessionRequests) { + NSMutableSet *pendingRequests = self.pendingSessionRequests[sessionID]; + BOOL wasPending = [pendingRequests containsObject:pendingRequest]; + if (wasPending) { + [pendingRequests removeObject:pendingRequest]; + if (0 == pendingRequests.count) { + [self.pendingSessionRequests removeObjectForKey:sessionID]; + } + } + return wasPending; + } +} + +- (void)abandonPendingRequestsForSessionID:(NSString *)sessionID withResponse:(RouteResponse *)response +{ + NSSet *pendingRequests; + @synchronized (self.pendingSessionRequests) { + pendingRequests = [self.pendingSessionRequests[sessionID] copy]; + [self.pendingSessionRequests removeObjectForKey:sessionID]; + } + for (FBPendingRequest *pendingRequest in pendingRequests) { + [self writeResponse:response toClient:pendingRequest.client]; + } +} + +#pragma mark - Standalone route dispatch + +- (void)dispatchStandaloneRoute:(FBHTTPRoute *)route + request:(RouteRequest *)request + response:(RouteResponse *)response + client:(nw_connection_t)client + method:(NSString *)method + pathAndQuery:(NSString *)pathAndQuery + sessionID:(nullable NSString *)sessionID +{ + // Includes the query string so requests with different params are never coalesced together. + NSString *key = [NSString stringWithFormat:@"%@ %@", method, pathAndQuery]; + FBPendingRequest *waiter = [[FBPendingRequest alloc] initWithClient:client]; + if (nil != sessionID) { + [self trackPendingRequest:waiter forSessionID:sessionID]; + } + + BOOL isInFlight = NO; + @synchronized (self.standaloneWaiters) { + NSMutableArray *waiters = self.standaloneWaiters[key]; + if (nil != waiters) { + [waiters addObject:waiter]; + isInFlight = YES; + } else { + self.standaloneWaiters[key] = [NSMutableArray array]; + } + } + if (isInFlight) { + // An identical request is already executing; it will deliver this connection's response too. + return; + } + + dispatch_queue_t queue = dispatch_queue_create(key.UTF8String, DISPATCH_QUEUE_SERIAL); + __weak typeof(self) weakSelf = self; + dispatch_async(queue, ^{ + route.block(request, response); + __strong typeof(weakSelf) strongSelf = weakSelf; + if (nil == strongSelf) { + return; + } + NSArray *joinedWaiters; + @synchronized (strongSelf.standaloneWaiters) { + joinedWaiters = [strongSelf.standaloneWaiters[key] copy]; + [strongSelf.standaloneWaiters removeObjectForKey:key]; + } + for (FBPendingRequest *joinedWaiter in [@[waiter] arrayByAddingObjectsFromArray:joinedWaiters]) { + BOOL shouldRespond = (nil == sessionID) || [strongSelf untrackPendingRequest:joinedWaiter forSessionID:sessionID]; + if (shouldRespond) { + [strongSelf writeResponse:response toClient:joinedWaiter.client]; + } + } + }); +} + - (void)writeResponse:(RouteResponse *)response toClient:(nw_connection_t)client { [self writeResponse:response toClient:client thenCloseConnection:NO]; @@ -439,7 +616,15 @@ - (void)writeResponse:(RouteResponse *)response toClient:(nw_connection_t)client [weakSelf closeClient:client]; }]; } else { + // Sent before unblocking the next pipelined request, so responses can't reach the wire out of order. [self.socket writeData:payload toClient:client]; + @synchronized (self.connectionBuffers) { + [self.connectionsAwaitingResponse removeObject:client]; + } + __weak typeof(self) weakSelf = self; + dispatch_async(self.bufferProcessingQueue, ^{ + [weakSelf processBufferForClient:client]; + }); } } @@ -447,6 +632,7 @@ - (void)closeClient:(nw_connection_t)client { @synchronized (self.connectionBuffers) { [self.connectionBuffers removeObjectForKey:client]; + [self.connectionsAwaitingResponse removeObject:client]; } nw_connection_cancel(client); } diff --git a/WebDriverAgentLib/Routing/FBRoute.h b/WebDriverAgentLib/Routing/FBRoute.h index fce8dd8a9..35d55add2 100644 --- a/WebDriverAgentLib/Routing/FBRoute.h +++ b/WebDriverAgentLib/Routing/FBRoute.h @@ -27,6 +27,9 @@ typedef __nonnull id (^FBRouteSyncHandler)(FBRouteRequest *re /*! Route's path */ @property (nonatomic, copy, readonly) NSString *path; +/*! Whether this route bypasses the shared route queue - see -standalone */ +@property (nonatomic, assign, readonly) BOOL isStandalone; + /** Convenience constructor for GET route with given pathPattern */ @@ -67,6 +70,12 @@ typedef __nonnull id (^FBRouteSyncHandler)(FBRouteRequest *re */ - (instancetype)withoutSession; +/** + Chain-able constructor for a route that bypasses the shared route queue - see FBHTTPServer.h's + -handleMethod:withPath:standalone:block: for what that changes about how/when the handler runs. + */ +- (instancetype)standalone; + /** Dispatches response for request */ diff --git a/WebDriverAgentLib/Routing/FBRoute.m b/WebDriverAgentLib/Routing/FBRoute.m index fbe69b8c3..47964014d 100644 --- a/WebDriverAgentLib/Routing/FBRoute.m +++ b/WebDriverAgentLib/Routing/FBRoute.m @@ -18,6 +18,7 @@ @interface FBRoute () @property (nonatomic, assign, readwrite) BOOL requiresSession; +@property (nonatomic, assign, readwrite) BOOL isStandalone; @property (nonatomic, copy, readwrite) NSString *verb; @property (nonatomic, copy, readwrite) NSString *path; @@ -126,10 +127,17 @@ - (instancetype)withoutSession return self; } +- (instancetype)standalone +{ + self.isStandalone = YES; + return self; +} + - (instancetype)respondWithBlock:(FBRouteSyncHandler)handler { FBRoute_Sync *route = [FBRoute_Sync withVerb:self.verb path:self.path requiresSession:self.requiresSession]; route.handler = handler; + route.isStandalone = self.isStandalone; return route; } @@ -138,6 +146,7 @@ - (instancetype)respondWithTarget:(id)target action:(SEL)action FBRoute_TargetAction *route = [FBRoute_TargetAction withVerb:self.verb path:self.path requiresSession:self.requiresSession]; route.target = target; route.action = action; + route.isStandalone = self.isStandalone; return route; } diff --git a/WebDriverAgentLib/Routing/FBSession.h b/WebDriverAgentLib/Routing/FBSession.h index 61b1f8742..821891ea3 100644 --- a/WebDriverAgentLib/Routing/FBSession.h +++ b/WebDriverAgentLib/Routing/FBSession.h @@ -16,6 +16,12 @@ NS_ASSUME_NONNULL_BEGIN /** Bundle identifier of Mobile Safari browser */ extern NSString* const FB_SAFARI_BUNDLE_ID; +/** + Posted (synchronously, on whatever thread calls -kill) once a session has been torn down. The + notification's object is the FBSession instance that was killed - see -identifier. + */ +extern NSString* const FBSessionWasKilledNotification; + /** Class that represents testing session */ @@ -44,6 +50,13 @@ extern NSString* const FB_SAFARI_BUNDLE_ID; + (nullable instancetype)activeSession; +/** + Kills the active session, if any, and blocks until its teardown - including one already started + by a concurrent caller - is fully finished. Call this before preparing/launching a replacement + application, so it can't race a still-in-progress termination of the outgoing one. + */ ++ (void)killActiveSessionAndWaitForTeardown; + /** Fetches session for given identifier. If identifier doesn't match activeSession identifier, will return nil. diff --git a/WebDriverAgentLib/Routing/FBSession.m b/WebDriverAgentLib/Routing/FBSession.m index 02889303a..34a80db0e 100644 --- a/WebDriverAgentLib/Routing/FBSession.m +++ b/WebDriverAgentLib/Routing/FBSession.m @@ -34,12 +34,23 @@ NSString *const FB_SAFARI_BUNDLE_ID = @"com.apple.mobilesafari"; +// FBXCAXClientProxy's shared accessibility channel can be stuck servicing another request. +static const NSTimeInterval FB_IS_SYSTEM_APP_CHECK_TIMEOUT_SEC = 5.; +// -terminate hard-asserts off the main thread, which may itself be busy - see -fb_terminate...:. +static const NSTimeInterval FB_APP_TERMINATE_TIMEOUT_SEC = 5.; +// How long a -kill caller that lost the race below waits for the winner's teardown to finish. +static const NSTimeInterval FB_KILL_WAIT_TIMEOUT_SEC = 35.; +NSString *const FBSessionWasKilledNotification = @"FBSessionWasKilledNotification"; + @interface FBSession () @property (nullable, nonatomic) XCUIApplication *testedApplication; @property (nonatomic) BOOL isTestedApplicationExpectedToRun; @property (nonatomic) BOOL shouldAppsWaitForQuiescence; @property (nonatomic, nullable) FBAlertsMonitor *alertsMonitor; @property (nonatomic, readwrite) NSMutableDictionary *> *elementsVisibilityCache; + +- (BOOL)fb_isTestedApplicationSameAsSystemAppWithTimeout:(NSTimeInterval)timeout; +- (void)fb_terminateTestedApplicationWithTimeout:(NSTimeInterval)timeout; @end @interface FBSession (FBAlertsMonitorDelegate) @@ -90,17 +101,54 @@ - (void)didDetectAlert:(FBAlert *)alert @implementation FBSession static FBSession *_activeSession = nil; +// Class-level, not per-instance: a caller that finds _activeSession already nil (a concurrent +// -kill beat it there) still needs to know whether that -kill's teardown is done, since it cleared +// the pointer before running it. See +waitForActiveTeardownWithTimeout:. +static BOOL _isTeardownInProgress = NO; + ++ (NSCondition *)teardownCondition +{ + static NSCondition *condition; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + condition = [NSCondition new]; + }); + return condition; +} + +// Waits (bounded) for any -kill teardown currently in progress to finish. ++ (void)waitForActiveTeardownWithTimeout:(NSTimeInterval)timeout +{ + NSCondition *condition = self.teardownCondition; + [condition lock]; + NSDate *deadline = [NSDate dateWithTimeIntervalSinceNow:timeout]; + while (_isTeardownInProgress && [condition waitUntilDate:deadline]) { + } + [condition unlock]; +} + (instancetype)activeSession { return _activeSession; } -+ (void)markSessionActive:(FBSession *)session ++ (void)killActiveSessionAndWaitForTeardown { - if (_activeSession) { - [_activeSession kill]; + FBSession *session = _activeSession; + if (nil != session) { + // Runs the real teardown synchronously if this call wins the race in -kill, or waits for + // whoever did to finish if it lost - either way, blocks until torn down. + [session kill]; + } else { + // _activeSession is already nil, but a concurrent -kill (e.g. from DELETE /session) may still + // be mid-teardown - wait for it, so we don't launch a replacement app too early. + [self waitForActiveTeardownWithTimeout:FB_KILL_WAIT_TIMEOUT_SEC]; } +} + ++ (void)markSessionActive:(FBSession *)session +{ + [self killActiveSessionAndWaitForTeardown]; _activeSession = session; } @@ -169,33 +217,58 @@ - (BOOL)disableAlertsMonitor - (void)kill { - if (nil == _activeSession) { + // DELETE /session and session creation can now run concurrently, so a session already + // superseded by a newer one can still reach here via a stale reference. Check-and-clear must be + // atomic, else a belated -kill could null out the new session's pointer instead of its own. + BOOL wasActive; + @synchronized (self.class) { + wasActive = (self == _activeSession); + if (wasActive) { + _activeSession = nil; + } + } + if (!wasActive) { + // Someone else is already tearing this session down - wait for that to finish (bounded), so + // we don't act as if it's gone (e.g. launch a new app) while its -terminate is still in flight. + [self.class waitForActiveTeardownWithTimeout:FB_KILL_WAIT_TIMEOUT_SEC]; return; } - [self disableAlertsMonitor]; + NSCondition *teardownCondition = self.class.teardownCondition; + [teardownCondition lock]; + _isTeardownInProgress = YES; + [teardownCondition unlock]; + + @try { + // Posted before teardown so pending HTTP requests for this session can stop waiting sooner. + [NSNotificationCenter.defaultCenter postNotificationName:FBSessionWasKilledNotification object:self]; - FBScreenRecordingPromise *activeScreenRecording = FBScreenRecordingContainer.sharedInstance.screenRecordingPromise; - if (nil != activeScreenRecording) { - NSError *error; - if (![FBXCTestDaemonsProxy stopScreenRecordingWithUUID:activeScreenRecording.identifier error:&error]) { - [FBLogger logFmt:@"%@", error]; + [self disableAlertsMonitor]; + + FBScreenRecordingPromise *activeScreenRecording = FBScreenRecordingContainer.sharedInstance.screenRecordingPromise; + if (nil != activeScreenRecording) { + NSError *error; + if (![FBXCTestDaemonsProxy stopScreenRecordingWithUUID:activeScreenRecording.identifier error:&error]) { + [FBLogger logFmt:@"%@", error]; + } + [FBScreenRecordingContainer.sharedInstance reset]; } - [FBScreenRecordingContainer.sharedInstance reset]; - } - if (nil != self.testedApplication - && FBConfiguration.sharedInstance.shouldTerminateApp - && self.testedApplication.running - && ![self.testedApplication fb_isSameAppAs:XCUIApplication.fb_systemApplication]) { - @try { - [self.testedApplication terminate]; - } @catch (NSException *e) { - [FBLogger logFmt:@"%@", e.description]; + if (nil != self.testedApplication + && FBConfiguration.sharedInstance.shouldTerminateApp + && self.testedApplication.running + && ![self fb_isTestedApplicationSameAsSystemAppWithTimeout:FB_IS_SYSTEM_APP_CHECK_TIMEOUT_SEC]) { + // Blocks until the app is either actually terminated or durably given up on (never left + // pending) - see -fb_terminateTestedApplicationWithTimeout: - so it's safe to report this + // teardown as finished as soon as this returns. + [self fb_terminateTestedApplicationWithTimeout:FB_APP_TERMINATE_TIMEOUT_SEC]; } + } @finally { + [teardownCondition lock]; + _isTeardownInProgress = NO; + [teardownCondition broadcast]; + [teardownCondition unlock]; } - - _activeSession = nil; } - (XCUIApplication *)activeApplication @@ -290,4 +363,62 @@ - (XCUIApplication *)makeApplicationWithBundleId:(NSString *)bundleIdentifier : [[XCUIApplication alloc] initWithBundleIdentifier:bundleIdentifier]; } +// Has no async variant and can block on the shared accessibility channel. Run off-thread and give +// up after `timeout`, assuming the tested app IS the system app - safer, since it means skipping +// termination rather than risking terminating springboard. +- (BOOL)fb_isTestedApplicationSameAsSystemAppWithTimeout:(NSTimeInterval)timeout +{ + __block XCUIApplication *systemApp = nil; + __block NSException *caughtException = nil; + dispatch_semaphore_t sem = dispatch_semaphore_create(0); + dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{ + // Undocumented private API; guard in case it hard-asserts off-main like -terminate does on + // some Xcode/iOS version - uncaught, that would crash the whole process. + @try { + systemApp = XCUIApplication.fb_systemApplication; + } @catch (NSException *e) { + caughtException = e; + } + dispatch_semaphore_signal(sem); + }); + int64_t timeoutNs = (int64_t)(timeout * NSEC_PER_SEC); + if (0 != dispatch_semaphore_wait(sem, dispatch_time(DISPATCH_TIME_NOW, timeoutNs)) || nil != caughtException) { + [FBLogger logFmt:@"Could not determine the system application within %@ seconds%@; assuming '%@' might be it and skipping its termination", @(timeout), nil == caughtException ? @"" : [NSString stringWithFormat:@" (%@)", caughtException.description], self.testedApplication.bundleID]; + return YES; + } + return [self.testedApplication fb_isSameAppAs:systemApp]; +} + +// -terminate hard-asserts off-main, but -kill can now run on a background queue. Dispatching to +// main and waiting indefinitely could hang just as long as main is stuck, so give up after +// `timeout` - but a "given up on" call must never still terminate whatever's running by the time +// main gets to it (e.g. a replacement session's app), so cancellation and the actual terminate +// call share a lock: whichever gets there first - the dispatched block, or the timeout - wins. +- (void)fb_terminateTestedApplicationWithTimeout:(NSTimeInterval)timeout +{ + XCUIApplication *application = self.testedApplication; + NSObject *lock = [NSObject new]; + __block BOOL isAllowedToTerminate = YES; + dispatch_semaphore_t sem = dispatch_semaphore_create(0); + dispatch_async(dispatch_get_main_queue(), ^{ + @synchronized (lock) { + if (isAllowedToTerminate) { + @try { + [application terminate]; + } @catch (NSException *e) { + [FBLogger logFmt:@"%@", e.description]; + } + } + } + dispatch_semaphore_signal(sem); + }); + int64_t timeoutNs = (int64_t)(timeout * NSEC_PER_SEC); + if (0 != dispatch_semaphore_wait(sem, dispatch_time(DISPATCH_TIME_NOW, timeoutNs))) { + @synchronized (lock) { + isAllowedToTerminate = NO; + } + [FBLogger logFmt:@"Could not terminate '%@' within %@ seconds; giving up on it rather than risk terminating a possible replacement session's app later", application.bundleID, @(timeout)]; + } +} + @end diff --git a/WebDriverAgentLib/Routing/FBTCPSocket.m b/WebDriverAgentLib/Routing/FBTCPSocket.m index 0b8ea5112..769add707 100644 --- a/WebDriverAgentLib/Routing/FBTCPSocket.m +++ b/WebDriverAgentLib/Routing/FBTCPSocket.m @@ -135,7 +135,8 @@ - (void)acceptConnection:(nw_connection_t)connection } [strongSelf scheduleReceiveForConnection:connection]; } else if (nw_connection_state_failed == state || nw_connection_state_cancelled == state) { - [weakSelf handleDisconnectForConnection:connection]; + __strong typeof(weakSelf) strongSelf = weakSelf; + [strongSelf handleDisconnectForConnection:connection]; } }); nw_connection_start(connection); diff --git a/WebDriverAgentLib/Routing/FBWebServer.m b/WebDriverAgentLib/Routing/FBWebServer.m index 46314f2a2..691ce4499 100644 --- a/WebDriverAgentLib/Routing/FBWebServer.m +++ b/WebDriverAgentLib/Routing/FBWebServer.m @@ -13,8 +13,10 @@ #import "FBTCPSocket.h" #import "FBCommandHandler.h" +#import "FBCommandStatus.h" #import "FBErrorBuilder.h" #import "FBExceptionHandler.h" +#import "FBResponsePayload.h" #import "FBRouteRequest.h" #import "FBRuntimeUtils.h" #import "FBSession.h" @@ -80,6 +82,11 @@ - (BOOL)startHTTPServer [self.server setDefaultHeader:@"Access-Control-Allow-Origin" value:@"*"]; [self.server setDefaultHeader:@"Access-Control-Allow-Headers" value:@"Content-Type, X-Requested-With"]; + [NSNotificationCenter.defaultCenter addObserver:self + selector:@selector(sessionWasKilled:) + name:FBSessionWasKilledNotification + object:nil]; + [self registerRouteHandlers:[self.class collectCommandHandlerClasses]]; [self registerServerKeyRouteHandlers]; @@ -167,8 +174,26 @@ - (void)readMjpegSettingsFromEnv } } +- (void)sessionWasKilled:(NSNotification *)notification +{ + FBSession *session = notification.object; + if (![session isKindOfClass:FBSession.class]) { + return; + } + // Same "invalid session id" shape a still-queued request would eventually get anyway, once + // -routeQueue drains and FBRoute.decorateRequest: finds the session gone - just delivered now + // instead of after however long the request would otherwise have been stuck waiting. + NSString *message = [NSString stringWithFormat:@"Session %@ was deleted while this request was still pending", session.identifier]; + id payload = FBResponseWithStatus([FBCommandStatus noSuchDriverErrorWithMessage:message + traceback:nil]); + RouteResponse *response = [RouteResponse new]; + [payload dispatchWithResponse:response]; + [self.server abandonPendingRequestsForSessionID:session.identifier withResponse:response]; +} + - (void)stopServing { + [NSNotificationCenter.defaultCenter removeObserver:self name:FBSessionWasKilledNotification object:nil]; [FBSession.activeSession kill]; [self stopScreenshotsBroadcaster]; if (self.server.isRunning) { @@ -208,7 +233,7 @@ - (void)registerRouteHandlers:(NSArray *)commandHandlerClasses for (Class commandHandler in commandHandlerClasses) { NSArray *routes = [commandHandler routes]; for (FBRoute *route in routes) { - [self.server handleMethod:route.verb withPath:route.path block:^(RouteRequest *request, RouteResponse *response) { + [self.server handleMethod:route.verb withPath:route.path standalone:route.isStandalone block:^(RouteRequest *request, RouteResponse *response) { __strong typeof(weakSelf) strongSelf = weakSelf; if (nil == strongSelf) { return; diff --git a/WebDriverAgentLib/Routing/RouteResponse.m b/WebDriverAgentLib/Routing/RouteResponse.m index 393ccab05..eb383724b 100644 --- a/WebDriverAgentLib/Routing/RouteResponse.m +++ b/WebDriverAgentLib/Routing/RouteResponse.m @@ -9,7 +9,7 @@ #import "RouteResponse.h" @interface RouteResponse () -@property (nonatomic, copy) NSMutableDictionary *mutableHeaders; +@property (nonatomic, strong) NSMutableDictionary *mutableHeaders; @end @implementation RouteResponse diff --git a/WebDriverAgentLib/Utilities/FBConfiguration.m b/WebDriverAgentLib/Utilities/FBConfiguration.m index 65bb37247..6a37b431c 100644 --- a/WebDriverAgentLib/Utilities/FBConfiguration.m +++ b/WebDriverAgentLib/Utilities/FBConfiguration.m @@ -124,17 +124,14 @@ - (NSRange)bindingPortRange { // 'WebDriverAgent --port 8080' can be passed via the arguments to the process NSRange rangeFromArguments = [self.class bindingPortRangeFromArguments]; - if (rangeFromArguments.location != NSNotFound) { - return rangeFromArguments; + if (rangeFromArguments.location == NSNotFound) { + // Existence of USE_PORT in the environment implies the port range is managed by the launching process. + NSString *usePort = NSProcessInfo.processInfo.environment[@"USE_PORT"]; + rangeFromArguments = usePort.length > 0 + ? NSMakeRange((NSUInteger)usePort.integerValue, 1) + : NSMakeRange(DefaultStartingPort, DefaultPortRange); } - - // Existence of USE_PORT in the environment implies the port range is managed by the launching process. - if (NSProcessInfo.processInfo.environment[@"USE_PORT"] && - [NSProcessInfo.processInfo.environment[@"USE_PORT"] length] > 0) { - return NSMakeRange([NSProcessInfo.processInfo.environment[@"USE_PORT"] integerValue] , 1); - } - - return NSMakeRange(DefaultStartingPort, DefaultPortRange); + return rangeFromArguments; } - (NSString *)bindingIPAddress diff --git a/WebDriverAgentLib/Utilities/FBXCTestDaemonsProxy.m b/WebDriverAgentLib/Utilities/FBXCTestDaemonsProxy.m index cec4a8bfa..34fca95e4 100644 --- a/WebDriverAgentLib/Utilities/FBXCTestDaemonsProxy.m +++ b/WebDriverAgentLib/Utilities/FBXCTestDaemonsProxy.m @@ -23,6 +23,7 @@ #import "XCUIDevice.h" #define LAUNCH_APP_TIMEOUT_SEC 300 +#define STOP_SCREEN_RECORDING_TIMEOUT_SEC 20 static void (*originalLaunchAppMethod)(id, SEL, NSString*, NSString*, NSArray*, NSDictionary*, void (^)(_Bool, NSError *)); @@ -342,14 +343,16 @@ + (BOOL)stopScreenRecordingWithUUID:(NSUUID *)uuid error:(NSError *__autoreleasi } __block NSError *innerError = nil; - [FBRunLoopSpinner spinUntilCompletion:^(void(^completion)(void)){ - [session stopScreenRecordingWithUUID:uuid withReply:^(NSError *invokeError) { - if (nil != invokeError) { - innerError = invokeError; - } - completion(); - }]; + dispatch_semaphore_t sem = dispatch_semaphore_create(0); + [session stopScreenRecordingWithUUID:uuid withReply:^(NSError *invokeError) { + innerError = invokeError; + dispatch_semaphore_signal(sem); }]; + int64_t timeoutNs = (int64_t)(STOP_SCREEN_RECORDING_TIMEOUT_SEC * NSEC_PER_SEC); + if (0 != dispatch_semaphore_wait(sem, dispatch_time(DISPATCH_TIME_NOW, timeoutNs)) && nil == innerError) { + NSString *message = [NSString stringWithFormat:@"Did not receive a reply to stop screen recording within %d seconds", STOP_SCREEN_RECORDING_TIMEOUT_SEC]; + innerError = [[[FBErrorBuilder builder] withDescription:message] build]; + } if (nil != innerError && error) { *error = innerError; } diff --git a/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m b/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m index e2fa95031..26cd2dc6e 100644 --- a/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m +++ b/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m @@ -77,32 +77,53 @@ + (BOOL)fb_areKeyEventsSupported @end +#define TESTMANAGERD_VERSION_TIMEOUT_SEC 20 + NSInteger FBTestmanagerdVersion(void) { - static dispatch_once_t getTestmanagerdVersion; - static NSInteger testmanagerdVersion; - dispatch_once(&getTestmanagerdVersion, ^{ + // Not dispatch_once: that would permanently cache the timeout fallback below if the first call's + // reply merely arrived late. -1 means "not yet determined"; a timeout isn't cached, so it retries. + static NSInteger cachedVersion = -1; + static dispatch_queue_t syncQueue; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + syncQueue = dispatch_queue_create("com.facebook.wda.testmanagerdVersion", DISPATCH_QUEUE_SERIAL); + }); + + __block NSInteger result; + dispatch_sync(syncQueue, ^{ + if (cachedVersion >= 0) { + result = cachedVersion; + return; + } + id proxy = [FBXCTestDaemonsProxy testRunnerProxy]; if ([(NSObject *)proxy respondsToSelector:@selector(_XCT_exchangeProtocolVersion:reply:)]) { id legacyProxy = (id)proxy; - [FBRunLoopSpinner spinUntilCompletion:^(void(^completion)(void)){ - [legacyProxy _XCT_exchangeProtocolVersion:testmanagerdVersion reply:^(unsigned long long code) { - testmanagerdVersion = (NSInteger) code; - completion(); - }]; + __block NSInteger receivedVersion = -1; + dispatch_semaphore_t sem = dispatch_semaphore_create(0); + [legacyProxy _XCT_exchangeProtocolVersion:0 reply:^(unsigned long long code) { + receivedVersion = (NSInteger) code; + dispatch_semaphore_signal(sem); }]; + int64_t timeoutNs = (int64_t)(TESTMANAGERD_VERSION_TIMEOUT_SEC * NSEC_PER_SEC); + if (0 != dispatch_semaphore_wait(sem, dispatch_time(DISPATCH_TIME_NOW, timeoutNs))) { + // Assume newest/full-featured on timeout, but don't cache it - retry on the next call. + [FBLogger logFmt:@"Did not receive a testmanagerd protocol version reply within %d seconds; assuming the newest/full-featured protocol", TESTMANAGERD_VERSION_TIMEOUT_SEC]; + result = 0xFFFF; + return; + } + result = receivedVersion; } else { - // Modern testmanagerd (Xcode 15+) has already negotiated named XCTCapabilities by the time - // a daemon session exists, instead of a single scalar protocol version. There is no direct - // integer equivalent to report here (this value is diagnostic-only, surfaced via the - // 'testmanagerdVersion' session capability), so keep reporting the existing "assume - // newest/full-featured" sentinel, while confirming capabilities did negotiate successfully. + // Modern testmanagerd (Xcode 15+) negotiates named XCTCapabilities instead of a scalar + // version; there's no direct integer equivalent, so just confirm capabilities negotiated. XCTCapabilities *capabilities = [XCTRunnerDaemonSession sharedSession].remoteInterfaceCapabilities; if (nil == capabilities) { [FBLogger log:@"Could not retrieve testmanagerd capabilities"]; } - testmanagerdVersion = 0xFFFF; + result = 0xFFFF; } + cachedVersion = result; }); - return testmanagerdVersion; + return result; }