From 86192112a6fa086eda6856196b7a64dfe5946bb0 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Thu, 9 Jul 2026 14:36:28 +1000 Subject: [PATCH 1/2] macos: call `IOHIDManagerCopyDevices` before starting the `CFRunLoop` and notify about all existing devices at the same time (https://github.com/mozilla/authenticator-rs/issues/362) --- src/transport/macos/iokit.rs | 4 +- src/transport/macos/monitor.rs | 86 +++++++++++++++++++++++------- src/transport/macos/transaction.rs | 1 + 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/src/transport/macos/iokit.rs b/src/transport/macos/iokit.rs index 656cdb04..efeb5e42 100644 --- a/src/transport/macos/iokit.rs +++ b/src/transport/macos/iokit.rs @@ -12,6 +12,7 @@ use core_foundation::base::*; use core_foundation::dictionary::*; use core_foundation::number::*; use core_foundation::runloop::*; +use core_foundation::set::*; use core_foundation::string::*; use std::ops::Deref; use std::os::raw::c_void; @@ -52,7 +53,7 @@ pub struct __IOHIDManager { #[repr(C)] #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] -pub struct IOHIDDeviceRef(*const c_void); +pub struct IOHIDDeviceRef(pub(crate) *const c_void); unsafe impl Send for IOHIDDeviceRef {} unsafe impl Sync for IOHIDDeviceRef {} @@ -188,6 +189,7 @@ extern "C" { allocator: CFAllocatorRef, options: IOHIDManagerOptions, ) -> IOHIDManagerRef; + pub fn IOHIDManagerCopyDevices(manager: IOHIDManagerRef) -> CFSetRef; pub fn IOHIDManagerSetDeviceMatching(manager: IOHIDManagerRef, matching: CFDictionaryRef); pub fn IOHIDManagerRegisterDeviceMatchingCallback( manager: IOHIDManagerRef, diff --git a/src/transport/macos/monitor.rs b/src/transport/macos/monitor.rs index 32200ee7..8d9b5f89 100644 --- a/src/transport/macos/monitor.rs +++ b/src/transport/macos/monitor.rs @@ -10,6 +10,7 @@ use crate::transport::platform::iokit::*; use crate::util::io_err; use core_foundation::base::*; use core_foundation::runloop::*; +use core_foundation::set::*; use runloop::RunLoop; use std::collections::HashMap; use std::os::raw::c_void; @@ -50,6 +51,7 @@ where &dyn Fn() -> bool, ) + Send + Sync + + Clone + 'static, { pub fn new( @@ -100,12 +102,23 @@ where ); let rv = IOHIDManagerOpen(self.manager, kIOHIDManagerOptionNone); - if rv == 0 { - Ok(()) - } else { - Err(io_err(&format!("Couldn't open HID Manager, rv={rv}"))) + if rv != 0 { + return Err(io_err(&format!("Couldn't open HID Manager, rv={rv}"))); } } + + // Enumerate all existing devices simultaneously, like what Linux does. This mitigates + // against a race condition in DeviceSelector if there is a slow device. + let devices = self.get_devices(); + let _ = self + .selector_sender + .send(DeviceSelectorEvent::DevicesAdded(devices.clone())); + + for device in devices { + self.add_device(device); + } + + Ok(()) } pub fn stop(&mut self) { @@ -119,6 +132,26 @@ where unsafe { IOHIDManagerClose(self.manager, kIOHIDManagerOptionNone) }; } + /// Set up a runloop for a new device. + fn add_device(&mut self, device_ref: IOHIDDeviceRef) { + let selector_sender = self.selector_sender.clone(); + let status_sender = self.status_sender.clone(); + let (tx, rx) = channel(); + let f = self.new_device_cb.clone(); + + // Create a new per-device runloop. + let runloop = RunLoop::new(move |alive| { + // Ensure that the runloop is still alive. + if alive() { + f((device_ref, rx), selector_sender, status_sender, alive); + } + }); + + if let Ok(runloop) = runloop { + self.map.insert(device_ref, DeviceData { tx, runloop }); + } + } + fn remove_device(&mut self, device_ref: IOHIDDeviceRef) { if let Some(DeviceData { tx, runloop }) = self.map.remove(&device_ref) { let _ = self @@ -132,6 +165,27 @@ where } } + /// Get all currently-connected devices. + fn get_devices(&mut self) -> Vec { + unsafe { + let devices = IOHIDManagerCopyDevices(self.manager); + if devices.is_null() { + // IOHIDManagerCopyDevices returns null on zero devices (undocumented!) + return Vec::with_capacity(0); + } + + let s: CFSet = CFSet::wrap_under_get_rule(devices); + let mut refs: Vec = Vec::with_capacity(s.len()); + + CFSetGetValues( + s.as_concrete_TypeRef(), + refs.as_mut_ptr() as *mut *const c_void, + ); + refs.set_len(s.len()); + refs + } + } + extern "C" fn on_input_report( context: *mut c_void, _: IOReturn, @@ -163,25 +217,17 @@ where device_ref: IOHIDDeviceRef, ) { let this = unsafe { &mut *(context as *mut Self) }; + // IOHIDManager sends a DeviceMatchingCallback for every already-connected device, but + // get_devices will have already handled this. + if this.map.contains_key(&device_ref) { + debug!("Ignoring duplicate device: {device_ref:?}"); + return; + } + let _ = this .selector_sender .send(DeviceSelectorEvent::DevicesAdded(vec![device_ref])); - let selector_sender = this.selector_sender.clone(); - let status_sender = this.status_sender.clone(); - let (tx, rx) = channel(); - let f = &this.new_device_cb; - - // Create a new per-device runloop. - let runloop = RunLoop::new(move |alive| { - // Ensure that the runloop is still alive. - if alive() { - f((device_ref, rx), selector_sender, status_sender, alive); - } - }); - - if let Ok(runloop) = runloop { - this.map.insert(device_ref, DeviceData { tx, runloop }); - } + this.add_device(device_ref); } extern "C" fn on_device_removal( diff --git a/src/transport/macos/transaction.rs b/src/transport/macos/transaction.rs index d9709e73..f2e3ffc8 100644 --- a/src/transport/macos/transaction.rs +++ b/src/transport/macos/transaction.rs @@ -41,6 +41,7 @@ impl Transaction { &dyn Fn() -> bool, ) + Sync + Send + + Clone + 'static, T: 'static, { From f9606fd8cf4a7c37cd14184e3c2797ec2b0a1a44 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Wed, 15 Jul 2026 09:52:55 +1000 Subject: [PATCH 2/2] macos: fix `IOHIDManagerCopyDevices` leak, unneeded `mut` --- src/transport/macos/monitor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transport/macos/monitor.rs b/src/transport/macos/monitor.rs index 8d9b5f89..5dbd95e5 100644 --- a/src/transport/macos/monitor.rs +++ b/src/transport/macos/monitor.rs @@ -166,7 +166,7 @@ where } /// Get all currently-connected devices. - fn get_devices(&mut self) -> Vec { + fn get_devices(&self) -> Vec { unsafe { let devices = IOHIDManagerCopyDevices(self.manager); if devices.is_null() { @@ -174,7 +174,7 @@ where return Vec::with_capacity(0); } - let s: CFSet = CFSet::wrap_under_get_rule(devices); + let s: CFSet = CFSet::wrap_under_create_rule(devices); let mut refs: Vec = Vec::with_capacity(s.len()); CFSetGetValues(