fix(audio): move blocking cpal work off the main thread + lock-free is_recording#1716
Open
xronocode wants to merge 1 commit into
Open
fix(audio): move blocking cpal work off the main thread + lock-free is_recording#1716xronocode wants to merge 1 commit into
xronocode wants to merge 1 commit into
Conversation
…s_recording
Synchronous #[tauri::command] handlers run inline on the webview/main run
loop, and the audio manager guards its state with a std Mutex held across
blocking CoreAudio syscalls (cpal stream start/stop, device enumeration).
A worker holding that mutex across a slow device open/close (Bluetooth/USB
mic) serializes the main thread, freezing the UI (spinning beachball).
Fix A: is_recording() reads a lock-free Arc<AtomicBool> mirror of the
"state in {Recording, Stopping}" membership, flipped at the state
transitions, instead of locking `state`. The hot-path UI poll can no longer
deadlock against a worker holding `state`.
Fix B: the four cpal-running commands (update_microphone_mode,
get_available_microphones, set_selected_microphone,
get_available_output_devices) become async and run their blocking cpal work
via tokio::task::spawn_blocking. Tauri's invoke is identical for sync/async
commands, so there is no frontend/binding change.
Live verification (Bluetooth/USB mic recording + device change mid-use) is
left to manual testing; concurrency/hardware behavior is not unit-testable.
Owner
|
Thanks I will take a look at this soon, I appreciate you submitting the fix and issue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before Submitting This PR
(Duplicate search done before opening #1715: no earlier open or closed issue/PR addresses the intermittent UI beachball while audio devices open/close.
#1354(poisoned mutexes inDropimpls) is a different problem: it handles panic-during-Dropon quit, not the live main-thread hang during recording. This PR targets the main-threadMutex-across-CoreAudio contention.)Human Written Description
I noticed Handy could intermittently beachball while recording, most often when using Bluetooth/USB microphones or changing audio devices. In the worst case the app stopped responding and had to be force-quit.
This change moves the blocking audio work away from UI-facing command paths and makes the recording-state poll lock-free, so normal recording/device operations should not freeze the app.
Related Issues/Discussions
Fixes #1715
Community Feedback
Bug fix: a production UI freeze during recording, amplified by Bluetooth/USB microphones. Tracked in #1715; not previously reported upstream as a distinct issue before this bug report. Sibling to #1644.
Testing
Root cause (class): synchronous
#[tauri::command] pub fn ...runs inline on the webview/main run loop. The audio manager guards its state withstd::sync::Mutex(blocks the OS thread, no timeout) and holds it across blocking CoreAudio syscalls (cpal stream start/stop intry_start_recording/schedule_lazy_close, and device enumeration). Any worker holding that mutex across a slow device open/close serializes the main thread, causing deadlock/freeze. The frontend pollsis_recording(), which lockedstate, the most-polled main-thread contender.Two manifestations, both still present on
main@d861e24:is_recording()lockedstate(src-tauri/src/managers/audio.rs,is_recording).pub fnon the main run loop:update_microphone_mode,get_available_microphones,set_selected_microphone,get_available_output_devices(src-tauri/src/commands/audio.rs).What changed:
managers/audio.rs): added a lock-freerecording_active: Arc<AtomicBool>mirroring the "statein {Recording, Stopping}" membership, flipped at the state transitions.is_recording()reads the atomic instead of taking thestatemutex. (Note:mainnow has aStoppingintermediate variant inRecordingState, andis_recording()already returnedtrueforRecording | Stopping; the atomic faithfully mirrors both: flippedtrueonIdletoRecording,falseonStoppingtoIdleand on cancelRecordingtoIdle. TheRecordingtoStoppingtransition needs no flip becauseStoppingstays in the active set.)commands/audio.rs): the four cpal-running commands are nowasyncand run their blocking cpal work viatokio::task::spawn_blocking. Settings reads/writes stay inline (fast).#[specta::specta]and all return types are preserved; Tauri'sinvokeis identical for sync/async commands, so there is no frontend/binding change (bindings regenerated cleanly).Build verification (on
main@d861e24, branchfix/main-thread-audio-hang):cargo check --lockedto 0 errorscargo clippy --lockedto no new lints (only one pre-existing warning: unused assignmentmodel_takes_initial_promptintranscription.rs:1168, unrelated to this change)cargo fmt --checkto cleanScope: 2 files, shared code only:
managers/audio.rs(Fix A: new atomic field + 3 flip sites + lock-freeis_recording),commands/audio.rs(Fix B: 4 syncpub fntoasync+spawn_blocking). No recording semantics changed; no frontend, bindings, or other managers touched.Manual verification needed (hardware/concurrency, not unit-testable):
tsc(bindings) green.Known follow-up (intentionally NOT in this PR):
try_start_recording/schedule_lazy_closestill holdstateacross the cpal open/close, but after A+B only worker threads contend there (no main-thread hot path remains), so no UI freeze. Restructuring them is a recording-semantics risk for no user-visible gain.Screenshots/Videos (if applicable)
Native beachball produces no static screenshot; repro is "record with a Bluetooth/USB mic, change device or toggle mic mode mid-session, and the UI freezes". Happy to capture a no-freeze video on request.
AI Assistance
If AI was used:
main(adapting Fix A tomain's newerRecordingState::Stoppingvariant) and running the verification. The hang reproduction and the decision to move cpal work off the main thread were driven by real Bluetooth/USB-mic testing in the fork.