diff --git a/WebDriverAgentLib/Commands/FBSessionCommands.h b/WebDriverAgentLib/Commands/FBSessionCommands.h index 95f3f258f..ff1f4c58b 100644 --- a/WebDriverAgentLib/Commands/FBSessionCommands.h +++ b/WebDriverAgentLib/Commands/FBSessionCommands.h @@ -14,6 +14,13 @@ NS_ASSUME_NONNULL_BEGIN @interface FBSessionCommands : NSObject +/** + Device properties served by /status (OS name, OS version, device kind), snapshotted once + behind a dispatch_once. /status runs off the main queue while UIDevice is formally + main-thread-only UIKit API, so FBWebServer burns the once-token on the main thread at startup. + */ ++ (NSDictionary *)cachedDeviceInfo; + @end NS_ASSUME_NONNULL_END diff --git a/WebDriverAgentLib/Commands/FBSessionCommands.m b/WebDriverAgentLib/Commands/FBSessionCommands.m index b23027daf..800c86ff4 100644 --- a/WebDriverAgentLib/Commands/FBSessionCommands.m +++ b/WebDriverAgentLib/Commands/FBSessionCommands.m @@ -193,15 +193,10 @@ + (NSArray *)routes [buildInfo setObject:version forKey:@"version"]; } -#if TARGET_OS_WATCH - NSString *osName = @"watchOS"; - NSString *osVersion = WKInterfaceDevice.currentDevice.systemVersion; - NSString *deviceKind = @"watch"; -#else - NSString *osName = [[UIDevice currentDevice] systemName]; - NSString *osVersion = [[UIDevice currentDevice] systemVersion]; - NSString *deviceKind = [self.class deviceNameByUserInterfaceIdiom:[UIDevice currentDevice].userInterfaceIdiom]; -#endif + NSDictionary *deviceInfo = [self.class cachedDeviceInfo]; + NSString *osName = deviceInfo[@"osName"]; + NSString *osVersion = deviceInfo[@"osVersion"]; + NSString *deviceKind = deviceInfo[@"deviceKind"]; return FBResponseWithObject( @{ @@ -447,6 +442,28 @@ + (NSDictionary *)sessionInformation }; } ++ (NSDictionary *)cachedDeviceInfo +{ + static NSDictionary *deviceInfo; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ +#if TARGET_OS_WATCH + deviceInfo = @{ + @"osName": @"watchOS", + @"osVersion": WKInterfaceDevice.currentDevice.systemVersion, + @"deviceKind": @"watch", + }; +#else + deviceInfo = @{ + @"osName": [[UIDevice currentDevice] systemName], + @"osVersion": [[UIDevice currentDevice] systemVersion], + @"deviceKind": [self deviceNameByUserInterfaceIdiom:[UIDevice currentDevice].userInterfaceIdiom], + }; +#endif + }); + return deviceInfo; +} + #if !TARGET_OS_WATCH /* Return the device kind as lower case diff --git a/WebDriverAgentLib/Routing/FBWebServer.m b/WebDriverAgentLib/Routing/FBWebServer.m index 4578f9993..738637f18 100644 --- a/WebDriverAgentLib/Routing/FBWebServer.m +++ b/WebDriverAgentLib/Routing/FBWebServer.m @@ -25,6 +25,7 @@ #import "FBRouteRequest.h" #import "FBRuntimeUtils.h" #import "FBSession.h" +#import "FBSessionCommands.h" #import "FBUnknownCommands.h" #import "FBConfiguration.h" #import "FBLogger.h" @@ -99,6 +100,11 @@ - (void)startServing { [FBLogger logFmt:@"Built at %s %s", __DATE__, __TIME__]; self.exceptionHandler = [FBExceptionHandler new]; + // Snapshot the /status device info on the main thread BEFORE the server binds: once it + // accepts connections, an early /status request could win the dispatch_once and run the + // formally main-thread-only UIDevice reads on its connection queue. Unlike the version + // pre-warms below, this is a cheap local read that cannot delay binding. + [FBSessionCommands cachedDeviceInfo]; if (![self startHTTPServer]) { return; } diff --git a/WebDriverAgentLib/Utilities/FBVideoStreamManager.m b/WebDriverAgentLib/Utilities/FBVideoStreamManager.m index 261d043bd..d8bae7aa0 100644 --- a/WebDriverAgentLib/Utilities/FBVideoStreamManager.m +++ b/WebDriverAgentLib/Utilities/FBVideoStreamManager.m @@ -30,6 +30,14 @@ static const NSTimeInterval FAILURE_BACKOFF_MAX = 10.0; static const char *QUEUE_NAME = "Screen Capture Encoder Queue"; +static NSString *const FBVideoStreamManagerErrorDomain = @"com.facebook.WebDriverAgent.FBVideoStreamManager"; +// No handler switches on these yet (they surface as unknown-error responses), but distinct +// codes keep the two start-failure modes distinguishable by more than the message text. +typedef NS_ENUM(NSInteger, FBVideoStreamManagerError) { + FBVideoStreamManagerErrorSessionLimitReached = 1, + FBVideoStreamManagerErrorStoppedWhileStarting = 2, +}; + @interface FBVideoStreamManager () @property (nonatomic) dispatch_queue_t backgroundQueue; @@ -83,15 +91,51 @@ - (nullable FBVideoStreamSession *)startSessionWithConfiguration:(FBScreenCaptur BOOL shouldStartLoop = NO; BOOL autoAssignPort = (0 == configuration.port); NSUInteger startGeneration; + // Snapshot the stop generation before the XCUIScreen read below, not inside the reservation + // lock: a stop-all that completes while this start is stuck in that lookup would otherwise be + // absorbed into the start's baseline and dodge both abort checks, letting a capture appear + // after the stop-all already returned success. @synchronized (self.sessions) { startGeneration = self.stopGeneration; + // Advisory fast-fail so a start that is already over the cap never reaches the XCUIScreen + // read: under an XCUI stall it would wedge the automation funnel despite never being + // eligible to start. The authoritative check runs in the reservation lock below, where + // concurrent starts that slipped in during the lookup are still counted. + if (self.sessions.count + self.pendingStarts >= MAX_SESSIONS) { + if (error) { + *error = [NSError errorWithDomain:FBVideoStreamManagerErrorDomain + code:FBVideoStreamManagerErrorSessionLimitReached + userInfo:@{NSLocalizedDescriptionKey: [NSString stringWithFormat:@"The maximum number of concurrent screen capture sessions (%@) has been reached", @(MAX_SESSIONS)]}]; + } + return nil; + } + } + + // Read before any state is reserved or bound: XCUIScreen goes through the automation + // machinery, and if it ever wedges it must strand nothing — no sessions-monitor hold (the + // control-marked capture routes stop/list/get/keyframe block on that lock and are supposed + // to stay wedge-immune), no pendingStarts reservation, and no bound-but-uninserted session + // that stopAllSessions cannot see or stop. + long long mainScreenID = [XCUIScreen.mainScreen displayID]; + + @synchronized (self.sessions) { + if (self.stopGeneration != startGeneration) { + // A stop-all ran to completion while this start was reading the display ID. Reject before + // reserving anything; the client's stop already promised this capture would not outlive it. + if (error) { + *error = [NSError errorWithDomain:FBVideoStreamManagerErrorDomain + code:FBVideoStreamManagerErrorStoppedWhileStarting + userInfo:@{NSLocalizedDescriptionKey: @"The screen capture session was stopped while it was starting"}]; + } + return nil; + } // Count in-flight starts toward the cap: their sessions are not inserted until after the // (slow) bind/encoder start, so without this two concurrent starts could both pass the check // and exceed MAX_SESSIONS. if (self.sessions.count + self.pendingStarts >= MAX_SESSIONS) { if (error) { - *error = [NSError errorWithDomain:@"com.facebook.WebDriverAgent.FBVideoStreamManager" - code:1 + *error = [NSError errorWithDomain:FBVideoStreamManagerErrorDomain + code:FBVideoStreamManagerErrorSessionLimitReached userInfo:@{NSLocalizedDescriptionKey: [NSString stringWithFormat:@"The maximum number of concurrent screen capture sessions (%@) has been reached", @(MAX_SESSIONS)]}]; } return nil; @@ -130,7 +174,7 @@ - (nullable FBVideoStreamSession *)startSessionWithConfiguration:(FBScreenCaptur } else { self.pendingStarts -= 1; self.sessions[@(identifier)] = session; - self.mainScreenID = [XCUIScreen.mainScreen displayID]; + self.mainScreenID = mainScreenID; if (!self.isStreaming) { self.isStreaming = YES; self.loopGeneration += 1; @@ -152,8 +196,8 @@ - (nullable FBVideoStreamSession *)startSessionWithConfiguration:(FBScreenCaptur if (abortedByStopAll) { [session stop]; if (error) { - *error = [NSError errorWithDomain:@"com.facebook.WebDriverAgent.FBVideoStreamManager" - code:1 + *error = [NSError errorWithDomain:FBVideoStreamManagerErrorDomain + code:FBVideoStreamManagerErrorStoppedWhileStarting userInfo:@{NSLocalizedDescriptionKey: @"The screen capture session was stopped while it was starting"}]; } return nil; diff --git a/WebDriverAgentLib/Utilities/FBVideoStreamSession.m b/WebDriverAgentLib/Utilities/FBVideoStreamSession.m index 45b3c3d6c..8a9387dba 100644 --- a/WebDriverAgentLib/Utilities/FBVideoStreamSession.m +++ b/WebDriverAgentLib/Utilities/FBVideoStreamSession.m @@ -112,8 +112,10 @@ + (BOOL)fb_pixelBudget:(NSUInteger *)outBudget fromArgument:(nullable id)maxPixe } // Validate the original numeric value: integerValue would silently truncate fractions // (0.5 -> 0 disables the cap; -0.5 -> 0 passes a sign check but converts to garbage). + // The range check must be >=: (double)NSUIntegerMax rounds up to 2^64, so > would accept + // exactly 2^64 and the NSUInteger cast below would overflow (undefined behavior). double rawBudget = ((NSNumber *)maxPixels).doubleValue; - if (!isfinite(rawBudget) || rawBudget < 0 || rawBudget != floor(rawBudget) || rawBudget > (double)NSUIntegerMax) { + if (!isfinite(rawBudget) || rawBudget < 0 || rawBudget != floor(rawBudget) || rawBudget >= (double)NSUIntegerMax) { return NO; } // 1..3 cannot be honored: 2x2 = 4 is the minimum encodable size. diff --git a/WebDriverAgentTests/UnitTests/FBRouteTests.m b/WebDriverAgentTests/UnitTests/FBRouteTests.m index 01e420e84..fe8ca3d2b 100644 --- a/WebDriverAgentTests/UnitTests/FBRouteTests.m +++ b/WebDriverAgentTests/UnitTests/FBRouteTests.m @@ -201,7 +201,7 @@ + (NSArray *)routes while (depth > prevMax && !atomic_compare_exchange_weak(&gSpinningProbeMaxDepth, &prevMax, depth)) { // retry until either our depth is recorded or another thread recorded a higher one } - [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.4]]; + [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1.0]]; atomic_fetch_sub(&gSpinningProbeDepth, 1); atomic_fetch_add(&gSpinningProbeCompletions, 1); return FBResponseWithOK(); @@ -310,8 +310,12 @@ - (void)testAutomationRequestsDoNotNestInsideRunLoopSpin // handler; without the automation funnel a nested run loop drain would let the second // handler execute reentrantly inside the first (depth 2). With the funnel, the second // request blocks on its own connection queue until the first finishes on main (depth 1). + // The 0.3 s gap lets the first request get through the server and enqueued at the funnel + // before the second fires, even on a loaded runner (the handler itself only starts once the + // wait loop below spins the run loop), while the probe's 1.0 s spin keeps the second request + // well inside the first's spin window. [self fireRequestForPath:@"/probe/spinning"]; - [NSThread sleepForTimeInterval:0.1]; + [NSThread sleepForTimeInterval:0.3]; [self fireRequestForPath:@"/probe/spinning"]; NSDate *deadline = [NSDate dateWithTimeIntervalSinceNow:10.0]; diff --git a/WebDriverAgentTests/UnitTests/FBVideoStreamSessionTests.m b/WebDriverAgentTests/UnitTests/FBVideoStreamSessionTests.m index 47134e8a3..4ca21a7b1 100644 --- a/WebDriverAgentTests/UnitTests/FBVideoStreamSessionTests.m +++ b/WebDriverAgentTests/UnitTests/FBVideoStreamSessionTests.m @@ -206,6 +206,10 @@ - (void)testPixelBudgetArgumentParsingRejectsMalformedValues XCTAssertFalse([FBScreenCaptureConfiguration fb_pixelBudget:&budget fromArgument:@(2) deviceDefault:0]); XCTAssertFalse([FBScreenCaptureConfiguration fb_pixelBudget:&budget fromArgument:@(NAN) deviceDefault:0]); XCTAssertFalse([FBScreenCaptureConfiguration fb_pixelBudget:&budget fromArgument:@(INFINITY) deviceDefault:0]); + // (double)NSUIntegerMax rounds up to exactly 2^64, which does not fit in NSUInteger; the + // parser must reject it, not cast it (undefined behavior). + XCTAssertFalse([FBScreenCaptureConfiguration fb_pixelBudget:&budget fromArgument:@((double)NSUIntegerMax) deviceDefault:0]); + XCTAssertFalse([FBScreenCaptureConfiguration fb_pixelBudget:&budget fromArgument:@(0x1p64) deviceDefault:0]); } @end