fix: permission checks might freeze or restart the app (closes #5112)

src/experimentations/IOKit.swift

@@ -12,7 +12,7 @@ class IOKitPrototype {
             kIOHIDDeviceUsageKey: kHIDUsage_GD_Keyboard
         ] as CFDictionary)
         IOHIDManagerRegisterInputValueCallback(manager, keyboardEventHandler, nil)
-        IOHIDManagerScheduleWithRunLoop(manager, BackgroundWork.keyboardAndTrackpadEventsThread.runLoop!, CFRunLoopMode.commonModes.rawValue)
+        IOHIDManagerScheduleWithRunLoop(manager, BackgroundWork.keyboardAndMouseAndTrackpadEventsThread.runLoop!, CFRunLoopMode.commonModes.rawValue)
         IOHIDManagerOpen(manager, IOOptionBits(kIOHIDOptionsTypeNone))
     }
 }

src/logic/BackgroundWork.swift

@@ -2,24 +2,36 @@ import Foundation
 
 // queues and dedicated threads to observe background events such as keyboard inputs, or accessibility events
 class BackgroundWork {
-    // we use an OperationQueue tied to the global DispatchQueue to call blocking APIs in parallel
+    // we use Threads when the APIs we observe require a RunLoop (e.g. CGEvent.tapCreate, AXObserverGetRunLoopSource, CFMessagePortCreateRunLoopSource)
+    // when possible, we prefer OperationQueues
+    static var accessibilityEventsThread: BackgroundThreadWithRunLoop!
+    static var keyboardAndMouseAndTrackpadEventsThread: BackgroundThreadWithRunLoop!
+    static var missionControlThread: BackgroundThreadWithRunLoop!
+    static var cliEventsThread: BackgroundThreadWithRunLoop!
+
+    // TODO: use OperationQueue instead of thread
+    static var repeatingKeyThread: BackgroundThreadWithRunLoop!
+
+    // we use an OperationQueue for most tasks, especially when we need to call blocking APIs in parallel
     static var screenshotsQueue: LabeledOperationQueue!
     static var accessibilityCommandsQueue: LabeledOperationQueue!
     static var axCallsFirstAttemptQueue: LabeledOperationQueue!
     static var axCallsRetriesQueue: LabeledOperationQueue!
     static var axCallsManualDiscoveryQueue: LabeledOperationQueue!
     static var crashReportsQueue: LabeledOperationQueue!
-    // we use Threads to observe events in sequence
-    static var accessibilityEventsThread: BackgroundThreadWithRunLoop!
-    static var keyboardAndTrackpadEventsThread: BackgroundThreadWithRunLoop!
-    static var systemPermissionsThread: BackgroundThreadWithRunLoop!
-    static var repeatingKeyThread: BackgroundThreadWithRunLoop!
-    static var missionControlThread: BackgroundThreadWithRunLoop!
-    static var cliEventsThread: BackgroundThreadWithRunLoop!
-    static var debugMenu: DebugMenu!
+    static var systemPermissionsCheckOnTimerQueue: LabeledOperationQueue!
+    static var systemPermissionsSystemCallsQueue: LabeledOperationQueue!
 
+    private static var debugMenu: DebugMenu!
     private static var totalPotentialThreadCount = 0
 
+    static func preStart() {
+        // we make calls to the system permissions API to know if permissions are granted. We do this on a timer
+        systemPermissionsCheckOnTimerQueue = LabeledOperationQueue("systemPermissionsCheckOnTimerQueue", .userInteractive, 1)
+        // if macOS is overwhelmed, let's reduce the pressure on it by calling permission APIs one at a time
+        systemPermissionsSystemCallsQueue = LabeledOperationQueue("systemPermissionsSystemCallsQueue", .userInteractive, 1)
+    }
+
     static func start() {
         screenshotsQueue = LabeledOperationQueue("screenshotsQueue", .userInteractive, 8)
         // calls to focus/close/minimize/etc windows
@@ -34,7 +46,7 @@ class BackgroundWork {
         // we observe app and windows notifications. They arrive on this thread, and are handled off the main thread initially
         accessibilityEventsThread = BackgroundThreadWithRunLoop("accessibilityEventsThread", .userInteractive)
         // we listen to as any keyboard events as possible on a background thread, as it's more available/reliable than the main thread
-        keyboardAndTrackpadEventsThread = BackgroundThreadWithRunLoop("keyboardAndTrackpadEventsThread", .userInteractive)
+        keyboardAndMouseAndTrackpadEventsThread = BackgroundThreadWithRunLoop("keyboardAndMouseAndTrackpadEventsThread", .userInteractive)
         // we time key repeat on a background thread for precision. We handle their consequence on the main-thread
         repeatingKeyThread = BackgroundThreadWithRunLoop("repeatingKeyThread", .userInteractive)
         // we main Mission Control state on a background thread. We protect reads from main-thread with an NSLock
@@ -51,15 +63,10 @@ class BackgroundWork {
         }
     }
 
-    static func startSystemPermissionThread() {
-        // not 100% sure this shouldn't be on the main-thread; it doesn't do anything except dispatch to main.async
-        systemPermissionsThread = BackgroundThreadWithRunLoop("systemPermissionsThread", .utility)
-    }
-
     static func addPotentialThreadCount(_ additionalCount: Int) {
         totalPotentialThreadCount += additionalCount
         // a macos process has a soft limit of 64 threads. We need to be careful to don't spawn too many threads through DispatchQueues
-        assert(totalPotentialThreadCount <= 43)
+        assert(totalPotentialThreadCount <= 45)
     }
 
     // useful during development to inspect how many threads are used by AltTab

src/logic/SystemPermissions.swift

@@ -4,58 +4,56 @@ import ScreenCaptureKit.SCShareableContent
 // macOS has some privacy restrictions. The user needs to grant certain permissions, app by app, in System Preferences > Security & Privacy
 class SystemPermissions {
     static var preStartupPermissionsPassed = false
-    static var timerPermissionsToUpdatePermissionsWindow: Timer?
-    static var timerPermissionsRemovedWhileAltTabIsRunning: Timer?
+    private static var timer: DispatchSourceTimer!
 
-    static func ensurePermissionsAreGranted(_ continueAppStartup: @escaping () -> Void) {
-        let startupBlock = {
-            pollPermissionsRemovedWhileAltTabIsRunning()
-            continueAppStartup()
-        }
-        if AccessibilityPermission.update() != .notGranted && ScreenRecordingPermission.update() != .notGranted {
-            preStartupPermissionsPassed = true
-            startupBlock()
+    static func ensurePermissionsAreGranted() {
+        timer = DispatchSource.makeTimerSource(queue: BackgroundWork.systemPermissionsCheckOnTimerQueue.strongUnderlyingQueue)
+        timer.setEventHandler(handler: checkPermissionsOnTimer)
+        setImmediateTimer()
+        timer.resume()
+    }
+
+    private static func checkPermissionsOnTimer() {
+        AccessibilityPermission.update()
+        ScreenRecordingPermission.update()
+        if !preStartupPermissionsPassed {
+            checkPermissionsPreStartup()
         } else {
-            App.app.permissionsWindow.show(startupBlock)
+            checkPermissionsPostStartup()
+        }
+        DispatchQueue.main.async {
+            Menubar.togglePermissionCallout(ScreenRecordingPermission.status == .skipped)
+            App.app.permissionsWindow.updatePermissionViews()
         }
     }
 
-    static func pollPermissionsToUpdatePermissionsWindow(_ startupBlock: @escaping () -> Void) {
-        timerPermissionsToUpdatePermissionsWindow = Timer(timeInterval: 0.1, repeats: true) { _ in
-            checkPermissionsToUpdatePermissionsWindow(startupBlock)
-        }
-        timerPermissionsToUpdatePermissionsWindow!.tolerance = 0.1
-        CFRunLoopAddTimer(BackgroundWork.systemPermissionsThread.runLoop, timerPermissionsToUpdatePermissionsWindow!, .commonModes)
-    }
-
-    private static func pollPermissionsRemovedWhileAltTabIsRunning() {
-        timerPermissionsRemovedWhileAltTabIsRunning = Timer(timeInterval: 5, repeats: true) { _ in
+    private static func checkPermissionsPreStartup() {
+        Logger.info("accessibility:\(AccessibilityPermission.status), screen-recording:\(ScreenRecordingPermission.status)")
+        if AccessibilityPermission.status != .notGranted && ScreenRecordingPermission.status != .notGranted {
             DispatchQueue.main.async {
-                checkPermissionsWhileAltTabIsRunning()
+                preStartupPermissionsPassed = true
+                App.app.permissionsWindow?.close()
+                setInfrequentTimer()
+                App.app.continueAppLaunchAfterPermissionsAreGranted()
+            }
+        } else {
+            DispatchQueue.main.async {
+                App.app.permissionsWindow.show()
             }
         }
-        timerPermissionsRemovedWhileAltTabIsRunning!.tolerance = 1
-        CFRunLoopAddTimer(BackgroundWork.systemPermissionsThread.runLoop, timerPermissionsRemovedWhileAltTabIsRunning!, .commonModes)
     }
 
-    private static func checkPermissionsWhileAltTabIsRunning() {
-        Logger.debug(AccessibilityPermission.status, ScreenRecordingPermission.status, preStartupPermissionsPassed, ScreenRecordingPermission.status, Appearance.hideThumbnails, Preferences.previewFocusedWindow)
-        AccessibilityPermission.update()
-        Logger.debug(AccessibilityPermission.status)
+    private static func checkPermissionsPostStartup() {
+        Logger.debug("accessibility:\(AccessibilityPermission.status), screen-recording:\(ScreenRecordingPermission.status)")
         if AccessibilityPermission.status == .notGranted {
-            Logger.info("accessibilityPermission not granted; restarting")
-            App.app.restart()
+            Logger.error("accessibility permission revoked while AltTab was running; restarting")
+            DispatchQueue.main.async { App.app.restart() }
         }
-        if ScreenRecordingPermission.status == .skipped || (Appearance.hideThumbnails && !Preferences.previewFocusedWindow) { return }
-        ScreenRecordingPermission.update()
-        Logger.debug(ScreenRecordingPermission.status)
-        Menubar.togglePermissionCallout(ScreenRecordingPermission.status == .skipped)
         if ScreenRecordingPermission.status == .notGranted {
-            // permission check may yield a false negative during wake-up
-            // we restart after 2 negative checks
+            // permission check may yield a false negative during wake-up; we restart after 2 negative checks
             if ScreenRecordingPermission.flakyCounter >= 2 {
-                Logger.info("screenRecordingPermission not granted 3 times; restarting")
-                App.app.restart()
+                Logger.error("screen-recording permission revoked while AltTab was running (checked 2 times); restarting")
+                DispatchQueue.main.async { App.app.restart() }
             } else {
                 ScreenRecordingPermission.flakyCounter += 1
             }
@@ -64,30 +62,16 @@ class SystemPermissions {
         }
     }
 
-    private static func checkPermissionsToUpdatePermissionsWindow(_ startupBlock: @escaping () -> Void) {
-        AccessibilityPermission.update()
-        ScreenRecordingPermission.update()
-        Logger.debug(AccessibilityPermission.status, ScreenRecordingPermission.status, preStartupPermissionsPassed)
-        DispatchQueue.main.async {
-            Menubar.togglePermissionCallout(ScreenRecordingPermission.status == .skipped)
-            if AccessibilityPermission.status != App.app.permissionsWindow?.accessibilityView?.permissionStatus {
-                App.app.permissionsWindow?.accessibilityView.updatePermissionStatus(AccessibilityPermission.status)
-            }
-            if #available(macOS 10.15, *), ScreenRecordingPermission.status != App.app.permissionsWindow?.screenRecordingView?.permissionStatus {
-                App.app.permissionsWindow?.screenRecordingView?.updatePermissionStatus(ScreenRecordingPermission.status)
-            }
-            if !preStartupPermissionsPassed {
-                if AccessibilityPermission.status != .notGranted && ScreenRecordingPermission.status != .notGranted {
-                    preStartupPermissionsPassed = true
-                    App.app.permissionsWindow?.close()
-                    startupBlock()
-                }
-            } else {
-                if AccessibilityPermission.status == .notGranted || ScreenRecordingPermission.status == .notGranted {
-                    App.app.restart()
-                }
-            }
-        }
+    static func setInfrequentTimer() {
+        timer.schedule(deadline: .now() + 5, repeating: 5, leeway: .seconds(1))
+    }
+
+    static func setFrequentTimer() {
+        timer.schedule(deadline: .now() + 0.5, repeating: 0.5, leeway: .milliseconds(500))
+    }
+
+    private static func setImmediateTimer() {
+        timer.schedule(deadline: .now(), repeating: .never, leeway: .never)
     }
 }
 
@@ -176,12 +160,15 @@ class ScreenRecordingPermission {
     private static func runWithTimeout(_ block: @escaping (@escaping (Bool) -> Void) -> Void) -> Bool {
         let semaphore = DispatchSemaphore(value: 0)
         var result = false
-        block { r in
-            result = r
-            semaphore.signal()
+        BackgroundWork.systemPermissionsSystemCallsQueue.addOperation {
+            block { r in
+                result = r
+                semaphore.signal()
+            }
         }
-        let timeoutResult = semaphore.wait(timeout: .now() + 1)
+        let timeoutResult = semaphore.wait(timeout: .now() + 6)
         if timeoutResult == .timedOut {
+            Logger.error("screen-recording permission call timed out after 6s")
             return false
         }
         return result

src/logic/events/KeyboardEvents.swift

@@ -97,7 +97,7 @@ class KeyboardEvents {
             userInfo: nil)
         if let eventTap {
             let runLoopSource = CFMachPortCreateRunLoopSource(nil, eventTap, 0)
-            CFRunLoopAddSource(BackgroundWork.keyboardAndTrackpadEventsThread.runLoop, runLoopSource, .commonModes)
+            CFRunLoopAddSource(BackgroundWork.keyboardAndMouseAndTrackpadEventsThread.runLoop, runLoopSource, .commonModes)
         } else {
             App.app.restart()
         }

src/logic/events/ScrollwheelEvents.swift

@@ -28,7 +28,7 @@ class ScrollwheelEvents {
             userInfo: nil)
         if let eventTap {
             let runLoopSource = CFMachPortCreateRunLoopSource(nil, eventTap, 0)
-            CFRunLoopAddSource(BackgroundWork.keyboardAndTrackpadEventsThread.runLoop, runLoopSource, .commonModes)
+            CFRunLoopAddSource(BackgroundWork.keyboardAndMouseAndTrackpadEventsThread.runLoop, runLoopSource, .commonModes)
         } else {
             App.app.restart()
         }

src/logic/events/TrackpadEvents.swift

@@ -30,7 +30,7 @@ class TrackpadEvents {
             userInfo: nil)
         if let eventTap {
             let runLoopSource = CFMachPortCreateRunLoopSource(nil, eventTap, 0)
-            CFRunLoopAddSource(BackgroundWork.keyboardAndTrackpadEventsThread.runLoop, runLoopSource, .commonModes)
+            CFRunLoopAddSource(BackgroundWork.keyboardAndMouseAndTrackpadEventsThread.runLoop, runLoopSource, .commonModes)
         } else {
             App.app.restart()
         }

src/ui/App.swift

@@ -124,7 +124,7 @@ class App: AppCenterApplication {
     }
 
     @objc func checkPermissions(_ sender: NSMenuItem) {
-        permissionsWindow.show({})
+        permissionsWindow.show()
     }
 
     @objc func supportProject() {
@@ -287,35 +287,36 @@ extension App: NSApplicationDelegate {
         #endif
         AXUIElement.setGlobalTimeout()
         Preferences.initialize()
-        BackgroundWork.startSystemPermissionThread()
         permissionsWindow = PermissionsWindow()
-        SystemPermissions.ensurePermissionsAreGranted { [weak self] in
-            guard let self else { return }
-            Logger.info("System Permissions are granted")
-            BackgroundWork.start()
-            NSScreen.updatePreferred()
-            Appearance.update()
-            Menubar.initialize()
-            MainMenu.loadFromXib()
-            self.thumbnailsPanel = ThumbnailsPanel()
-            self.previewPanel = PreviewPanel()
-            Spaces.refresh()
-            SpacesEvents.observe()
-            ScreensEvents.observe()
-            SystemAppearanceEvents.observe()
-            SystemScrollerStyleEvents.observe()
-            Applications.initialDiscovery()
-            self.preferencesWindow = PreferencesWindow()
-            self.feedbackWindow = FeedbackWindow()
-            KeyboardEvents.addEventHandlers()
-            MouseEvents.observe()
-            TrackpadEvents.observe()
-            CliEvents.observe()
-            Logger.info("AltTab finished launching")
-            #if DEBUG
+        BackgroundWork.preStart()
+        SystemPermissions.ensurePermissionsAreGranted()
+    }
+
+    func continueAppLaunchAfterPermissionsAreGranted() {
+        Logger.info("system permissions are granted")
+        BackgroundWork.start()
+        NSScreen.updatePreferred()
+        Appearance.update()
+        Menubar.initialize()
+        MainMenu.loadFromXib()
+        self.thumbnailsPanel = ThumbnailsPanel()
+        self.previewPanel = PreviewPanel()
+        Spaces.refresh()
+        SpacesEvents.observe()
+        ScreensEvents.observe()
+        SystemAppearanceEvents.observe()
+        SystemScrollerStyleEvents.observe()
+        Applications.initialDiscovery()
+        self.preferencesWindow = PreferencesWindow()
+        self.feedbackWindow = FeedbackWindow()
+        KeyboardEvents.addEventHandlers()
+        MouseEvents.observe()
+        TrackpadEvents.observe()
+        CliEvents.observe()
+        Logger.info("AltTab finished launching")
+        #if DEBUG
 //            self.showPreferencesWindow()
-            #endif
-        }
+        #endif
     }
 
     func applicationShouldHandleReopen(_ sender: NSApplication, hasVisibleWindows flag: Bool) -> Bool {

src/ui/Menubar.swift

@@ -4,6 +4,7 @@ class Menubar {
     static var statusItem: NSStatusItem!
     static var menu: NSMenu!
     static var permissionCalloutMenuItems: [NSMenuItem]?
+    static var isShowingPermissionCallout = false
 
     static func addMenuItem(_ title: String, _ action: Selector, _ keyEquivalent: String, _ symbolName: String?, _ color: NSColor? = nil) {
         let item = menu.addItem(withTitle: title, action: action, keyEquivalent: keyEquivalent)
@@ -41,6 +42,8 @@ class Menubar {
 
     // NSMenuItem.isHidden isn't reliable with custom views. We add/remove to hide/show these items
     static func togglePermissionCallout(_ show: Bool) {
+        guard isShowingPermissionCallout != show else { return }
+        isShowingPermissionCallout = show
         permissionCalloutMenuItems?.enumerated().forEach { offset, element in
             if show && !menu.items.contains(element) {
                 menu.insertItem(element, at: offset)

src/ui/permission-window/PermissionView.swift

@@ -43,6 +43,7 @@ class PermissionView: StackView {
     }
 
     func updatePermissionStatus(_ permissionStatus: PermissionStatus) {
+        guard status.stringValue.isEmpty || permissionStatus != self.permissionStatus else { return }
         self.permissionStatus = permissionStatus
         var color: NSColor
         var label: String

src/ui/permission-window/PermissionsWindow.swift

@@ -13,19 +13,22 @@ class PermissionsWindow: NSWindow {
         setupView()
     }
 
-    func show(_ startupBlock: @escaping () -> Void) {
-        accessibilityView.updatePermissionStatus(AccessibilityPermission.update())
-        if #available(macOS 10.15, *) {
-            screenRecordingView.updatePermissionStatus(ScreenRecordingPermission.update())
-        }
+    func show() {
+        guard !isVisible else { return }
+        Logger.debug()
+        SystemPermissions.setFrequentTimer()
+        updatePermissionViews()
         center()
         App.shared.activate(ignoringOtherApps: true)
         makeKeyAndOrderFront(nil)
-        if #available(macOS 10.15, *), !SystemPermissions.preStartupPermissionsPassed {
-            // this call triggers the permission prompt, however it's the only way to force the app to be listed with a checkbox
-            SLSRequestScreenCaptureAccess()
+    }
+
+    func updatePermissionViews() {
+        guard isVisible else { return }
+        accessibilityView.updatePermissionStatus(AccessibilityPermission.status)
+        if #available(macOS 10.15, *) {
+            screenRecordingView.updatePermissionStatus(ScreenRecordingPermission.status)
         }
-        SystemPermissions.pollPermissionsToUpdatePermissionsWindow(startupBlock)
     }
 
     private func setupWindow() {
@@ -84,17 +87,18 @@ class PermissionsWindow: NSWindow {
 }
 
 extension PermissionsWindow: NSWindowDelegate {
-    func windowWillClose(_ notification: Notification) {
-        Logger.debug(SystemPermissions.preStartupPermissionsPassed)
+    func windowShouldClose(_ sender: NSWindow) -> Bool {
+        Logger.debug("preStartupPermissionsPassed:\(SystemPermissions.preStartupPermissionsPassed), accessibility:\(AccessibilityPermission.status), screen-recording:\(ScreenRecordingPermission.status)")
         if !SystemPermissions.preStartupPermissionsPassed {
-            if AccessibilityPermission.update() == .notGranted || ScreenRecordingPermission.update() == .notGranted {
+            if AccessibilityPermission.status == .notGranted || ScreenRecordingPermission.status == .notGranted {
                 Logger.error("Before using this app, you need to give permission in System Settings > Privacy & Security > Accessibility.",
                     "Please authorize and re-launch.",
                     "See https://help.rescuetime.com/article/59-how-do-i-enable-accessibility-permissions-on-mac-osx")
                 App.shared.terminate(self)
+                return false // prevent the close; termination will close everything once
             }
-        } else {
-            SystemPermissions.timerPermissionsToUpdatePermissionsWindow?.invalidate()
         }
+        SystemPermissions.setInfrequentTimer()
+        return true
     }
 }