fix(audio): replace Mutex unwrap with poisoned-mutex recovery (TODO-006)

Replace 42 .lock().unwrap() calls with .unwrap_or_else(|e| e.into_inner())
across 7 files. Poisoned mutex recovery prevents panics in realtime audio
callbacks. Add SAFETY comment to WebRtcFallbackVad Send impl (TODO-007).
This commit is contained in:
Edison Jwa
2026-06-11 09:46:51 +09:00
parent 13c7648a65
commit b20e6b663a
7 changed files with 48 additions and 45 deletions
@@ -855,7 +855,7 @@ impl AndroidVoiceUnit {
// by the capture callback's WebRtcApmProcessor. // by the capture callback's WebRtcApmProcessor.
{ {
use crate::audio_processing::EffectOwner; use crate::audio_processing::EffectOwner;
let mut apm_cfg = apm_config_clone.lock().unwrap(); let mut apm_cfg = apm_config_clone.lock().unwrap_or_else(|e| e.into_inner());
let hw_aec = hw_effects.aec.is_some(); let hw_aec = hw_effects.aec.is_some();
let hw_ns = hw_effects.ns.is_some(); let hw_ns = hw_effects.ns.is_some();
let hw_agc = hw_effects.agc.is_some(); let hw_agc = hw_effects.agc.is_some();
+31 -31
View File
@@ -481,7 +481,7 @@ impl AudioEngine {
crate::android_voice_unit::chanora_android_abandon_audio_focus(); crate::android_voice_unit::chanora_android_abandon_audio_focus();
crate::android_voice_unit::clear_global_event_sender(); crate::android_voice_unit::clear_global_event_sender();
if let Some(mut unit) = android_voice_unit.lock().unwrap().take() { if let Some(mut unit) = android_voice_unit.lock().unwrap_or_else(|e| e.into_inner()).take() {
use crate::mobile_voice_backend::MobileVoiceAudioBackend; use crate::mobile_voice_backend::MobileVoiceAudioBackend;
if let Err(e) = unit.close() { if let Err(e) = unit.close() {
warn!( warn!(
@@ -524,7 +524,7 @@ impl AudioEngine {
), ),
} }
let cfg_av = android_voice_stream_config.lock().unwrap().clone(); let cfg_av = android_voice_stream_config.lock().unwrap_or_else(|e| e.into_inner()).clone();
let params = crate::mobile_voice_backend::VoiceAudioParams { let params = crate::mobile_voice_backend::VoiceAudioParams {
voice_out_tx: voice_out_tx.clone(), voice_out_tx: voice_out_tx.clone(),
transmit_active: transmit_gate.flag_arc(), transmit_active: transmit_gate.flag_arc(),
@@ -577,7 +577,7 @@ impl AudioEngine {
"android: bluetooth route re-engaged after backend disconnect" "android: bluetooth route re-engaged after backend disconnect"
); );
} }
*android_voice_unit.lock().unwrap() = Some(reopened); *android_voice_unit.lock().unwrap_or_else(|e| e.into_inner()) = Some(reopened);
} }
Err(e) => warn!( Err(e) => warn!(
target: "chanora_audio", target: "chanora_audio",
@@ -941,7 +941,7 @@ impl AudioEngine {
match item { match item {
Some(v) => { Some(v) => {
let id = SessionAudioId(v.from_client); let id = SessionAudioId(v.from_client);
let mut h = handler_for_task.lock().unwrap(); let mut h = handler_for_task.lock().unwrap_or_else(|e| e.into_inner());
if let Err(e) = h.handle_packet(id, v.packet) { if let Err(e) = h.handle_packet(id, v.packet) {
debug!(target: "chanora_audio", error = %e, "decode failed"); debug!(target: "chanora_audio", error = %e, "decode failed");
} else { } else {
@@ -1283,7 +1283,7 @@ impl AudioEngine {
match item { match item {
Some(v) => { Some(v) => {
let id = SessionAudioId(v.from_client); let id = SessionAudioId(v.from_client);
let mut h = handler_for_task.lock().unwrap(); let mut h = handler_for_task.lock().unwrap_or_else(|e| e.into_inner());
let res = h.handle_packet(id, v.packet); let res = h.handle_packet(id, v.packet);
drop(h); drop(h);
match res { match res {
@@ -1371,12 +1371,12 @@ impl AudioEngine {
not(target_os = "android") not(target_os = "android")
))] ))]
{ {
let _ = self._input_stream.lock().unwrap().take(); let _ = self._input_stream.lock().unwrap_or_else(|e| e.into_inner()).take();
let _ = self._output_stream.lock().unwrap().take(); let _ = self._output_stream.lock().unwrap_or_else(|e| e.into_inner()).take();
} }
#[cfg(any(target_os = "ios", target_os = "macos"))] #[cfg(any(target_os = "ios", target_os = "macos"))]
{ {
let _ = self._ios_voice_backend.lock().unwrap().take(); let _ = self._ios_voice_backend.lock().unwrap_or_else(|e| e.into_inner()).take();
} }
// SDD-115 reverse-order teardown on Android: // SDD-115 reverse-order teardown on Android:
// 1) stop Bluetooth SCO + abandon audio focus; // 1) stop Bluetooth SCO + abandon audio focus;
@@ -1390,7 +1390,7 @@ impl AudioEngine {
crate::android_voice_unit::chanora_android_stop_bluetooth_sco(); crate::android_voice_unit::chanora_android_stop_bluetooth_sco();
crate::android_voice_unit::chanora_android_abandon_audio_focus(); crate::android_voice_unit::chanora_android_abandon_audio_focus();
crate::android_voice_unit::clear_global_event_sender(); crate::android_voice_unit::clear_global_event_sender();
if let Some(mut unit) = self._android_voice_unit.lock().unwrap().take() { if let Some(mut unit) = self._android_voice_unit.lock().unwrap_or_else(|e| e.into_inner()).take() {
use crate::mobile_voice_backend::MobileVoiceAudioBackend; use crate::mobile_voice_backend::MobileVoiceAudioBackend;
if let Err(e) = unit.close() { if let Err(e) = unit.close() {
warn!( warn!(
@@ -1456,7 +1456,7 @@ impl AudioEngine {
pub fn ios_restart_voice_unit(&self) -> Result<(), AudioError> { pub fn ios_restart_voice_unit(&self) -> Result<(), AudioError> {
#[cfg(any(target_os = "ios", target_os = "macos"))] #[cfg(any(target_os = "ios", target_os = "macos"))]
{ {
let mut guard = self._ios_voice_backend.lock().unwrap(); let mut guard = self._ios_voice_backend.lock().unwrap_or_else(|e| e.into_inner());
let backend = guard let backend = guard
.as_mut() .as_mut()
.ok_or_else(|| AudioError::Backend("ios voice backend not running".to_string()))?; .ok_or_else(|| AudioError::Backend("ios voice backend not running".to_string()))?;
@@ -1479,7 +1479,7 @@ impl AudioEngine {
crate::android_voice_unit::chanora_android_abandon_audio_focus(); crate::android_voice_unit::chanora_android_abandon_audio_focus();
crate::android_voice_unit::clear_global_event_sender(); crate::android_voice_unit::clear_global_event_sender();
if let Some(mut unit) = self._android_voice_unit.lock().unwrap().take() { if let Some(mut unit) = self._android_voice_unit.lock().unwrap_or_else(|e| e.into_inner()).take() {
unit.close().map_err(|e| { unit.close().map_err(|e| {
AudioError::Backend(format!( AudioError::Backend(format!(
"android: failed to close Oboe voice unit during route restart: {e}" "android: failed to close Oboe voice unit during route restart: {e}"
@@ -1501,7 +1501,7 @@ impl AudioEngine {
); );
} }
let cfg_av = self.android_voice_stream_config.lock().unwrap().clone(); let cfg_av = self.android_voice_stream_config.lock().unwrap_or_else(|e| e.into_inner()).clone();
let params = crate::mobile_voice_backend::VoiceAudioParams { let params = crate::mobile_voice_backend::VoiceAudioParams {
voice_out_tx: self.voice_out_tx.clone(), voice_out_tx: self.voice_out_tx.clone(),
transmit_active: self.transmit_gate.flag_arc(), transmit_active: self.transmit_gate.flag_arc(),
@@ -1549,7 +1549,7 @@ impl AudioEngine {
); );
} }
let mut guard = self._android_voice_unit.lock().unwrap(); let mut guard = self._android_voice_unit.lock().unwrap_or_else(|e| e.into_inner());
*guard = Some(reopened); *guard = Some(reopened);
Ok(()) Ok(())
} }
@@ -1563,7 +1563,7 @@ impl AudioEngine {
pub fn ios_pause_voice_unit(&self) -> Result<(), AudioError> { pub fn ios_pause_voice_unit(&self) -> Result<(), AudioError> {
#[cfg(any(target_os = "ios", target_os = "macos"))] #[cfg(any(target_os = "ios", target_os = "macos"))]
{ {
let mut guard = self._ios_voice_backend.lock().unwrap(); let mut guard = self._ios_voice_backend.lock().unwrap_or_else(|e| e.into_inner());
let unit = guard let unit = guard
.as_mut() .as_mut()
.ok_or_else(|| AudioError::Backend("ios voice backend not running".to_string()))?; .ok_or_else(|| AudioError::Backend("ios voice backend not running".to_string()))?;
@@ -1579,7 +1579,7 @@ impl AudioEngine {
pub fn ios_resume_voice_unit(&self) -> Result<(), AudioError> { pub fn ios_resume_voice_unit(&self) -> Result<(), AudioError> {
#[cfg(any(target_os = "ios", target_os = "macos"))] #[cfg(any(target_os = "ios", target_os = "macos"))]
{ {
let mut guard = self._ios_voice_backend.lock().unwrap(); let mut guard = self._ios_voice_backend.lock().unwrap_or_else(|e| e.into_inner());
let unit = guard let unit = guard
.as_mut() .as_mut()
.ok_or_else(|| AudioError::Backend("ios voice backend not running".to_string()))?; .ok_or_else(|| AudioError::Backend("ios voice backend not running".to_string()))?;
@@ -1639,7 +1639,7 @@ impl AudioEngine {
/// Current audio-processing config snapshot. /// Current audio-processing config snapshot.
pub fn audio_processing_config_snapshot(&self) -> crate::AudioProcessingConfig { pub fn audio_processing_config_snapshot(&self) -> crate::AudioProcessingConfig {
self.audio_processing_config.lock().unwrap().clone() self.audio_processing_config.lock().unwrap_or_else(|e| e.into_inner()).clone()
} }
/// Apply a voice-processing config after validating iOS invariants. /// Apply a voice-processing config after validating iOS invariants.
@@ -1655,7 +1655,7 @@ impl AudioEngine {
} }
#[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))] #[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))]
self.apply_desktop_vad_backend(&config); self.apply_desktop_vad_backend(&config);
let mut guard = self.audio_processing_config.lock().unwrap(); let mut guard = self.audio_processing_config.lock().unwrap_or_else(|e| e.into_inner());
*guard = config; *guard = config;
Ok(()) Ok(())
} }
@@ -1681,7 +1681,7 @@ impl AudioEngine {
/// Current voice-processing stats snapshot. /// Current voice-processing stats snapshot.
pub fn audio_processing_stats(&self) -> crate::AudioProcessingStats { pub fn audio_processing_stats(&self) -> crate::AudioProcessingStats {
let config = self.audio_processing_config.lock().unwrap().clone(); let config = self.audio_processing_config.lock().unwrap_or_else(|e| e.into_inner()).clone();
self.audio_processing_stats.snapshot(&config) self.audio_processing_stats.snapshot(&config)
} }
@@ -1816,7 +1816,7 @@ fn apply_desktop_vad_backend_to_worker(
audio_processing_stats: &crate::SharedAudioProcessingStats, audio_processing_stats: &crate::SharedAudioProcessingStats,
) { ) {
if config.vad_backend != crate::VadBackend::SileroOnnx { if config.vad_backend != crate::VadBackend::SileroOnnx {
let mut worker_guard = silero_vad_worker.lock().unwrap(); let mut worker_guard = silero_vad_worker.lock().unwrap_or_else(|e| e.into_inner());
if worker_guard.is_some() { if worker_guard.is_some() {
info!( info!(
target: "chanora_audio", target: "chanora_audio",
@@ -1841,12 +1841,12 @@ fn apply_desktop_vad_backend_to_worker(
); );
let new_worker = crate::vad::silero_onnx::SileroOnnxVadWorker::try_new(&model_path); let new_worker = crate::vad::silero_onnx::SileroOnnxVadWorker::try_new(&model_path);
{ {
let mut worker_guard = silero_vad_worker.lock().unwrap(); let mut worker_guard = silero_vad_worker.lock().unwrap_or_else(|e| e.into_inner());
*worker_guard = new_worker; *worker_guard = new_worker;
} }
// Check result after releasing the lock. Re-acquire is cheap and // Check result after releasing the lock. Re-acquire is cheap and
// ensures we log the correct state without holding the mutex. // ensures we log the correct state without holding the mutex.
let worker_installed = silero_vad_worker.lock().unwrap().is_some(); let worker_installed = silero_vad_worker.lock().unwrap_or_else(|e| e.into_inner()).is_some();
if worker_installed { if worker_installed {
info!( info!(
target: "chanora_audio", target: "chanora_audio",
@@ -1926,21 +1926,21 @@ mod tests {
#[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))] #[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))]
#[test] #[test]
fn desktop_startup_audio_processing_state_applies_default_silero_fallback() { fn desktop_startup_audio_processing_state_applies_default_silero_fallback() {
let _guard = crate::vad::SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap(); let _guard = crate::vad::SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner());
crate::vad::clear_silero_model_path_for_test(); crate::vad::clear_silero_model_path_for_test();
let (config, stats, worker) = new_desktop_audio_processing_state(); let (config, stats, worker) = new_desktop_audio_processing_state();
assert_eq!( assert_eq!(
config.lock().unwrap().vad_backend, config.lock().unwrap_or_else(|e| e.into_inner()).vad_backend,
crate::VadBackend::SileroOnnx, crate::VadBackend::SileroOnnx,
"desktop startup config should keep the default Silero backend selected" "desktop startup config should keep the default Silero backend selected"
); );
assert!( assert!(
worker.lock().unwrap().is_none(), worker.lock().unwrap_or_else(|e| e.into_inner()).is_none(),
"missing startup model should not create an ONNX worker" "missing startup model should not create an ONNX worker"
); );
let snapshot = stats.snapshot(&config.lock().unwrap()); let snapshot = stats.snapshot(&config.lock().unwrap_or_else(|e| e.into_inner()));
assert!( assert!(
snapshot.vad_fallback_active, snapshot.vad_fallback_active,
"desktop startup should mark WebRTC fallback active when the default Silero worker cannot load" "desktop startup should mark WebRTC fallback active when the default Silero worker cannot load"
@@ -1983,7 +1983,7 @@ mod tests {
#[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))] #[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))]
#[test] #[test]
fn desktop_capture_silero_selection_reports_fallback_when_worker_unavailable() { fn desktop_capture_silero_selection_reports_fallback_when_worker_unavailable() {
let _guard = crate::vad::SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap(); let _guard = crate::vad::SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner());
crate::vad::clear_silero_model_path_for_test(); crate::vad::clear_silero_model_path_for_test();
let gate = crate::ptt::AudioTransmitGate::new(false); let gate = crate::ptt::AudioTransmitGate::new(false);
@@ -2025,7 +2025,7 @@ mod tests {
} }
capture.ingest(&voiced); capture.ingest(&voiced);
let snapshot = stats.snapshot(&config.lock().unwrap()); let snapshot = stats.snapshot(&config.lock().unwrap_or_else(|e| e.into_inner()));
assert_eq!(snapshot.vad_backend, crate::VadBackend::SileroOnnx); assert_eq!(snapshot.vad_backend, crate::VadBackend::SileroOnnx);
assert!( assert!(
snapshot.vad_fallback_active, snapshot.vad_fallback_active,
@@ -2132,7 +2132,7 @@ mod tests {
capture.process_10ms_capture_frame(&voiced); capture.process_10ms_capture_frame(&voiced);
let snapshot = stats.snapshot(&config.lock().unwrap()); let snapshot = stats.snapshot(&config.lock().unwrap_or_else(|e| e.into_inner()));
assert!( assert!(
snapshot.vad_fallback_active, snapshot.vad_fallback_active,
"stale Silero worker output should report active WebRTC fallback" "stale Silero worker output should report active WebRTC fallback"
@@ -2725,7 +2725,7 @@ where
.build_input_stream( .build_input_stream(
*config, *config,
move |data: &[T], _: &cpal::InputCallbackInfo| { move |data: &[T], _: &cpal::InputCallbackInfo| {
let mut s = state.lock().unwrap(); let mut s = state.lock().unwrap_or_else(|e| e.into_inner());
s.ingest(data); s.ingest(data);
}, },
move |e| { move |e| {
@@ -2816,7 +2816,7 @@ where
// only for the duration of fill_buffer; the inbound // only for the duration of fill_buffer; the inbound
// forwarder uses handle_packet which is queue-fast. // forwarder uses handle_packet which is queue-fast.
{ {
let mut h = handler.lock().unwrap(); let mut h = handler.lock().unwrap_or_else(|e| e.into_inner());
h.fill_buffer(&mut scratch[..needed]); h.fill_buffer(&mut scratch[..needed]);
} }
@@ -2835,7 +2835,7 @@ where
// Resample 48 kHz stereo → device-rate × // Resample 48 kHz stereo → device-rate ×
// device-channels with continuity across // device-channels with continuity across
// callback boundaries. // callback boundaries.
let mut state = resample_state.lock().unwrap(); let mut state = resample_state.lock().unwrap_or_else(|e| e.into_inner());
let mut pos = state.pos; let mut pos = state.pos;
let mut last_l = state.last_l; let mut last_l = state.last_l;
let mut last_r = state.last_r; let mut last_r = state.last_r;
+4 -4
View File
@@ -549,7 +549,7 @@ impl IosVoiceUnit {
let unit_arc2 = Arc::clone(&unit_arc); let unit_arc2 = Arc::clone(&unit_arc);
dispatch2::DispatchQueue::main().exec_async(move || { dispatch2::DispatchQueue::main().exec_async(move || {
let mut guard = unit_arc2.lock().unwrap(); let mut guard = unit_arc2.lock().unwrap_or_else(|e| e.into_inner());
let unit = guard.as_mut().unwrap(); let unit = guard.as_mut().unwrap();
let _ = tx.send(op(unit)); let _ = tx.send(op(unit));
}); });
@@ -559,7 +559,7 @@ impl IosVoiceUnit {
Err(_) => Err("vpio lifecycle: main thread channel closed unexpectedly".to_string()), Err(_) => Err("vpio lifecycle: main thread channel closed unexpectedly".to_string()),
}; };
self.unit = unit_arc.lock().unwrap().take(); self.unit = unit_arc.lock().unwrap_or_else(|e| e.into_inner()).take();
result.map_err(AudioError::Backend) result.map_err(AudioError::Backend)
} }
@@ -1072,7 +1072,7 @@ impl IosVoiceUnit {
let unit_arc2 = unit_arc.clone(); let unit_arc2 = unit_arc.clone();
dispatch2::DispatchQueue::main().exec_async(move || { dispatch2::DispatchQueue::main().exec_async(move || {
let mut guard = unit_arc2.lock().unwrap(); let mut guard = unit_arc2.lock().unwrap_or_else(|e| e.into_inner());
let u = guard.as_mut().unwrap(); let u = guard.as_mut().unwrap();
let result = u let result = u
.initialize() .initialize()
@@ -1094,7 +1094,7 @@ impl IosVoiceUnit {
} }
} }
unit = unit_arc.lock().unwrap().take().unwrap(); unit = unit_arc.lock().unwrap_or_else(|e| e.into_inner()).take().unwrap();
} }
info!( info!(
@@ -543,7 +543,7 @@ impl DesktopPttBackend for MacOSEventTapBackend {
} }
let runloop = unsafe { CFRunLoopGetCurrent() }; let runloop = unsafe { CFRunLoopGetCurrent() };
{ {
let mut g = worker_runloop.lock().unwrap(); let mut g = worker_runloop.lock().unwrap_or_else(|e| e.into_inner());
*g = Some(RunLoopHandle(runloop)); *g = Some(RunLoopHandle(runloop));
} }
unsafe { unsafe {
+1 -1
View File
@@ -174,7 +174,7 @@ impl AudioCallback for TsPlaybackCallback {
// in the cpal path, but the upstream design has shipped // in the cpal path, but the upstream design has shipped
// this way for years. // this way for years.
{ {
let mut data = self.handler.lock().unwrap(); let mut data = self.handler.lock().unwrap_or_else(|e| e.into_inner());
let _removed_ids = data.fill_buffer(buffer); let _removed_ids = data.fill_buffer(buffer);
// `_removed_ids` is the list of clients whose stream the // `_removed_ids` is the list of clients whose stream the
// handler just finished draining. We could publish that // handler just finished draining. We could publish that
+8 -5
View File
@@ -42,9 +42,12 @@ pub struct WebRtcFallbackVad {
frame_i16: [i16; INPUT_FRAME_10MS], frame_i16: [i16; INPUT_FRAME_10MS],
} }
// `webrtc_vad::Vad` owns an FFI pointer and is only touched from the // SAFETY: `webrtc_vad::Vad` wraps an opaque FFI pointer to the WebRTC C VAD
// capture thread after construction. Moving the wrapper between threads is // state. The underlying C struct has no interior mutability that would cause
// safe; sharing it concurrently is not required and not implemented. // data races when moved between threads — `WebRtcVad_Process()` reads/writes
// the struct exclusively through the passed pointer with no shared static state.
// This wrapper is only used from a single capture thread after construction;
// we never share `&WebRtcFallbackVad` across threads (no `Sync` impl).
unsafe impl Send for WebRtcFallbackVad {} unsafe impl Send for WebRtcFallbackVad {}
impl Default for WebRtcFallbackVad { impl Default for WebRtcFallbackVad {
@@ -250,7 +253,7 @@ mod tests {
#[test] #[test]
fn set_silero_model_path_rejects_missing_file() { fn set_silero_model_path_rejects_missing_file() {
let _guard = SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap(); let _guard = SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner());
clear_silero_model_path_for_test(); clear_silero_model_path_for_test();
let result = set_silero_model_path("/definitely/not/a/silero_vad.onnx"); let result = set_silero_model_path("/definitely/not/a/silero_vad.onnx");
@@ -261,7 +264,7 @@ mod tests {
#[test] #[test]
fn set_silero_model_path_updates_override_and_epoch() { fn set_silero_model_path_updates_override_and_epoch() {
let _guard = SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap(); let _guard = SILERO_MODEL_PATH_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner());
clear_silero_model_path_for_test(); clear_silero_model_path_for_test();
let path = let path =
+2 -2
View File
@@ -82,7 +82,7 @@ struct RecordingLayer {
impl RecordingLayer { impl RecordingLayer {
fn snapshot(&self) -> Vec<Captured> { fn snapshot(&self) -> Vec<Captured> {
self.records.lock().unwrap().clone() self.records.lock().unwrap_or_else(|e| e.into_inner()).clone()
} }
} }
@@ -118,7 +118,7 @@ where
target: event.metadata().target().to_string(), target: event.metadata().target().to_string(),
field_names: names.0, field_names: names.0,
}; };
self.records.lock().unwrap().push(captured); self.records.lock().unwrap_or_else(|e| e.into_inner()).push(captured);
} }
} }