Skip to content

Commit 4f61626

Browse files
committed
fix: Defer HID API initialization until scan
Mitigates INTIFACE-CENTRAL-R6
1 parent 9a33523 commit 4f61626

1 file changed

Lines changed: 87 additions & 10 deletions

File tree

crates/buttplug_server_hwmgr_hid/src/hid_comm_manager.rs

Lines changed: 87 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ use buttplug_server::device::hardware::communication::{
1414
TimedRetryCommunicationManager,
1515
TimedRetryCommunicationManagerImpl,
1616
};
17-
use hidapi::HidApi;
17+
use hidapi::{HidApi, HidResult};
1818
use log::*;
19-
use std::sync::Arc;
19+
use std::sync::{Arc, Mutex};
2020
use tokio::sync::mpsc::Sender;
2121

2222
use super::hid_device_impl::HidHardwareConnector;
@@ -37,15 +37,46 @@ impl HardwareCommunicationManagerBuilder for HidCommunicationManagerBuilder {
3737

3838
pub struct HidCommunicationManager {
3939
sender: Sender<HardwareCommunicationManagerEvent>,
40-
hidapi: Arc<HidApi>,
40+
hidapi: Mutex<Option<Arc<HidApi>>>,
41+
hidapi_factory: Box<dyn Fn() -> HidResult<HidApi> + Send + Sync>,
4142
}
4243

4344
impl HidCommunicationManager {
4445
fn new(sender: Sender<HardwareCommunicationManagerEvent>) -> Self {
46+
Self::new_with_hidapi_factory(sender, Box::new(HidApi::new))
47+
}
48+
49+
fn new_with_hidapi_factory(
50+
sender: Sender<HardwareCommunicationManagerEvent>,
51+
hidapi_factory: Box<dyn Fn() -> HidResult<HidApi> + Send + Sync>,
52+
) -> Self {
4553
Self {
4654
sender,
47-
hidapi: Arc::new(HidApi::new().unwrap()),
55+
hidapi: Mutex::new(None),
56+
hidapi_factory,
57+
}
58+
}
59+
60+
fn hidapi(&self) -> Result<Arc<HidApi>, ButtplugDeviceError> {
61+
let mut hidapi = self.hidapi.lock().map_err(|_| {
62+
ButtplugDeviceError::DeviceCommunicationError("HIDAPI lock poisoned.".to_owned())
63+
})?;
64+
if let Some(api) = hidapi.as_ref() {
65+
return Ok(api.clone());
4866
}
67+
68+
let api = (self.hidapi_factory)().map_err(|err| {
69+
error!("Failed to create HIDAPI instance: {}", err);
70+
ButtplugDeviceError::DeviceConnectionError(format!("Cannot create HIDAPI: {err}"))
71+
})?;
72+
let api = Arc::new(api);
73+
*hidapi = Some(api.clone());
74+
Ok(api)
75+
}
76+
77+
#[cfg(target_os = "macos")]
78+
fn hidapi_initialized(&self) -> bool {
79+
self.hidapi.lock().map(|api| api.is_some()).unwrap_or(false)
4980
}
5081
}
5182

@@ -58,22 +89,22 @@ impl TimedRetryCommunicationManagerImpl for HidCommunicationManager {
5889
async fn scan(&self) -> Result<(), ButtplugDeviceError> {
5990
// TODO Does this block? Should it run in one of our threads?
6091
let device_sender = self.sender.clone();
61-
let api = self.hidapi.clone();
92+
let api = self.hidapi()?;
6293

6394
let mut seen_addresses = vec![];
6495
for device in api.device_list() {
65-
if device.serial_number().is_none() {
96+
let Some(serial_number) = device.serial_number().map(str::to_owned) else {
6697
continue;
67-
}
68-
let serial_number = device.serial_number().unwrap().to_owned();
98+
};
6999
if seen_addresses.contains(&serial_number) {
70100
continue;
71101
}
72102
seen_addresses.push(serial_number.clone());
103+
let name = device.product_string().unwrap_or("Unknown HID Device");
73104
let device_creator = HidHardwareConnector::new(api.clone(), device);
74105
if device_sender
75106
.send(HardwareCommunicationManagerEvent::DeviceFound {
76-
name: device.product_string().unwrap().to_owned(),
107+
name: name.to_owned(),
77108
address: serial_number,
78109
creator: Box::new(device_creator),
79110
})
@@ -105,6 +136,52 @@ fn reset_hidapi() {
105136
impl Drop for HidCommunicationManager {
106137
fn drop(&mut self) {
107138
#[cfg(target_os = "macos")]
108-
reset_hidapi();
139+
if self.hidapi_initialized() {
140+
reset_hidapi();
141+
}
142+
}
143+
}
144+
145+
#[cfg(test)]
146+
mod tests {
147+
use super::*;
148+
use hidapi::HidError;
149+
use std::sync::{
150+
Arc,
151+
atomic::{AtomicBool, Ordering},
152+
};
153+
154+
#[test]
155+
fn construction_does_not_initialize_hidapi() {
156+
let (sender, _receiver) = tokio::sync::mpsc::channel(1);
157+
let called = Arc::new(AtomicBool::new(false));
158+
let factory_called = called.clone();
159+
160+
let _manager = HidCommunicationManager::new_with_hidapi_factory(
161+
sender,
162+
Box::new(move || {
163+
factory_called.store(true, Ordering::Relaxed);
164+
Err(HidError::InitializationError)
165+
}),
166+
);
167+
168+
assert!(!called.load(Ordering::Relaxed));
169+
}
170+
171+
#[test]
172+
fn scan_returns_error_when_hidapi_initialization_fails() {
173+
let (sender, _receiver) = tokio::sync::mpsc::channel(1);
174+
let manager = HidCommunicationManager::new_with_hidapi_factory(
175+
sender,
176+
Box::new(|| Err(HidError::InitializationError)),
177+
);
178+
179+
let result = futures::executor::block_on(manager.scan());
180+
181+
assert!(matches!(
182+
result,
183+
Err(ButtplugDeviceError::DeviceConnectionError(message))
184+
if message.contains("Cannot create HIDAPI")
185+
));
109186
}
110187
}

0 commit comments

Comments
 (0)