From 694e1ff3db52f0ef8f6f961b827f371c86eb06a8 Mon Sep 17 00:00:00 2001 From: Bob Date: Tue, 15 Sep 2026 16:08:28 +0000 Subject: [PATCH] fix(macos): survive non-string AX titles and stop leaking AXObserver Accessibility can return non-NSString values for kAXTitleAttribute. Bridging those with `as? String` calls `-[obj length]` and aborts the helper (NSHTTPURLResponse in #144). Read titles via CF type checks. Also tear down the previous AXObserver properly: unregister the title and focused-window notifications, drop the run-loop source, and nil the observer before AXObserverCreate. Overwriting the CF out-pointer leaked Mach receive ports and wired memory (#139). If the helper still dies, propagate its signal/exit status so aw-tauri restarts window tracking instead of treating the crash as a clean shutdown. Git-Session-Id: 03164cc9-6bb0-5435-925f-f425e310dded --- aw_watcher_window/macos.swift | 100 +++++++++++++++++++++++++++------- aw_watcher_window/main.py | 21 ++++++- tests/test_main.py | 70 ++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 21 deletions(-) diff --git a/aw_watcher_window/macos.swift b/aw_watcher_window/macos.swift index 9eb97df..83a29ac 100644 --- a/aw_watcher_window/macos.swift +++ b/aw_watcher_window/macos.swift @@ -117,6 +117,32 @@ func error(_ msg: String) { fflush(stdout) } +// AX attributes are CF types. `as? String` uses String's ObjC bridge, which +// calls `-[obj length]` without a class check. A non-NSString then aborts the +// helper — the #144 crash was `-[NSHTTPURLResponse length]`. Check CF type first. +func axString(_ value: AnyObject?) -> String? { + guard let value = value else { return nil } + let cfValue = value as CFTypeRef + let typeID = CFGetTypeID(cfValue) + if typeID == CFStringGetTypeID() { + return (cfValue as! CFString) as String + } + if typeID == CFAttributedStringGetTypeID() { + return (value as! NSAttributedString).string + } + debug("Ignoring non-string AX value of type \(type(of: value))") + return nil +} + +func axElement(_ value: AnyObject?) -> AXUIElement? { + guard let value = value else { return nil } + if CFGetTypeID(value as CFTypeRef) == AXUIElementGetTypeID() { + return (value as! AXUIElement) + } + debug("Ignoring non-AXUIElement value of type \(type(of: value))") + return nil +} + // Placeholder values, set in start() from CLI arguments var baseurl = "http://localhost:5600" // NOTE: this differs from the hostname we get from Python, here we get `.local`, but in Python we get `.localdomain` @@ -461,6 +487,7 @@ func sendHeartbeatSingle(_ heartbeat: Heartbeat, pulsetime: Double) async throws class MainThing { var observer: AXObserver? + var observedApp: AXUIElement? var oldWindow: AXUIElement? var pollingTimer: Timer? @@ -505,7 +532,7 @@ class MainThing { var roleRef: AnyObject? AXUIElementCopyAttributeValue(element, kAXRoleAttribute as CFString, &roleRef) - if roleRef as? String == "AXWebArea" { + if axString(roleRef) == "AXWebArea" { var urlRef: AnyObject? AXUIElementCopyAttributeValue(element, kAXURLAttribute as CFString, &urlRef) if let url = urlRef as? NSURL { @@ -513,7 +540,7 @@ class MainThing { } // no URL on the web area (e.g. page still loading); stop rather than // keep searching, since a deeper hit would be an iframe's web area - return urlRef as? String + return axString(urlRef) } var childrenRef: AnyObject? @@ -528,6 +555,11 @@ class MainThing { @objc func pollActiveWindow() { debug("Polling active window") + guard let observer = observer else { + debug("Polling skipped: no accessibility observer") + return + } + guard let frontmost = NSWorkspace.shared.frontmostApplication else { log("Failed to get frontmost application from polling") return @@ -539,13 +571,14 @@ class MainThing { var focusedWindow: AnyObject? AXUIElementCopyAttributeValue(focusedApp, kAXFocusedWindowAttribute as CFString, &focusedWindow) - if focusedWindow != nil { - focusedWindowChanged(observer!, window: focusedWindow as! AXUIElement) + if let focusedWindow = axElement(focusedWindow) { + focusedWindowChanged(observer, window: focusedWindow) } } deinit { pollingTimer?.invalidate() + tearDownObserver() } func windowTitleChanged( @@ -565,7 +598,7 @@ class MainThing { AXUIElementCopyAttributeValue(axElement, kAXTitleAttribute as CFString, &windowTitle) let applicationName = frontmost.localizedName ?? frontmost.bundleIdentifier ?? "" - var data = NetworkMessage(app: applicationName, title: windowTitle as? String ?? "") + var data = NetworkMessage(app: applicationName, title: axString(windowTitle) ?? "") if CHROME_BROWSERS.contains(applicationName) { debug("Chrome browser detected, extracting URL and title") @@ -664,11 +697,35 @@ class MainThing { sendHeartbeat(heartbeat) } + func tearDownObserver() { + // Unregister notifications before dropping the observer. Removing only the + // run-loop source leaves the Mach receive port alive; AX events then queue + // up to qlimit (1024) and leak wired memory (#139). AXObserverCreate also + // overwrites the out-pointer without CFRelease, so the previous observer + // must be niled first. + if let previous = observer { + if let window = oldWindow { + AXObserverRemoveNotification(previous, window, kAXTitleChangedNotification as CFString) + } + if let app = observedApp { + AXObserverRemoveNotification(previous, app, kAXFocusedWindowChangedNotification as CFString) + } + CFRunLoopRemoveSource( + RunLoop.current.getCFRunLoop(), + AXObserverGetRunLoopSource(previous), + CFRunLoopMode.defaultMode + ) + } + oldWindow = nil + observedApp = nil + observer = nil + } + @objc func focusedWindowChanged(_ observer: AXObserver, window: AXUIElement) { debug("Focused window changed") - if oldWindow != nil { - AXObserverRemoveNotification(observer, oldWindow!, kAXFocusedWindowChangedNotification as CFString) + if let oldWindow = oldWindow { + AXObserverRemoveNotification(observer, oldWindow, kAXTitleChangedNotification as CFString) } let selfPtr = UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque()) @@ -682,14 +739,7 @@ class MainThing { @objc func focusedAppChanged() { debug("Focused app changed") - - if observer != nil { - CFRunLoopRemoveSource( - RunLoop.current.getCFRunLoop(), - AXObserverGetRunLoopSource(observer!), - CFRunLoopMode.defaultMode - ) - } + tearDownObserver() guard let frontmost = NSWorkspace.shared.frontmostApplication else { log("Failed to get frontmost application from app change notification") @@ -699,6 +749,7 @@ class MainThing { let pid = frontmost.processIdentifier let focusedApp = AXUIElementCreateApplication(pid) + var newObserver: AXObserver? AXObserverCreate( pid, { @@ -722,22 +773,31 @@ class MainThing { notification: notification ) } - }, &observer) + }, &newObserver) + + guard let newObserver = newObserver else { + log("Failed to create accessibility observer") + return + } + + observer = newObserver + observedApp = focusedApp let selfPtr = UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque()) - AXObserverAddNotification(observer!, focusedApp, kAXFocusedWindowChangedNotification as CFString, selfPtr) + AXObserverAddNotification( + newObserver, focusedApp, kAXFocusedWindowChangedNotification as CFString, selfPtr) CFRunLoopAddSource( RunLoop.current.getCFRunLoop(), - AXObserverGetRunLoopSource(observer!), + AXObserverGetRunLoopSource(newObserver), CFRunLoopMode.defaultMode ) var focusedWindow: AnyObject? AXUIElementCopyAttributeValue(focusedApp, kAXFocusedWindowAttribute as CFString, &focusedWindow) - if focusedWindow != nil { - focusedWindowChanged(observer!, window: focusedWindow as! AXUIElement) + if let focusedWindow = axElement(focusedWindow) { + focusedWindowChanged(newObserver, window: focusedWindow) } } } diff --git a/aw_watcher_window/main.py b/aw_watcher_window/main.py index 9d20740..26b9431 100644 --- a/aw_watcher_window/main.py +++ b/aw_watcher_window/main.py @@ -46,6 +46,22 @@ def kill_process(pid): logger.info("Process {} already dead".format(pid)) +def swift_helper_exit_status(returncode): + """Translate Popen.wait() into a process exit status for the module manager. + + A crashed Swift helper is killed by a signal (SIGABRT for uncaught + NSExceptions). Popen reports that as a negative returncode, but falling + off main() exits 0, so aw-tauri treats the crash as a clean shutdown and + does not restart window tracking (ActivityWatch/aw-watcher-window#144, + #101, #139). + """ + if not returncode: + return 0 + if returncode < 0: + return 128 + (-returncode) + return returncode + + def try_compile_title_regex(title): try: return re.compile(title, re.IGNORECASE) @@ -117,7 +133,10 @@ def main(): ) # terminate swift process when this process dies signal.signal(signal.SIGTERM, lambda *_: kill_process(p.pid)) - p.wait() + status = swift_helper_exit_status(p.wait()) + if status: + logger.error("Swift helper exited with status %s", status) + sys.exit(status) except KeyboardInterrupt: print("KeyboardInterrupt") kill_process(p.pid) diff --git a/tests/test_main.py b/tests/test_main.py index facda11..31f216c 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -213,3 +213,73 @@ def test_pulsetime_scales_with_poll_time( poll_time: float, expected_pulsetime: float ): assert main_module.compute_pulsetime(poll_time) == expected_pulsetime + + +@pytest.mark.parametrize( + "returncode,expected", + [ + (None, 0), + (0, 0), + (1, 1), + (-6, 134), # SIGABRT + (-11, 139), # SIGSEGV + (134, 134), + ], +) +def test_swift_helper_exit_status(returncode, expected): + assert main_module.swift_helper_exit_status(returncode) == expected + + +def test_swift_strategy_propagates_helper_crash(monkeypatch): + class FakeProcess: + pid = 123 + + def wait(self): + return -6 # SIGABRT from an uncaught NSException + + class FakeClient: + client_name = "aw-watcher-window" + client_hostname = "host.localdomain" + server_address = "http://localhost:5600" + + def __init__(self, *args, **kwargs): + pass + + def create_bucket(self, *args, **kwargs): + pass + + def wait_for_start(self): + pass + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + monkeypatch.setattr(main_module.sys, "platform", "darwin") + monkeypatch.setattr(main_module, "background_ensure_permissions", lambda: None) + monkeypatch.setattr(main_module, "setup_logging", lambda **kwargs: None) + monkeypatch.setattr(main_module, "ActivityWatchClient", FakeClient) + monkeypatch.setattr(main_module.signal, "signal", lambda *args, **kwargs: None) + monkeypatch.setattr(main_module.subprocess, "Popen", lambda command: FakeProcess()) + monkeypatch.setattr( + main_module, + "parse_args", + lambda: SimpleNamespace( + testing=True, + verbose=False, + host=None, + port=None, + strategy="swift", + exclude_title=False, + exclude_titles=[], + research_enabled=False, + research_category_map={}, + research_app_category_map={}, + ), + ) + + with pytest.raises(SystemExit) as exc: + main_module.main() + assert exc.value.code == 134