fix: implement a 2min timeout for unresponsive apps (closes #274)

lwouis · May 10, 2020 · 6eae5c4 · 6eae5c4
1 parent b0ff902
commit 6eae5c4
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 78 deletions.
diff --git a/src/api-wrappers/AXUIElement.swift b/src/api-wrappers/AXUIElement.swift
@@ -103,45 +103,31 @@ extension AXUIElement {
         AXUIElementPerformAction(self, kAXRaiseAction as CFString)
     }
 
-    func subscribeWithRetry(_ axObserver: AXObserver, _ notification: String, _ pointer: UnsafeMutableRawPointer?, _ callback: (() -> Void)? = nil, _ runningApplication: NSRunningApplication? = nil, _ wid: CGWindowID? = nil, _ attemptsCount: Int = 0) {
+    func subscribeWithRetry(_ axObserver: AXObserver, _ notification: String, _ pointer: UnsafeMutableRawPointer?, _ callback: (() -> Void)? = nil, _ runningApplication: NSRunningApplication? = nil, _ wid: CGWindowID? = nil, _ startTime: DispatchTime = DispatchTime.now()) {
         DispatchQueue.global(qos: .userInteractive).async { [weak self] () -> () in
             guard let self = self else { return }
-            DispatchQueue.main.async { () -> () in
-                // TODO: this code is probably wrong and should be reviewed
-                if let runningApplication = runningApplication, (!Applications.appsInSubscriptionRetryLoop.contains { $0 == String(runningApplication.processIdentifier) + String(notification) }) { return }
-                if let wid = wid, (!Windows.windowsInSubscriptionRetryLoop.contains { $0 == String(wid) + String(notification) }) { return }
-            }
+            // some apps return .isFinishedLaunching = true but will return .cannotComplete when we try to subscribe to them
+            // this happens for example when apps launch and have heavy loading to do (e.g. Gimp).
+            // we have no way to know if they are one day going to be letting us subscribe, so we timeout after 2 min
+            let timePassedInSeconds = Double(DispatchTime.now().uptimeNanoseconds - startTime.uptimeNanoseconds) / 1_000_000_000
+            if timePassedInSeconds > 120 { return }
             let result = AXObserverAddNotification(axObserver, self, notification as CFString, pointer)
-            self.handleSubscriptionAttempt(result, axObserver, notification, pointer, callback, runningApplication, wid, attemptsCount)
+            self.handleSubscriptionAttempt(result, axObserver, notification, pointer, callback, runningApplication, wid, startTime)
         }
     }
 
-    func handleSubscriptionAttempt(_ result: AXError, _ axObserver: AXObserver, _ notification: String, _ pointer: UnsafeMutableRawPointer?, _ callback: (() -> Void)?, _ runningApplication: NSRunningApplication?, _ wid: CGWindowID?, _ attemptsCount: Int) -> Void {
+    func handleSubscriptionAttempt(_ result: AXError, _ axObserver: AXObserver, _ notification: String, _ pointer: UnsafeMutableRawPointer?, _ callback: (() -> Void)?, _ runningApplication: NSRunningApplication?, _ wid: CGWindowID?, _ startTime: DispatchTime) -> Void {
         if result == .success || result == .notificationAlreadyRegistered {
             DispatchQueue.main.async { [weak self] () -> () in
                 callback?()
-                self?.stopRetries(runningApplication, wid, notification)
-            }
-        } else if result == .notificationUnsupported || result == .notImplemented {
-            DispatchQueue.main.async { [weak self] () -> () in
-                self?.stopRetries(runningApplication, wid, notification)
             }
-        } else {
+        } else if result != .notificationUnsupported && result != .notImplemented {
             DispatchQueue.global(qos: .userInteractive).asyncAfter(deadline: .now() + .milliseconds(10), execute: { [weak self] in
-                self?.subscribeWithRetry(axObserver, notification, pointer, callback, runningApplication, wid, attemptsCount + 1)
+                self?.subscribeWithRetry(axObserver, notification, pointer, callback, runningApplication, wid, startTime)
             })
         }
     }
 
-    func stopRetries(_ runningApplication: NSRunningApplication?, _ wid: CGWindowID?, _ notification: String) {
-        if let runningApplication = runningApplication {
-            Application.stopSubscriptionRetries(notification, runningApplication)
-        }
-        if let wid = wid {
-            Window.stopSubscriptionRetries(notification, wid)
-        }
-    }
-
     private func attribute<T>(_ key: String, _ type: T.Type) -> T? {
         var value: AnyObject?
         let result = AXUIElementCopyAttributeValue(self, key as CFString, &value)

diff --git a/src/logic/Application.swift b/src/logic/Application.swift
@@ -23,14 +23,6 @@ class Application: NSObject {
         return n
     }
 
-    // some apps never finish their subscription retry loop; they should be stopped to avoid infinite loop
-    static func stopSubscriptionRetries(_ notification: String, _ runningApplication: NSRunningApplication) {
-        let subscriptionToRemove: String = String(runningApplication.processIdentifier) + notification
-        Applications.appsInSubscriptionRetryLoop.removeAll { (subscription: String) -> Bool in
-            return subscription == subscriptionToRemove
-        }
-    }
-
     init(_ runningApplication: NSRunningApplication) {
         self.runningApplication = runningApplication
         super.init()
@@ -42,8 +34,6 @@ class Application: NSObject {
     }
 
     deinit {
-        // some apps never finish launching; subscription retries should be stopped to avoid infinite loops
-        Application.notifications(runningApplication).forEach { Application.stopSubscriptionRetries($0, runningApplication) }
         // some apps never finish launching; observer should be removed to avoid leak
         removeObserver()
     }
@@ -88,7 +78,6 @@ class Application: NSObject {
         guard let axObserver = axObserver else { return }
         let selfPointer = UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())
         for notification in Application.notifications(runningApplication) {
-            Applications.appsInSubscriptionRetryLoop.append(String(runningApplication.processIdentifier) + String(notification))
             axUiElement!.subscribeWithRetry(axObserver, notification, selfPointer, { [weak self] in
                 // some apps have `isFinishedLaunching == true` but are actually not finished, and will return .cannotComplete
                 // we consider them ready when the first subscription succeeds, and list their windows again at that point

diff --git a/src/logic/Applications.swift b/src/logic/Applications.swift
@@ -3,7 +3,6 @@ import ApplicationServices
 
 class Applications {
     static var list = [Application]()
-    static var appsInSubscriptionRetryLoop = [String]()
 
     static func observeNewWindows() {
         for app in list {

diff --git a/src/logic/DebugProfile.swift b/src/logic/DebugProfile.swift
@@ -6,7 +6,6 @@ class DebugProfile {
     static let interSeparator = ", "
     static let bulletPoint = "* "
     static let nestedSeparator = "\n  " + bulletPoint
-    static let subscriptionRetriesRegex = try! NSRegularExpression(pattern: "[^0-9]+")
 
     static func make() -> String {
         let tuples: [(String, String)] = [
@@ -15,8 +14,6 @@ class DebugProfile {
             ("App preferences", appPreferences()),
             ("Applications", String(Applications.list.count)),
             ("Windows", listLevel2(Windows.list, appWindow)),
-            ("Apps subscription retries", listLevel2(Applications.appsInSubscriptionRetryLoop, subscriptionRetriesForApp)),
-            ("Windows subscription retries", listLevel2(Windows.windowsInSubscriptionRetryLoop, subscriptionRetriesForWindow)),
             // os
             ("OS version", ProcessInfo.processInfo.operatingSystemVersionString),
             ("OS architecture", Sysctl.run("hw.machine")),
@@ -58,32 +55,6 @@ class DebugProfile {
             + "}"
     }
 
-    static func subscriptionRetriesForWindow(_ subscriptionId: String) -> String {
-        let range = NSMakeRange(0, subscriptionId.count)
-        let widString = subscriptionRetriesRegex.stringByReplacingMatches(in: subscriptionId, range: range, withTemplate: "")
-        let wid = CGWindowID(truncating: NumberFormatter().number(from: widString)!)
-        let window = (CGWindowListCopyWindowInfo(.optionAll, wid) as! [CGWindow]).first!
-        return listLevel3([
-            ("wid", widString),
-            ("title", window.title() ?? "nil"),
-            ("ownerPID", window.ownerPID().flatMap { String($0) } ?? "nil"),
-            ("ownerName", window.ownerName() ?? "nil"),
-            ("layer", window.layer().flatMap { String($0) } ?? "nil"),
-        ])
-    }
-
-    static func subscriptionRetriesForApp(_ subscriptionId: String) -> String {
-        let range = NSMakeRange(0, subscriptionId.count)
-        let pidString = subscriptionRetriesRegex.stringByReplacingMatches(in: subscriptionId, range: range, withTemplate: "")
-        let pid = pid_t(truncating: NumberFormatter().number(from: pidString)!)
-        let app = NSRunningApplication(processIdentifier: pid)!
-        return listLevel3([
-            ("pid", pidString),
-            ("bundleIdentifier", app.bundleIdentifier ?? "nil"),
-            ("bundleURL", app.bundleURL?.path ?? "nil"),
-        ])
-    }
-
     private static func appPreferences() -> String {
         nestedSeparator + Preferences.all
             .sorted { $0.0 < $1.0 }

diff --git a/src/logic/Window.swift b/src/logic/Window.swift
@@ -25,12 +25,6 @@ class Window {
         kAXWindowResizedNotification,
     ]
 
-    static func stopSubscriptionRetries(_ notification: String, _ cgWindowId: CGWindowID) {
-        Windows.windowsInSubscriptionRetryLoop.removeAll { (subscription: String) -> Bool in
-            subscription == String(cgWindowId) + notification
-        }
-    }
-
     init(_ axUiElement: AXUIElement, _ application: Application) {
         // TODO: make a efficient batched AXUIElementCopyMultipleAttributeValues call once for each window, and store the values
         self.axUiElement = axUiElement
@@ -48,16 +42,10 @@ class Window {
         observeEvents()
     }
 
-    deinit {
-        // some windows never finish launching; subscription retries should be stopped to avoid infinite loops
-        Window.notifications.forEach { Window.stopSubscriptionRetries($0, cgWindowId) }
-    }
-
     private func observeEvents() {
         AXObserverCreate(application.runningApplication.processIdentifier, axObserverCallback, &axObserver)
         guard let axObserver = axObserver else { return }
         for notification in Window.notifications {
-            Windows.windowsInSubscriptionRetryLoop.append(String(cgWindowId) + String(notification))
             axUiElement.subscribeWithRetry(axObserver, notification, nil, nil, nil, cgWindowId)
         }
         CFRunLoopAddSource(CFRunLoopGetCurrent(), AXObserverGetRunLoopSource(axObserver), .defaultMode)

diff --git a/src/logic/Windows.swift b/src/logic/Windows.swift
@@ -5,7 +5,6 @@ class Windows {
     static var list = [Window]()
     static var previousFocusedWindowIndex = Int(0)
     static var focusedWindowIndex = Int(0)
-    static var windowsInSubscriptionRetryLoop = [String]()
 
     static func updateFocusedWindowIndex(_ newIndex: Int) {
         previousFocusedWindowIndex = focusedWindowIndex