Skip to content

fix(alsa): hanging double-opens with BufferSize::Fixed om some drivers#1214

Merged
roderickvd merged 5 commits into
masterfrom
fix/alsa-double-open
May 23, 2026
Merged

fix(alsa): hanging double-opens with BufferSize::Fixed om some drivers#1214
roderickvd merged 5 commits into
masterfrom
fix/alsa-double-open

Conversation

@roderickvd
Copy link
Copy Markdown
Member

The range validation for BufferSize::Fixed was calling default_output_config() before opening the stream, which opened and closed the PCM device once just to get the buffer size range. This may leave the driver in a bad state for the subsequent streaming open.

Second, to defend against hanging processes in such an event, the worker now checks PCM state when poll() times out.

Fixes #1157

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the ALSA backend to avoid an extra open/close cycle when validating BufferSize::Fixed (which can leave some drivers in a bad state), and adds a defensive PCM-state check when poll() times out to prevent hangs when error events aren’t delivered.

Changes:

  • Move BufferSize::Fixed range validation to set_hw_params_from_format() so it uses the same PCM handle as the streaming open (avoids a second open/close).
  • On poll() timeout, query PCM state and surface Disconnected / XRun / Suspended conditions even if POLLERR/POLLHUP wasn’t delivered.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/host/alsa/mod.rs Outdated
@roderickvd roderickvd changed the title fix(alsa): hanging double-opens with fixed buffersize om some drivers fix(alsa): hanging double-opens with BufferSize::Fixed om some drivers May 20, 2026
@DanielHabenicht
Copy link
Copy Markdown

As I wrote in the issue there are no apparent changes for me, but it also does not negatively effect the current behavior on the rasperry pi zero w, so I would say its better than before.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/host/alsa/mod.rs Outdated
Comment thread CHANGELOG.md Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/host/alsa/mod.rs Outdated
Comment thread src/host/alsa/mod.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread src/host/alsa/mod.rs
Comment thread src/host/alsa/mod.rs
Comment thread src/host/alsa/mod.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread src/host/alsa/mod.rs Outdated
Comment thread src/host/alsa/mod.rs
Comment thread src/host/alsa/mod.rs
Comment thread src/lib.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/host/alsa/mod.rs:595

  • default_config treats ErrorKind::UnsupportedConfig from supported_configs(stream_t) as “device does not support {dir}”, but UnsupportedConfig can also mean other configuration/parameter failures. Since Device already tracks direction via self.direction/supports_input()/supports_output(), consider checking that first and returning ErrorKind::UnsupportedOperation without relying on errno/kind mapping from snd_pcm_open(). This would also make behavior consistent for supported_*_configs() callers on the wrong direction.
        let mut formats: Vec<_> = {
            match self.supported_configs(stream_t) {
                // EINVAL when querying direction the device does not support (input-only or output-only)
                Err(err) if err.kind() == ErrorKind::UnsupportedConfig => {
                    let dir = match stream_t {
                        alsa::Direction::Capture => "input",
                        alsa::Direction::Playback => "output",
                    };
                    return Err(Error::with_message(
                        ErrorKind::UnsupportedOperation,
                        format!("Device does not support {dir}"),
                    ));

Comment thread src/lib.rs Outdated
Comment thread src/host/alsa/mod.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/host/alsa/mod.rs Outdated
Comment thread src/lib.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread src/host/alsa/mod.rs
@roderickvd roderickvd force-pushed the fix/alsa-double-open branch from 3c0b18b to d6fe895 Compare May 23, 2026 19:04
@roderickvd roderickvd merged commit 3e73d82 into master May 23, 2026
32 checks passed
@roderickvd roderickvd deleted the fix/alsa-double-open branch May 23, 2026 19:05
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.

alsa: xrun on RPi Zero since v0.17

3 participants