From 7fdde63460a4c8ea800f56e300af1e23eabf0c4f Mon Sep 17 00:00:00 2001 From: johnpyp Date: Mon, 16 Feb 2026 09:18:50 -0500 Subject: [PATCH] fix: transcription lock-up race condition & add small debounce (#824) * fix: add TranscriptionState to prevent race conditions & add debounce * refactor(transcription): replace state machine with actor coordinator * some cleanup * format * minor cleanup * format --------- Co-authored-by: CJ Pais --- src-tauri/src/actions.rs | 19 ++- src-tauri/src/lib.rs | 15 +- src-tauri/src/shortcut/handler.rs | 53 ++---- src-tauri/src/signal_handle.rs | 65 +------- src-tauri/src/transcription_coordinator.rs | 184 +++++++++++++++++++++ src-tauri/src/utils.rs | 19 +-- 6 files changed, 234 insertions(+), 121 deletions(-) create mode 100644 src-tauri/src/transcription_coordinator.rs diff --git a/src-tauri/src/actions.rs b/src-tauri/src/actions.rs index f15efc0..5339388 100644 --- a/src-tauri/src/actions.rs +++ b/src-tauri/src/actions.rs @@ -10,7 +10,7 @@ use crate::tray::{change_tray_icon, TrayIconState}; use crate::utils::{ self, show_processing_overlay, show_recording_overlay, show_transcribing_overlay, }; -use crate::ManagedToggleState; +use crate::TranscriptionCoordinator; use ferrous_opencc::{config::BuiltinConfig, OpenCC}; use log::{debug, error}; use once_cell::sync::Lazy; @@ -20,6 +20,17 @@ use std::time::Instant; use tauri::AppHandle; use tauri::Manager; +/// Drop guard that notifies the [`TranscriptionCoordinator`] when the +/// transcription pipeline finishes — whether it completes normally or panics. +struct FinishGuard(AppHandle); +impl Drop for FinishGuard { + fn drop(&mut self) { + if let Some(c) = self.0.try_state::() { + c.notify_processing_finished(); + } + } +} + // Shortcut Action Trait pub trait ShortcutAction: Send + Sync { fn start(&self, app: &AppHandle, binding_id: &str, shortcut_str: &str); @@ -305,6 +316,7 @@ impl ShortcutAction for TranscribeAction { let post_process = self.post_process; tauri::async_runtime::spawn(async move { + let _guard = FinishGuard(ah.clone()); let binding_id = binding_id.clone(); // Clone for the inner async task debug!( "Starting async transcription task for binding: {}", @@ -423,11 +435,6 @@ impl ShortcutAction for TranscribeAction { utils::hide_recording_overlay(&ah); change_tray_icon(&ah, TrayIconState::Idle); } - - // Clear toggle state now that transcription is complete - if let Ok(mut states) = ah.state::().lock() { - states.active_toggles.insert(binding_id, false); - } }); debug!( diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 7d00fc9..36f28e5 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -13,6 +13,7 @@ mod overlay; mod settings; mod shortcut; mod signal_handle; +mod transcription_coordinator; mod tray; mod tray_i18n; mod utils; @@ -28,10 +29,10 @@ use managers::transcription::TranscriptionManager; use signal_hook::consts::SIGUSR2; #[cfg(unix)] use signal_hook::iterator::Signals; -use std::collections::HashMap; use std::sync::atomic::{AtomicU8, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use tauri::image::Image; +pub use transcription_coordinator::TranscriptionCoordinator; use tauri::tray::TrayIconBuilder; use tauri::{AppHandle, Emitter, Listener, Manager}; @@ -78,14 +79,6 @@ fn build_console_filter() -> env_filter::Filter { builder.build() } -#[derive(Default)] -struct ShortcutToggleStates { - // Map: shortcut_binding_id -> is_active - active_toggles: HashMap, -} - -type ManagedToggleState = Mutex; - fn show_main_window(app: &AppHandle) { if let Some(main_window) = app.get_webview_window("main") { // First, ensure the window is visible @@ -407,7 +400,6 @@ pub fn run() { MacosLauncher::LaunchAgent, Some(vec![]), )) - .manage(Mutex::new(ShortcutToggleStates::default())) .setup(move |app| { let settings = get_settings(&app.handle()); let tauri_log_level: tauri_plugin_log::LogLevel = settings.log_level.into(); @@ -415,6 +407,7 @@ pub fn run() { // Store the file log level in the atomic for the filter to use FILE_LOG_LEVEL.store(file_log_level.to_level_filter() as u8, Ordering::Relaxed); let app_handle = app.handle().clone(); + app.manage(TranscriptionCoordinator::new(app_handle.clone())); initialize_core_logic(&app_handle); diff --git a/src-tauri/src/shortcut/handler.rs b/src-tauri/src/shortcut/handler.rs index c00fb47..c0ab7f3 100644 --- a/src-tauri/src/shortcut/handler.rs +++ b/src-tauri/src/shortcut/handler.rs @@ -10,7 +10,8 @@ use tauri::{AppHandle, Manager}; use crate::actions::ACTION_MAP; use crate::managers::audio::AudioRecordingManager; use crate::settings::get_settings; -use crate::ManagedToggleState; +use crate::transcription_coordinator::is_transcribe_binding; +use crate::TranscriptionCoordinator; /// Handle a shortcut event from either implementation. /// @@ -33,6 +34,16 @@ pub fn handle_shortcut_event( ) { let settings = get_settings(app); + // Transcribe bindings are handled by the coordinator. + if is_transcribe_binding(binding_id) { + if let Some(coordinator) = app.try_state::() { + coordinator.send_input(binding_id, hotkey_string, is_pressed, settings.push_to_talk); + } else { + warn!("TranscriptionCoordinator is not initialized"); + } + return; + } + let Some(action) = ACTION_MAP.get(binding_id) else { warn!( "No action defined in ACTION_MAP for shortcut ID '{}'. Shortcut: '{}', Pressed: {}", @@ -50,42 +61,10 @@ pub fn handle_shortcut_event( return; } - // Push-to-talk mode: start on press, stop on release - if settings.push_to_talk { - if is_pressed { - action.start(app, binding_id, hotkey_string); - } else { - action.stop(app, binding_id, hotkey_string); - } - return; - } - - // Toggle mode: toggle state on press only + // Remaining bindings (e.g. "test") use simple start/stop on press/release. if is_pressed { - // Determine action and update state while holding the lock, - // but RELEASE the lock before calling the action to avoid deadlocks. - // (Actions may need to acquire the lock themselves, e.g., cancel_current_operation) - let should_start: bool; - { - let toggle_state_manager = app.state::(); - let mut states = toggle_state_manager - .lock() - .expect("Failed to lock toggle state manager"); - - let is_currently_active = states - .active_toggles - .entry(binding_id.to_string()) - .or_insert(false); - - should_start = !*is_currently_active; - *is_currently_active = should_start; - } // Lock released here - - // Now call the action without holding the lock - if should_start { - action.start(app, binding_id, hotkey_string); - } else { - action.stop(app, binding_id, hotkey_string); - } + action.start(app, binding_id, hotkey_string); + } else { + action.stop(app, binding_id, hotkey_string); } } diff --git a/src-tauri/src/signal_handle.rs b/src-tauri/src/signal_handle.rs index f09a616..35ff261 100644 --- a/src-tauri/src/signal_handle.rs +++ b/src-tauri/src/signal_handle.rs @@ -1,6 +1,5 @@ -use crate::actions::ACTION_MAP; -use crate::ManagedToggleState; -use log::{debug, info, warn}; +use crate::TranscriptionCoordinator; +use log::{debug, warn}; use std::thread; use tauri::{AppHandle, Manager}; @@ -11,62 +10,16 @@ use signal_hook::iterator::Signals; #[cfg(unix)] pub fn setup_signal_handler(app_handle: AppHandle, mut signals: Signals) { - let app_handle_for_signal = app_handle.clone(); - - debug!("SIGUSR2 signal handler registered successfully"); + debug!("SIGUSR2 signal handler registered"); thread::spawn(move || { - debug!("SIGUSR2 signal handler thread started"); for sig in signals.forever() { - match sig { - SIGUSR2 => { - debug!("Received SIGUSR2 signal (signal number: {sig})"); - - let binding_id = "transcribe"; - let shortcut_string = "SIGUSR2"; - - if let Some(action) = ACTION_MAP.get(binding_id) { - // Determine action and update state while holding the lock, - // but RELEASE the lock before calling the action to avoid deadlocks. - // (Actions may need to acquire the lock themselves, e.g., cancel_current_operation) - let should_start: bool; - { - let toggle_state_manager = - app_handle_for_signal.state::(); - - let mut states = match toggle_state_manager.lock() { - Ok(s) => s, - Err(e) => { - warn!("Failed to lock toggle state manager: {e}"); - continue; - } - }; - - let is_currently_active = states - .active_toggles - .entry(binding_id.to_string()) - .or_insert(false); - - should_start = !*is_currently_active; - if should_start { - *is_currently_active = true; - } - } // Lock released here - - // Now call the action without holding the lock - if should_start { - debug!("SIGUSR2: Starting transcription (was inactive)"); - action.start(&app_handle_for_signal, binding_id, shortcut_string); - info!("SIGUSR2: Transcription started"); - } else { - debug!("SIGUSR2: Stopping transcription (was active)"); - action.stop(&app_handle_for_signal, binding_id, shortcut_string); - debug!("SIGUSR2: Transcription stopped"); - } - } else { - warn!("No action defined in ACTION_MAP for binding ID '{binding_id}'"); - } + if sig == SIGUSR2 { + debug!("Received SIGUSR2"); + if let Some(c) = app_handle.try_state::() { + c.send_input("transcribe", "SIGUSR2", true, false); + } else { + warn!("TranscriptionCoordinator not initialized"); } - _ => unreachable!(), } } }); diff --git a/src-tauri/src/transcription_coordinator.rs b/src-tauri/src/transcription_coordinator.rs new file mode 100644 index 0000000..255cd4d --- /dev/null +++ b/src-tauri/src/transcription_coordinator.rs @@ -0,0 +1,184 @@ +use crate::actions::ACTION_MAP; +use crate::managers::audio::AudioRecordingManager; +use log::{debug, error, warn}; +use std::sync::mpsc::{self, Sender}; +use std::sync::Arc; +use std::thread; +use std::time::{Duration, Instant}; +use tauri::{AppHandle, Manager}; + +const DEBOUNCE: Duration = Duration::from_millis(30); + +/// Commands processed sequentially by the coordinator thread. +enum Command { + Input { + binding_id: String, + hotkey_string: String, + is_pressed: bool, + push_to_talk: bool, + }, + Cancel { + recording_was_active: bool, + }, + ProcessingFinished, +} + +/// Pipeline lifecycle, owned exclusively by the coordinator thread. +enum Stage { + Idle, + Recording(String), // binding_id + Processing, +} + +/// Serialises all transcription lifecycle events through a single thread +/// to eliminate race conditions between keyboard shortcuts, signals, and +/// the async transcribe-paste pipeline. +pub struct TranscriptionCoordinator { + tx: Sender, +} + +pub fn is_transcribe_binding(id: &str) -> bool { + id == "transcribe" || id == "transcribe_with_post_process" +} + +impl TranscriptionCoordinator { + pub fn new(app: AppHandle) -> Self { + let (tx, rx) = mpsc::channel(); + + thread::spawn(move || { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let mut stage = Stage::Idle; + let mut last_press: Option = None; + + while let Ok(cmd) = rx.recv() { + match cmd { + Command::Input { + binding_id, + hotkey_string, + is_pressed, + push_to_talk, + } => { + // Debounce rapid-fire press events (key repeat / double-tap). + // Releases always pass through for push-to-talk. + if is_pressed { + let now = Instant::now(); + if last_press.map_or(false, |t| now.duration_since(t) < DEBOUNCE) { + debug!("Debounced press for '{binding_id}'"); + continue; + } + last_press = Some(now); + } + + if push_to_talk { + if is_pressed && matches!(stage, Stage::Idle) { + start(&app, &mut stage, &binding_id, &hotkey_string); + } else if !is_pressed + && matches!(&stage, Stage::Recording(id) if id == &binding_id) + { + stop(&app, &mut stage, &binding_id, &hotkey_string); + } + } else if is_pressed { + match &stage { + Stage::Idle => { + start(&app, &mut stage, &binding_id, &hotkey_string); + } + Stage::Recording(id) if id == &binding_id => { + stop(&app, &mut stage, &binding_id, &hotkey_string); + } + _ => { + debug!("Ignoring press for '{binding_id}': pipeline busy") + } + } + } + } + Command::Cancel { + recording_was_active, + } => { + // Don't reset during processing — wait for the pipeline to finish. + if !matches!(stage, Stage::Processing) + && (recording_was_active || matches!(stage, Stage::Recording(_))) + { + stage = Stage::Idle; + } + } + Command::ProcessingFinished => { + stage = Stage::Idle; + } + } + } + debug!("Transcription coordinator exited"); + })); + if let Err(e) = result { + error!("Transcription coordinator panicked: {e:?}"); + } + }); + + Self { tx } + } + + /// Send a keyboard/signal input event for a transcribe binding. + /// For signal-based toggles, use `is_pressed: true` and `push_to_talk: false`. + pub fn send_input( + &self, + binding_id: &str, + hotkey_string: &str, + is_pressed: bool, + push_to_talk: bool, + ) { + if self + .tx + .send(Command::Input { + binding_id: binding_id.to_string(), + hotkey_string: hotkey_string.to_string(), + is_pressed, + push_to_talk, + }) + .is_err() + { + warn!("Transcription coordinator channel closed"); + } + } + + pub fn notify_cancel(&self, recording_was_active: bool) { + if self + .tx + .send(Command::Cancel { + recording_was_active, + }) + .is_err() + { + warn!("Transcription coordinator channel closed"); + } + } + + pub fn notify_processing_finished(&self) { + if self.tx.send(Command::ProcessingFinished).is_err() { + warn!("Transcription coordinator channel closed"); + } + } +} + +fn start(app: &AppHandle, stage: &mut Stage, binding_id: &str, hotkey_string: &str) { + let Some(action) = ACTION_MAP.get(binding_id) else { + warn!("No action in ACTION_MAP for '{binding_id}'"); + return; + }; + action.start(app, binding_id, hotkey_string); + if app + .try_state::>() + .map_or(false, |a| a.is_recording()) + { + *stage = Stage::Recording(binding_id.to_string()); + } else { + debug!("Start for '{binding_id}' did not begin recording; staying idle"); + } +} + +fn stop(app: &AppHandle, stage: &mut Stage, binding_id: &str, hotkey_string: &str) { + let Some(action) = ACTION_MAP.get(binding_id) else { + warn!("No action in ACTION_MAP for '{binding_id}'"); + return; + }; + action.stop(app, binding_id, hotkey_string); + *stage = Stage::Processing; +} diff --git a/src-tauri/src/utils.rs b/src-tauri/src/utils.rs index 10a4a6c..063e4b3 100644 --- a/src-tauri/src/utils.rs +++ b/src-tauri/src/utils.rs @@ -1,8 +1,8 @@ use crate::managers::audio::AudioRecordingManager; use crate::managers::transcription::TranscriptionManager; use crate::shortcut; -use crate::ManagedToggleState; -use log::{info, warn}; +use crate::TranscriptionCoordinator; +use log::info; use std::sync::Arc; use tauri::{AppHandle, Manager}; @@ -20,17 +20,9 @@ pub fn cancel_current_operation(app: &AppHandle) { // Unregister the cancel shortcut asynchronously shortcut::unregister_cancel_shortcut(app); - // First, reset all shortcut toggle states. - // This is critical for non-push-to-talk mode where shortcuts toggle on/off - let toggle_state_manager = app.state::(); - if let Ok(mut states) = toggle_state_manager.lock() { - states.active_toggles.values_mut().for_each(|v| *v = false); - } else { - warn!("Failed to lock toggle state manager during cancellation"); - } - // Cancel any ongoing recording let audio_manager = app.state::>(); + let recording_was_active = audio_manager.is_recording(); audio_manager.cancel_recording(); // Update tray icon and hide overlay @@ -41,6 +33,11 @@ pub fn cancel_current_operation(app: &AppHandle) { let tm = app.state::>(); tm.maybe_unload_immediately("cancellation"); + // Notify coordinator so it can keep lifecycle state coherent. + if let Some(coordinator) = app.try_state::() { + coordinator.notify_cancel(recording_was_active); + } + info!("Operation cancellation completed - returned to idle state"); }