1613156 - Crash in [@ cubeb_coreaudio::backend::mixer::get_default_channel_order]

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P2)

Description

[Alex Chronopoulos [:achronop]]

This bug is for crash report bp-16b21328-f194-48d0-b531-2c63d0200202.

Top 10 frames of crashing thread:

0 XUL RustMozCrash mozglue/static/rust/wrappers.cpp:17
1 XUL mozglue_static::panic_hook mozglue/static/rust/lib.rs:89
2 XUL core::ops::function::Fn::call src/libcore/ops/function.rs:69
3 XUL std::panicking::rust_panic_with_hook src/libstd/panicking.rs:477
4 XUL std::panicking::continue_panic_fmt src/libstd/panicking.rs:380
5 XUL std::panicking::begin_panic_fmt src/libstd/panicking.rs:335
6 XUL cubeb_coreaudio::backend::mixer::get_default_channel_order third_party/rust/cubeb-coreaudio/src/backend/mixer.rs:45
7 XUL cubeb_coreaudio::backend::CoreStreamData::setup third_party/rust/cubeb-coreaudio/src/backend/mixer.rs:210
8 XUL <cubeb_coreaudio::backend::AudioUnitContext as cubeb_backend::traits::ContextOps>::stream_init third_party/rust/cubeb-coreaudio/src/backend/mod.rs:2169
9 XUL cubeb_backend::capi::capi_stream_init third_party/rust/cubeb-backend/src/capi.rs:155

Low traffic. Channels can be zero when there is no soundcard in the system.

Comment 1

[C.M.Chang[:chunmin]]

Alex, do you have a way to reproduce it? Not sure why the output_stream_params.channels() become 0 here. According the validate_stream_params in cubeb.c, we should return an error in this case.

Comment 2

[Alex Chronopoulos [:achronop]]

I know it's odd. No, I'm sorry I haven't reproduced it, I have found it in the crash-stats during regular triage. Feel free to lower the priority if it is not actionable.

Comment 3

[Paul Adenot (:padenot)]

It must be when we disable the last audio device of the machine. On OSX, one can do this by removing the kext for the default audio device I think.

Comment 4

[C.M.Chang[:chunmin]]

(In reply to Alex Chronopoulos [:achronop] from comment #0)

Low traffic. Channels can be zero when there is no soundcard in the system.

(In reply to Paul Adenot (:padenot) from comment #3)

It must be when we disable the last audio device of the machine. On OSX, one can do this by removing the kext for the default audio device I think.

Yeah, you guys are right. I traced the wrong path. From the stack, the crash caused by hw_channels = 0 instead of output_stream_params.channels() = 0. I will try reproducing it by the following commands

sudo kextunload /System/Library/Extensions/AppleHDA.kext # Remove system sound devices
sudo kextload /System/Library/Extensions/AppleHDA.kext # Re-enable system sound devices

Comment 5

[C.M.Chang[:chunmin]]

Attachment: GitHub Pull Request

Comment 6

[C.M.Chang[:chunmin]]

I didn't find a way to reproduce it.

If there is no output device when the stream is initialized, the initialization will fail. If there is only one output device in the system, and the stream is initialized successfully with that output, then when the output device is unplugged, the stream reinitialization will fail before hitting that assertion. Since we check if there is an available device before the stream is reinitialized.

Maybe there is some weird/customized output hardware(something like soundflower?) that reports it has 0 output channel to the system. Anywat, the easy solution is to bail out if the output device has no output channel (0 output channel).

Comment 7

[C.M.Chang[:chunmin]]

Attachment: Bug 1613156 - Update cubeb-coreaudio to 7fe03b4. r?padenot

Pick commits:

• 7fe03b4: Bail out if the output device has no output channel (#50)
• ad74bad: Update README
• 3f38b17: Replace *.get_mut() by *.store() on all the Atomic* (#47)
• b1bc781: Run tests by sanitizers (#46)

Comment 8

[Pascal Chevrel:pascalc]

Only a couple of crashes in nightly/Beta74, fix-optional for 74.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24b97b0c24f0
Update cubeb-coreaudio to 7fe03b4. r=padenot

Comment 10

[C.M.Chang[:chunmin]]

Comment on attachment 9126231 [details]
Bug 1613156 - Update cubeb-coreaudio to 7fe03b4. r?padenot

Beta/Release Uplift Approval Request

• User impact if declined: This patch solves some issues in the new Rust audio backend on Mac OSX, which is introduced in Firefox 74 in bug 1606690.
  The patch solves not only this crash(bug 1613156) but also some data race issues in bug 1614389 that possibly causes crashes in bug 1595457 and bug 1594426. Without this patch, users on Mac OSX may encounter the crashes mentioned in the last sentence.

Those crashes are hard to be reproduced since it depends on the timing of operations and the system audio hardware. But the good news is the volumes of those crashes are low in Nightly (and Firefox 74). The crash in bug 1613156 should be suppressed in this patch, and other crashes possibly benefit from this fix.

• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change doesn't change the main logic. What it does is to

• Return an error if the system output device reports it has no output channel
• Make sure the atomic variables are accessible at most one thread at the same time

• String changes made/needed: none

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/24b97b0c24f0

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9126231 [details]
Bug 1613156 - Update cubeb-coreaudio to 7fe03b4. r?padenot

Fixes some cubeb crashes and races. Approved for 74.0b4.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/04a0b8a0c7f2