From f4678cc2ab910627eb5ab2ccda472fdcf29a75a7 Mon Sep 17 00:00:00 2001 From: baiqing Date: Sun, 10 May 2026 01:29:22 +0800 Subject: [PATCH] fix(hotkey): MacHotkeyAdapter::shutdown stops CFRunLoop + disables tap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CFRunLoopRun() in run_listen_loop has no return path; when HotkeyMonitor::drop runs adapter.shutdown(), the macOS branch fell through to the empty default impl from the trait. So every preference- driven monitor swap (e.g. user changes hotkey trigger) leaked the listener thread + CGEventTap. Long-running sessions that cycle bindings would steadily accumulate background threads. Fix: - Add CFRunLoopStop FFI extern. - Add MacShutdownHandles { tap, runloop } shared via Arc between the listener thread (writes refs in run_listen_loop after CGEventTapCreate and CFRunLoopGetCurrent) and MacHotkeyAdapter (reads them in shutdown). - Implement MacHotkeyAdapter::shutdown — disable tap then stop runloop, in that order, so the OS stops dispatching before we tear down. take()s on the option locks make shutdown idempotent. - run_listen_loop now does Box::from_raw(context) after CFRunLoopRun returns so the listener thread releases the callback context before exiting. - Move tap re-enable in tap_callback (TAP_DISABLED_BY_TIMEOUT) to read through ctx.handles.tap (same lock the adapter releases on shutdown, consistent with the rest of the FFI thread-safety story). Mirrors the Windows adapter's PostThreadMessageW(WM_QUIT) shutdown pattern. Apple documents CGEventTapEnable + CFRunLoopStop as safe to call from any thread, captured in a SAFETY comment on the unsafe Send/Sync impl. Audit ID 3.1.1 (CONFIRMED 严重). Test: hotkey + coordinator + types + commands lib tests pass (31/31). Functional verification of the leak fix requires running the app and swapping bindings — to be done after PR ships. --- openless-all/app/src-tauri/src/hotkey.rs | 65 +++++++++++++++++++++--- 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/openless-all/app/src-tauri/src/hotkey.rs b/openless-all/app/src-tauri/src/hotkey.rs index 663fe889..9be0abea 100644 --- a/openless-all/app/src-tauri/src/hotkey.rs +++ b/openless-all/app/src-tauri/src/hotkey.rs @@ -292,14 +292,32 @@ mod platform { "hotkey hook 启动超时", run_listen_loop, )?; - listener.startup; Ok(Box::new(MacHotkeyAdapter { shared: listener.shared, + handles: listener.startup, })) } + /// Refs needed to stop the Mac CFRunLoop / CGEventTap from outside the listener + /// thread. Filled in by `run_listen_loop` once the tap is created and the runloop + /// reference is captured; consumed by `MacHotkeyAdapter::shutdown` when the + /// monitor is dropped (so a binding swap or app shutdown doesn't leak the + /// listener thread + tap). Cf. audit 3.1.1. + struct MacShutdownHandles { + tap: std::sync::Mutex>, + runloop: std::sync::Mutex>, + } + + // SAFETY: CfMachPortRef / CfRunLoopRef are CoreFoundation handles; the only + // operations we perform on them across threads are CGEventTapEnable and + // CFRunLoopStop, both of which Apple documents as safe to call from any + // thread. + unsafe impl Send for MacShutdownHandles {} + unsafe impl Sync for MacShutdownHandles {} + struct MacHotkeyAdapter { shared: Arc, + handles: Arc, } impl HotkeyAdapter for MacHotkeyAdapter { @@ -322,6 +340,19 @@ mod platform { fn reset_held_state(&self) { reset_shared_held_state(&self.shared); } + + fn shutdown(&self) { + // 顺序：先 disable tap 让 OS 不再向我们派发事件，然后 stop runloop + // 让 listener 线程从 CFRunLoopRun() 返回退出。take() 保证幂等。 + let tap = self.handles.tap.lock().ok().and_then(|mut g| g.take()); + if let Some(tap) = tap { + unsafe { CGEventTapEnable(tap, false) }; + } + let runloop = self.handles.runloop.lock().ok().and_then(|mut g| g.take()); + if let Some(rl) = runloop { + unsafe { CFRunLoopStop(rl) }; + } + } } // ── Raw CG/CF FFI ────────────────────────────────────────────────────── @@ -404,24 +435,35 @@ mod platform { fn CFRunLoopGetCurrent() -> CfRunLoopRef; fn CFRunLoopAddSource(rl: CfRunLoopRef, source: CfRunLoopSourceRef, mode: CfStringRef); fn CFRunLoopRun(); + fn CFRunLoopStop(rl: CfRunLoopRef); static kCFRunLoopCommonModes: CfStringRef; } struct CallbackContext { shared: Arc, tx: Sender, - tap: std::sync::Mutex>, + /// 与 MacHotkeyAdapter 共享的 (tap, runloop) refs。tap re-enable on + /// TAP_DISABLED_BY_TIMEOUT 走 handles.tap；adapter shutdown 也走这两个 lock。 + handles: Arc, } unsafe impl Send for CallbackContext {} unsafe impl Sync for CallbackContext {} - fn run_listen_loop(shared: Arc, tx: Sender, status_tx: StartupTx<()>) { + fn run_listen_loop( + shared: Arc, + tx: Sender, + status_tx: StartupTx>, + ) { let mask: CgEventMask = (1u64 << FLAGS_CHANGED) | (1u64 << KEY_DOWN); + let handles = Arc::new(MacShutdownHandles { + tap: std::sync::Mutex::new(None), + runloop: std::sync::Mutex::new(None), + }); let context = Box::into_raw(Box::new(CallbackContext { shared, tx, - tap: std::sync::Mutex::new(None), + handles: Arc::clone(&handles), })); unsafe { @@ -444,16 +486,20 @@ mod platform { ))); return; } - *(*context).tap.lock().unwrap() = Some(tap); + *handles.tap.lock().unwrap() = Some(tap); let source = CFMachPortCreateRunLoopSource(std::ptr::null(), tap, 0); let runloop = CFRunLoopGetCurrent(); + *handles.runloop.lock().unwrap() = Some(runloop); CFRunLoopAddSource(runloop, source, kCFRunLoopCommonModes); CGEventTapEnable(tap, true); log::info!("[hotkey] CGEventTap 已启动"); - let _ = status_tx.send(Ok(())); + let _ = status_tx.send(Ok(handles)); + // CFRunLoopRun 阻塞直到 CFRunLoopStop 被调用（由 MacHotkeyAdapter::shutdown + // 触发）。返回后 listener 线程清理 context 并自然退出。 CFRunLoopRun(); + let _ = Box::from_raw(context); } } @@ -470,7 +516,7 @@ mod platform { match event_type { TAP_DISABLED_BY_TIMEOUT | TAP_DISABLED_BY_USER_INPUT => { - if let Some(tap) = *ctx.tap.lock().unwrap() { + if let Some(tap) = *ctx.handles.tap.lock().unwrap() { unsafe { CGEventTapEnable(tap, true) }; } return event; @@ -622,7 +668,10 @@ mod platform { CallbackContext { shared, tx, - tap: std::sync::Mutex::new(None), + handles: Arc::new(MacShutdownHandles { + tap: std::sync::Mutex::new(None), + runloop: std::sync::Mutex::new(None), + }), }, rx, )