1902989 - Crash in [@ <audioipc2_client::stream::CallbackServer as audioipc2::rpccore::Server>::process]

Status: RESOLVED FIXED

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

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This macOS crash signature saw an increase in volume compared to a low base.

Crash report: https://crash-stats.mozilla.org/report/index/3aa02c0e-d301-473d-8680-507820240617

MOZ_CRASH Reason: assertion failed: input_nbytes > 0

Top 10 frames:

0  XUL  MOZ_Crash(char const*, int, char const*)  mfbt/Assertions.h:317
0  XUL  RustMozCrash  mozglue/static/rust/wrappers.cpp:18
1  XUL  mozglue_static::panic_hook  mozglue/static/rust/lib.rs:98
2  XUL  core::ops::function::Fn::call  library/core/src/ops/function.rs:79
3  XUL  <alloc::boxed::Box<F, A> as core::ops::function::Fn<Args>>::call  library/alloc/src/boxed.rs:2034
3  XUL  std::panicking::rust_panic_with_hook  library/std/src/panicking.rs:783
4  XUL  std::panicking::begin_panic_handler::{{closure}}  library/std/src/panicking.rs:649
5  XUL  std::sys_common::backtrace::__rust_end_short_backtrace  library/std/src/sys_common/backtrace.rs:171
6  XUL  rust_begin_unwind  library/std/src/panicking.rs:645
7  XUL  core::panicking::panic_fmt  library/core/src/panicking.rs:72

Comment 1

[Andreas Pehrson [:pehrsons]]

[Tracking Requested - why for this release]: parent process may crash on macOS when using a microphone

To hit this assert it seems to me that audioipc either:

1. set up a stream with no input params (frame size is 0, so input_nbytes is 0) but with a Some duplex_input.
2. received a data callback of 0 frames (making input_nbytes 0)

1 is impossible, so this must be the coreaudio backend calling the data callback with 0 frames. There appears to be no asserts in the backend's output callback (which calls the data callback for duplex streams) or in the resampler (which is what actually calls the data callback). Note the resampler has two modes:

1. passthrough - which fills the input buffer with silence to match the output frames requested from us by the platform -- we could still in theory be requested to produce 0 frames by the platform
2. speex - which actually resamples, and passes to the data callback the number of frames needed for the speex resampler to produce the number of frames requested by the platform. It doesn't appear to be a bug with sample rates at least.

I'll conclude CoreAudio seems to be notifying our output callback that we need to render 0 frames. Let's add a guard as appropriate for this case, and put in some asserts that could catch this earlier than in audioipc.

I would have set S3 because this is latent, but seems exacerbated in 128 so S2 since we don't know what the rate would be in release. If 128 is indeed spiking the reason is likely the enabling of VPIO.

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1902989 - Update cubeb-coreaudio-rs to 8bce3b333a. r?#cubeb-reviewers!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d7c1dfde669
Update cubeb-coreaudio-rs to 8bce3b333a. r=cubeb-reviewers,padenot

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1902989 - Update cubeb-coreaudio-rs to 8bce3b333a. r?#cubeb-reviewers!

Original Revision: https://phabricator.services.mozilla.com/D214954

Comment 5

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Parent process may crash when capturing audio. The trigger is likely the unplugging of the captured device
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: N/A
• Risk associated with taking this patch: Low
• Explanation of risk level: Somewhat speculative fix since we haven't reproduced but an audit suggests it is correct. Should be entirely harmless as it's skipping calling a callback that would not do anything anyway. Note that D215061 is ahead of this upstream so is also included in the stack. It is equally harmless. In fact, in case of an error in a bit of code added in bug 1474175 it will allow for a fallback to older behavior.
• String changes made/needed: None
• Is Android affected?: no

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9d7c1dfde669

Comment 7

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/1175352e1390