Skip to content

Handle null AudioUnit gracefully after failed reinit#276

Open
kinetiknz wants to merge 1 commit intotrailblazerfrom
fix-null-unit-crashes
Open

Handle null AudioUnit gracefully after failed reinit#276
kinetiknz wants to merge 1 commit intotrailblazerfrom
fix-null-unit-crashes

Conversation

@kinetiknz
Copy link
Copy Markdown
Contributor

After a failed device reinit, AudioUnits are closed (nulled) and the stream enters an error state, but calls to set_volume or set_input_processing_params can still arrive before the client processes the error notification. These calls are dispatched to the serial queue, where they encounter a null AudioUnit and crash the process (~95 crash pings per 30 days on macOS).

Fix:

  • Convert assert!(!unit.is_null()) to error returns with logging in set_volume, get_volume, set_input_mute, and set_input_processing_params. Retain debug_assert so unexpected null units are still caught in dev builds.
  • Set stopped=true in the reinit_async error path so the stream is in a consistent non-operational state after failed reinit, preventing callbacks from running on a closed stream.

After a failed device reinit, AudioUnits are closed (nulled) and
the stream enters an error state, but calls to set_volume or
set_input_processing_params can still arrive before the client
processes the error notification.  These calls are dispatched to
the serial queue, where they encounter a null AudioUnit and
crash the process (~95 crash pings per 30 days on macOS).

Fix:
- Convert assert!(!unit.is_null()) to error returns with logging
  in set_volume, get_volume, set_input_mute, and
  set_input_processing_params.  Retain debug_assert so unexpected
  null units are still caught in dev builds.
- Set stopped=true in the reinit_async error path so the stream
  is in a consistent non-operational state after failed reinit,
  preventing callbacks from running on a closed stream.
@kinetiknz kinetiknz requested a review from ChunMinChang March 25, 2026 01:05
@kinetiknz kinetiknz self-assigned this Mar 25, 2026
Copy link
Copy Markdown
Member

@ChunMinChang ChunMinChang left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

fn set_volume(unit: AudioUnit, volume: f32) -> Result<()> {
assert!(!unit.is_null());
debug_assert!(!unit.is_null());
if unit.is_null() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not easy to tell why both assertion and if-check for non-null both exist here without checking the commit message or PR, mind adding some comment somewhere?

This situation makes me wonder what else APIs we shouldn't call after the stream runs into the error state. We don't have a state variable indicating the stream status now, but maybe we should? That would be in another PR I think.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants