Skip to content

fix(alsa): drop PCM IO before underrun recovery - #1401

Open
gulbrand wants to merge 1 commit into
RustAudio:masterfrom
gulbrand:fix/alsa-duplex-io-double-borrow
Open

gulbrand wants to merge 1 commit into
RustAudio:masterfrom
gulbrand:fix/alsa-duplex-io-double-borrow

Conversation

@gulbrand

Copy link
Copy Markdown
Contributor

I was testing my DAW on linux now that cpal has duplex for alsa. Ran into a few buffer underruns and my audio engine crapped out. Found a panic in the callback thread:

=== ENGINE PANIC (pid 414610) ===
panicked at /home/gulbrand/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/alsa-0.12.1/src/pcm.rs:149:27:
No hw_params call or additional IO objects allowed
Backtrace:
   ...
   6: <alsa::pcm::PCM>::io_bytes
   7: cpal::host::alsa::duplex::begin_duplex_playback
   8: cpal::host::alsa::duplex::duplex_stream_worker
   ...

I then used an ai agent to help me dig into this, as the error message wasn't quite helpful. It looks like it is a double borrow checked at runtime, and a solution is to let go of the borrow before entering the match arms so that calling recover_duplex isn't called while already holding the borrow on the io object.

It only hits on the match arms that call recover_duplex, so only in error conditions as far as I can tell.

I've confirmed that an underrun no longer panics -- I'm using Omarchy Quattro as my lab test.

An alsa IO created in a match scrutinee lives across the match arms, and alsa allows only one IO per PCM. On duplex playback underrun (EPIPE/EAGAIN or Suspended), recover_duplex() -> begin_duplex_playback() therefore panicked with 'No hw_params call or additional IO objects allowed' when creating the prefill IO on the same PCM. Bind the readi/writei result first so the IO drops before recovery, in both duplex loops and the single-direction input/output loops.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants