diff --git a/WebDriverAgentLib/Categories/XCAXClient_iOS+FBSnapshotReqParams.m b/WebDriverAgentLib/Categories/XCAXClient_iOS+FBSnapshotReqParams.m index 8698eb734..20d6baa31 100644 --- a/WebDriverAgentLib/Categories/XCAXClient_iOS+FBSnapshotReqParams.m +++ b/WebDriverAgentLib/Categories/XCAXClient_iOS+FBSnapshotReqParams.m @@ -10,6 +10,13 @@ #import +#import "FBConfiguration.h" +#import "FBErrorBuilder.h" +#import "FBLogger.h" +#import "FBXCAccessibilityElement.h" +#import "FBXCAXClientProxy.h" +#import "XCUIApplication.h" + /** Available parameters with their default values for XCTest: @"maxChildren" : (int)2147483647 @@ -60,6 +67,82 @@ static id swizzledSnapshotParameters(id self, SEL _cmd) return result; } +static id (*original_requestSnapshotForElement)(id, SEL, id, id, id, NSError **); + +// pid -> last-unresponsive-at. XCTest retries a failed snapshot request several +// times in a row; this lets retries fail fast within `timeout` of the last check +// instead of each re-running the full wait. +static NSMutableDictionary *unresponsiveApplicationPids; +static NSObject *unresponsiveApplicationPidsLock; + +static NSError *FBBuildUnresponsiveApplicationError(int pid, NSTimeInterval timeout) +{ + // https://github.com/appium/WebDriverAgent/issues/1210 + NSString *description = [NSString stringWithFormat: + @"The application with process identifier %d did not confirm its main run loop is " + @"responsive within %.1f second(s) and is likely in an unresponsive state. " + @"Aborting the accessibility snapshot request instead of risking an indefinite " + @"hang.", + pid, timeout]; + [FBLogger logFmt:@"%@", description]; + NSError *error; + [[[FBErrorBuilder builder] withDescription:description] buildError:&error]; + return error; +} + +// Guards -[XCAXClient_iOS requestSnapshotForElement:...] against hanging forever +// on an unresponsive app (#1210). If accessibilityDeadline > 0, checks run loop +// responsiveness first and aborts with an error instead of risking an unbounded +// wait; otherwise falls through to the original, unbounded behavior. +static id swizzledRequestSnapshotForElement(id self, SEL _cmd, id element, id attributes, id parameters, NSError **error) +{ + NSTimeInterval timeout = FBConfiguration.sharedInstance.accessibilityDeadline; + if (timeout < DBL_EPSILON) { + return original_requestSnapshotForElement(self, _cmd, element, attributes, parameters, error); + } + + int pid = [(id)element processIdentifier]; + XCUIApplication *application = [FBXCAXClientProxy.sharedClient monitoredApplicationWithProcessIdentifier:pid]; + if (nil == application) { + // Nothing to confirm responsiveness for (e.g. the system element) - fall + // through to the original behavior. + return original_requestSnapshotForElement(self, _cmd, element, attributes, parameters, error); + } + + NSNumber *pidKey = @(pid); + @synchronized (unresponsiveApplicationPidsLock) { + NSDate *markedUnresponsiveAt = unresponsiveApplicationPids[pidKey]; + if (nil != markedUnresponsiveAt && -markedUnresponsiveAt.timeIntervalSinceNow < timeout) { + if (nil != error) { + *error = FBBuildUnresponsiveApplicationError(pid, timeout); + } + return nil; + } + } + + dispatch_semaphore_t sem = dispatch_semaphore_create(0); + __block BOOL isResponsive = NO; + [FBXCAXClientProxy.sharedClient notifyWhenEventLoopIsIdleForApplication:application + reply:^(id result, NSError *idleError) { + isResponsive = (nil == idleError); + dispatch_semaphore_signal(sem); + }]; + BOOL didReplyInTime = 0 == dispatch_semaphore_wait(sem, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeout * NSEC_PER_SEC))); + if (didReplyInTime && isResponsive) { + @synchronized (unresponsiveApplicationPidsLock) { + [unresponsiveApplicationPids removeObjectForKey:pidKey]; + } + return original_requestSnapshotForElement(self, _cmd, element, attributes, parameters, error); + } + + @synchronized (unresponsiveApplicationPidsLock) { + unresponsiveApplicationPids[pidKey] = [NSDate date]; + } + if (nil != error) { + *error = FBBuildUnresponsiveApplicationError(pid, timeout); + } + return nil; +} @implementation XCAXClient_iOS (FBSnapshotReqParams) @@ -69,6 +152,9 @@ @implementation XCAXClient_iOS (FBSnapshotReqParams) + (void)load { + unresponsiveApplicationPids = [NSMutableDictionary new]; + unresponsiveApplicationPidsLock = [NSObject new]; + Method original_defaultParametersMethod = class_getInstanceMethod(self.class, @selector(defaultParameters)); IMP swizzledDefaultParametersImp = (IMP)swizzledDefaultParameters; original_defaultParameters = (id (*)(id, SEL)) method_setImplementation(original_defaultParametersMethod, swizzledDefaultParametersImp); @@ -76,6 +162,10 @@ + (void)load Method original_snapshotParametersMethod = class_getInstanceMethod(NSClassFromString(@"XCTElementQuery"), NSSelectorFromString(@"snapshotParameters")); IMP swizzledSnapshotParametersImp = (IMP)swizzledSnapshotParameters; original_snapshotParameters = (id (*)(id, SEL)) method_setImplementation(original_snapshotParametersMethod, swizzledSnapshotParametersImp); + + Method original_requestSnapshotForElementMethod = class_getInstanceMethod(self.class, @selector(requestSnapshotForElement:attributes:parameters:error:)); + IMP swizzledRequestSnapshotForElementImp = (IMP)swizzledRequestSnapshotForElement; + original_requestSnapshotForElement = (id (*)(id, SEL, id, id, id, NSError **)) method_setImplementation(original_requestSnapshotForElementMethod, swizzledRequestSnapshotForElementImp); } #pragma clang diagnostic pop diff --git a/WebDriverAgentLib/Commands/FBCustomCommands.m b/WebDriverAgentLib/Commands/FBCustomCommands.m index 192c2e994..91a58fc2a 100644 --- a/WebDriverAgentLib/Commands/FBCustomCommands.m +++ b/WebDriverAgentLib/Commands/FBCustomCommands.m @@ -205,10 +205,13 @@ + (NSArray *)routes + (id)handleActiveAppInfo:(FBRouteRequest *)request { XCUIApplication *app = request.session.activeApplication ?: XCUIApplication.fb_activeApplication; + // .identifier can be nil if the app stopped answering accessibility requests + // and accessibilityDeadline aborted the underlying snapshot fetch (#1210). + NSString *name = app.identifier ?: @"unknown"; return FBResponseWithObject(@{ @"pid": @(app.processID), @"bundleId": app.bundleID, - @"name": app.identifier, + @"name": name, @"processArguments": [self processArguments:app], }); } diff --git a/WebDriverAgentLib/Commands/FBSessionCommands.m b/WebDriverAgentLib/Commands/FBSessionCommands.m index c03cdd552..f0b1a7324 100644 --- a/WebDriverAgentLib/Commands/FBSessionCommands.m +++ b/WebDriverAgentLib/Commands/FBSessionCommands.m @@ -311,6 +311,9 @@ + (void)applyConfigurationFromCapabilities:(NSDictionary *)capab if (nil != capabilities[FB_SETTING_WAIT_FOR_IDLE_TIMEOUT]) { FBConfiguration.sharedInstance.waitForIdleTimeout = [capabilities[FB_SETTING_WAIT_FOR_IDLE_TIMEOUT] doubleValue]; } + if (nil != capabilities[FB_SETTING_ACCESSIBILITY_DEADLINE]) { + FBConfiguration.sharedInstance.accessibilityDeadline = [capabilities[FB_SETTING_ACCESSIBILITY_DEADLINE] doubleValue]; + } if (nil == capabilities[FB_CAP_FORCE_SIMULATOR_SOFTWARE_KEYBOARD_PRESENCE] || [capabilities[FB_CAP_FORCE_SIMULATOR_SOFTWARE_KEYBOARD_PRESENCE] boolValue]) { [FBConfiguration.sharedInstance forceSimulatorSoftwareKeyboardPresence]; diff --git a/WebDriverAgentLib/Utilities/FBConfiguration.h b/WebDriverAgentLib/Utilities/FBConfiguration.h index 26f96655b..08b755b95 100644 --- a/WebDriverAgentLib/Utilities/FBConfiguration.h +++ b/WebDriverAgentLib/Utilities/FBConfiguration.h @@ -234,6 +234,17 @@ typedef NS_ENUM(NSInteger, FBConfigurationKeyboardPreference) { */ @property (atomic, assign) NSTimeInterval animationCoolOffTimeout; +/** + * Maximum time to wait for the frontmost application to confirm its main run loop + * is responsive before an accessibility snapshot request (element attribute + * lookups, active app detection, etc). XCTest has no bounded timeout of its own + * here, so a frozen app could otherwise block WDA forever (#1210); past this + * timeout the request is aborted with an error instead. + * Set to zero or negative to disable, restoring unbounded behavior. Disabled (0) + * by default. + */ +@property (atomic, assign) NSTimeInterval accessibilityDeadline; + /** Custom class chain locator for accept alert button location. This might be useful if the default buttons detection algorithm fails to determine alert buttons properly diff --git a/WebDriverAgentLib/Utilities/FBConfiguration.m b/WebDriverAgentLib/Utilities/FBConfiguration.m index 65bb37247..afbeaa4a3 100644 --- a/WebDriverAgentLib/Utilities/FBConfiguration.m +++ b/WebDriverAgentLib/Utilities/FBConfiguration.m @@ -354,6 +354,7 @@ - (void)resetSessionSettings self.autoClickAlertSelector = @""; self.waitForIdleTimeout = 10.; self.animationCoolOffTimeout = 2.; + self.accessibilityDeadline = 0.; // 50 should be enough for the majority of the cases. The performance is acceptable for values up to 100. FBSetCustomParameterForElementSnapshot(FBSnapshotMaxDepthKey, @50); FBSetCustomParameterForElementSnapshot(FBSnapshotMaxChildrenKey, @INT_MAX); diff --git a/WebDriverAgentLib/Utilities/FBSettings.h b/WebDriverAgentLib/Utilities/FBSettings.h index c3f1523e2..92a436db7 100644 --- a/WebDriverAgentLib/Utilities/FBSettings.h +++ b/WebDriverAgentLib/Utilities/FBSettings.h @@ -23,6 +23,7 @@ extern NSString* const FB_SETTING_KEYBOARD_AUTOCORRECTION; extern NSString* const FB_SETTING_KEYBOARD_PREDICTION; extern NSString* const FB_SETTING_SNAPSHOT_MAX_DEPTH; extern NSString* const FB_SETTING_SNAPSHOT_MAX_CHILDREN; +extern NSString* const FB_SETTING_ACCESSIBILITY_DEADLINE; extern NSString* const FB_SETTING_USE_FIRST_MATCH; extern NSString* const FB_SETTING_BOUND_ELEMENTS_BY_INDEX; extern NSString* const FB_SETTING_REDUCE_MOTION; diff --git a/WebDriverAgentLib/Utilities/FBSettings.m b/WebDriverAgentLib/Utilities/FBSettings.m index b2b219d85..826a22651 100644 --- a/WebDriverAgentLib/Utilities/FBSettings.m +++ b/WebDriverAgentLib/Utilities/FBSettings.m @@ -19,6 +19,7 @@ NSString* const FB_SETTING_KEYBOARD_PREDICTION = @"keyboardPrediction"; NSString* const FB_SETTING_SNAPSHOT_MAX_DEPTH = @"snapshotMaxDepth"; NSString* const FB_SETTING_SNAPSHOT_MAX_CHILDREN = @"snapshotMaxChildren"; +NSString* const FB_SETTING_ACCESSIBILITY_DEADLINE = @"accessibilityDeadline"; NSString* const FB_SETTING_USE_FIRST_MATCH = @"useFirstMatch"; NSString* const FB_SETTING_BOUND_ELEMENTS_BY_INDEX = @"boundElementsByIndex"; NSString* const FB_SETTING_REDUCE_MOTION = @"reduceMotion"; diff --git a/WebDriverAgentLib/Utilities/FBSettingsHandler.m b/WebDriverAgentLib/Utilities/FBSettingsHandler.m index c5110cc64..6184cab5b 100644 --- a/WebDriverAgentLib/Utilities/FBSettingsHandler.m +++ b/WebDriverAgentLib/Utilities/FBSettingsHandler.m @@ -142,6 +142,10 @@ @implementation FBSettingsHandler FBConfiguration.sharedInstance.animationCoolOffTimeout = [value doubleValue]; return nil; }; + map[FB_SETTING_ACCESSIBILITY_DEADLINE] = ^FBCommandStatus *(FBSession *session, id value) { + FBConfiguration.sharedInstance.accessibilityDeadline = [value doubleValue]; + return nil; + }; map[FB_SETTING_DEFAULT_ALERT_ACTION] = ^FBCommandStatus *(FBSession *session, id value) { if (nil == value) { session.defaultAlertAction = nil; @@ -249,6 +253,9 @@ @implementation FBSettingsHandler map[FB_SETTING_ANIMATION_COOL_OFF_TIMEOUT] = ^id(FBSession *session) { return @(FBConfiguration.sharedInstance.animationCoolOffTimeout); }; + map[FB_SETTING_ACCESSIBILITY_DEADLINE] = ^id(FBSession *session) { + return @(FBConfiguration.sharedInstance.accessibilityDeadline); + }; map[FB_SETTING_BOUND_ELEMENTS_BY_INDEX] = ^id(FBSession *session) { return @(FBConfiguration.sharedInstance.boundElementsByIndex); }; diff --git a/WebDriverAgentLib/Utilities/FBXCAXClientProxy.h b/WebDriverAgentLib/Utilities/FBXCAXClientProxy.h index b216e303f..208672af7 100644 --- a/WebDriverAgentLib/Utilities/FBXCAXClientProxy.h +++ b/WebDriverAgentLib/Utilities/FBXCAXClientProxy.h @@ -38,6 +38,15 @@ NS_ASSUME_NONNULL_BEGIN - (void)notifyWhenNoAnimationsAreActiveForApplication:(XCUIApplication *)application reply:(void (^)(void))reply; +/** + Wraps the private -[XCAXClient_iOS notifyWhenEventLoopIsIdleForApplication:reply:], + used to check run loop responsiveness before a snapshot request (#1210). + `reply` may fire more than once per call; `error` is non-nil only if monitoring + itself could not be started. + */ +- (void)notifyWhenEventLoopIsIdleForApplication:(XCUIApplication *)application + reply:(void (^)(id _Nullable result, NSError * _Nullable error))reply; + - (nullable NSDictionary *)attributesForElement:(id)element attributes:(NSArray *)attributes error:(NSError**)error; diff --git a/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m b/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m index f84f803fb..fc90d43f9 100644 --- a/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m +++ b/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m @@ -81,6 +81,12 @@ - (void)notifyWhenNoAnimationsAreActiveForApplication:(XCUIApplication *)applica [FBAXClient notifyWhenNoAnimationsAreActiveForApplication:application reply:reply]; } +- (void)notifyWhenEventLoopIsIdleForApplication:(XCUIApplication *)application + reply:(void (^)(id _Nullable result, NSError * _Nullable error))reply +{ + [FBAXClient notifyWhenEventLoopIsIdleForApplication:application reply:reply]; +} + - (NSDictionary *)attributesForElement:(id)element attributes:(NSArray *)attributes error:(NSError**)error; @@ -92,28 +98,30 @@ - (NSDictionary *)attributesForElement:(id)element - (XCUIApplication *)monitoredApplicationWithProcessIdentifier:(int)pid { - NSMutableSet *terminatedAppIds = [NSMutableSet set]; - for (NSNumber *appPid in self.appsCache) { - if (![self.appsCache[appPid] running]) { - [terminatedAppIds addObject:appPid]; + @synchronized (self) { + NSMutableSet *terminatedAppIds = [NSMutableSet set]; + for (NSNumber *appPid in self.appsCache) { + if (![self.appsCache[appPid] running]) { + [terminatedAppIds addObject:appPid]; + } + } + for (NSNumber *appPid in terminatedAppIds) { + [self.appsCache removeObjectForKey:appPid]; } - } - for (NSNumber *appPid in terminatedAppIds) { - [self.appsCache removeObjectForKey:appPid]; - } - XCUIApplication *result = [self.appsCache objectForKey:@(pid)]; - if (nil != result) { - return result; - } + XCUIApplication *result = [self.appsCache objectForKey:@(pid)]; + if (nil != result) { + return result; + } - XCUIApplication *app = [[FBAXClient applicationProcessTracker] - monitoredApplicationWithProcessIdentifier:pid]; - if (nil == app) { - return nil; + XCUIApplication *app = [[FBAXClient applicationProcessTracker] + monitoredApplicationWithProcessIdentifier:pid]; + if (nil == app) { + return nil; + } + [self.appsCache setObject:app forKey:@(pid)]; + return app; } - [self.appsCache setObject:app forKey:@(pid)]; - return app; } @end diff --git a/WebDriverAgentTests/IntegrationApp/Classes/ViewController.m b/WebDriverAgentTests/IntegrationApp/Classes/ViewController.m index 3bd1884ab..f63a91561 100644 --- a/WebDriverAgentTests/IntegrationApp/Classes/ViewController.m +++ b/WebDriverAgentTests/IntegrationApp/Classes/ViewController.m @@ -38,9 +38,10 @@ - (BOOL)handleCustomAction:(UIAccessibilityCustomAction *)action - (IBAction)deadlockApp:(id)sender { - dispatch_sync(dispatch_get_main_queue(), ^{ - // This will never execute - }); + // A self dispatch_sync would trip the OS watchdog and get the process + // killed outright. Sleeping instead simulates an app that stops answering + // accessibility requests while staying alive, per #1210. + [NSThread sleepForTimeInterval:20.0]; } - (IBAction)didTapButton:(UIButton *)button diff --git a/WebDriverAgentTests/IntegrationApp/Resources/Base.lproj/Main.storyboard b/WebDriverAgentTests/IntegrationApp/Resources/Base.lproj/Main.storyboard index ceeb4fb5b..7a9156769 100644 --- a/WebDriverAgentTests/IntegrationApp/Resources/Base.lproj/Main.storyboard +++ b/WebDriverAgentTests/IntegrationApp/Resources/Base.lproj/Main.storyboard @@ -38,7 +38,6 @@ -