From 097c9effc6bb705ca70850318206df822008c313 Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Mon, 29 Jul 2019 16:32:19 +1200 Subject: [PATCH 1/2] server: Don't swallow errors in register_device_collection_changed. --- server/src/server.rs | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/server/src/server.rs b/server/src/server.rs index 9d70cd3..8aac22c 100644 --- a/server/src/server.rs +++ b/server/src/server.rs @@ -50,7 +50,11 @@ impl CubebDeviceCollectionManager { } } - fn register(&mut self, context: &cubeb::Context, server: &Rc>) { + fn register( + &mut self, + context: &cubeb::Context, + server: &Rc>, + ) -> cubeb::Result<()> { if self .servers .iter() @@ -59,27 +63,32 @@ impl CubebDeviceCollectionManager { { self.servers.push(server.clone()); } - self.update(context); + self.update(context) } - fn unregister(&mut self, context: &cubeb::Context, server: &Rc>) { + fn unregister( + &mut self, + context: &cubeb::Context, + server: &Rc>, + ) -> cubeb::Result<()> { self.servers .retain(|s| !(Rc::ptr_eq(&s, server) && s.borrow().devtype.is_empty())); - self.update(context); + self.update(context) } - fn update(&mut self, context: &cubeb::Context) { + fn update(&mut self, context: &cubeb::Context) -> cubeb::Result<()> { let mut devtype = cubeb::DeviceType::empty(); for s in &self.servers { devtype |= s.borrow().devtype; } for &dir in &[cubeb::DeviceType::INPUT, cubeb::DeviceType::OUTPUT] { match (devtype.contains(dir), self.devtype.contains(dir)) { - (true, false) => self.internal_register(context, dir, true), - (false, true) => self.internal_register(context, dir, false), + (true, false) => self.internal_register(context, dir, true)?, + (false, true) => self.internal_register(context, dir, false)?, _ => {} } } + Ok(()) } fn internal_register( @@ -87,7 +96,7 @@ impl CubebDeviceCollectionManager { context: &cubeb::Context, devtype: cubeb::DeviceType, enable: bool, - ) { + ) -> cubeb::Result<()> { let user_ptr = if enable { self as *const CubebDeviceCollectionManager as *mut c_void } else { @@ -106,13 +115,11 @@ impl CubebDeviceCollectionManager { if devtype.contains(dir) { assert_eq!(self.devtype.contains(dir), !enable); unsafe { - context - .register_device_collection_changed( - dir, - if enable { Some(cb) } else { None }, - user_ptr, - ) - .expect("devcol register failed"); + context.register_device_collection_changed( + dir, + if enable { Some(cb) } else { None }, + user_ptr, + )?; } if enable { self.devtype.insert(dir); @@ -121,6 +128,7 @@ impl CubebDeviceCollectionManager { } } } + Ok(()) } // Warning: this is called from an internal cubeb thread, so we must not mutate unprotected shared state. @@ -540,12 +548,12 @@ impl CubebServer { if enable { cbs.borrow_mut().devtype.insert(devtype); - manager.register(context, cbs); + manager.register(context, cbs) } else { cbs.borrow_mut().devtype.remove(devtype); - manager.unregister(context, cbs); + manager.unregister(context, cbs) } - Ok(ClientMessage::ContextRegisteredDeviceCollectionChanged) + .map(|_| ClientMessage::ContextRegisteredDeviceCollectionChanged) } // Stream init is special, so it's been separated from process_msg. From 65e0be1d2d2847daea78a8dc8d6cfc4f865eed20 Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Tue, 30 Jul 2019 10:52:27 +1200 Subject: [PATCH 2/2] Refactor CubebDeviceCollectionManager::update slightly. --- server/src/server.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/server.rs b/server/src/server.rs index 8aac22c..94269d7 100644 --- a/server/src/server.rs +++ b/server/src/server.rs @@ -82,10 +82,8 @@ impl CubebDeviceCollectionManager { devtype |= s.borrow().devtype; } for &dir in &[cubeb::DeviceType::INPUT, cubeb::DeviceType::OUTPUT] { - match (devtype.contains(dir), self.devtype.contains(dir)) { - (true, false) => self.internal_register(context, dir, true)?, - (false, true) => self.internal_register(context, dir, false)?, - _ => {} + if devtype.contains(dir) != self.devtype.contains(dir) { + self.internal_register(context, dir, devtype.contains(dir))?; } } Ok(())