Closed Bug 1762563 Opened 4 months ago Closed 4 months ago

Crash in [@ cubeb_coreaudio::backend::CoreStreamData::setup]

Categories

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

All
macOS
defect

Tracking

()

RESOLVED FIXED
100 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox98 --- unaffected
firefox99 --- unaffected
firefox100 + fixed
firefox101 --- unaffected

People

(Reporter: aryx, Assigned: padenot)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

7 crashes from 2+ machines with macOS 12, oldest reported build ID is 100.0a1 20220326213356. Is this a regression from bug 1760774 which added the code directly executed before?

Crash report: https://crash-stats.mozilla.org/report/index/8d2394bd-c38c-430e-9f39-7fa120220331

MOZ_CRASH Reason: ```assertion failed: (left != right)
left: `0`,
right: `0````

Top 10 frames of crashing thread:

0 XUL RustMozCrash mozglue/static/rust/wrappers.cpp:18
1 XUL mozglue_static::panic_hook mozglue/static/rust/lib.rs:91
2 XUL core::ops::function::Fn::call library/core/src/ops/function.rs:70
3 XUL std::panicking::rust_panic_with_hook library/std/src/panicking.rs:610
4 XUL std::panicking::begin_panic_handler::{{closure}} library/std/src/panicking.rs:502
5 XUL std::sys_common::backtrace::__rust_end_short_backtrace library/std/src/sys_common/backtrace.rs:139
6 XUL rust_begin_unwind library/std/src/panicking.rs:498
7 XUL core::panicking::panic_fmt library/core/src/panicking.rs:116
8 XUL core::panicking::assert_failed_inner library/core/src/panicking.rs:197
9 XUL core::panicking::assert_failed library/core/src/panicking.rs:154
Flags: needinfo?(cchang)
Flags: needinfo?(cchang) → needinfo?(kinetik)

I assume we're hitting this assert as a result of one of the get_device_transport_type calls added to should_use_aggregate_device in bug 1760774 (importing https://github.com/mozilla/cubeb-coreaudio-rs/pull/153) passing a self.input_device.id or self.output_device.id with a value of kAudioObjectUnknown (0). Perhaps we can just remove the assert from get_device_transport_type - it seems fine to query kAudioObjectUnknown, it'll just fail with kAudioHardwareBadObjectError and treat it as a non-aggregate device.

Paul, does that make sense to you?

Flags: needinfo?(kinetik) → needinfo?(padenot)
OS: Unspecified → macOS
Regressed by: 1760774
Hardware: Unspecified → All
Has Regression Range: --- → yes
Severity: S2 → S4
Priority: -- → P3

It's probably the best solution yes, thanks. I'm having a hard time figuring out what assert failed without tracing everything manually...

Flags: needinfo?(padenot)
Assignee: nobody → padenot
Duplicate of this bug: 1764316
Crash Signature: [@ cubeb_coreaudio::backend::CoreStreamData::setup] → [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type]

:padenot, can you provide a patch for beta?

Crash Signature: [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type] → [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type]
Flags: needinfo?(padenot)
Crash Signature: [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type] → [@ (anonymous namespace)::setup_wasapi_stream] [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type]

Yes, likely tomorrow when it's reviewed upstream.

Flags: needinfo?(padenot)

Anything with WASAPI is windows, unrelated to this bug, please file something else is needed I'll look into it.

Crash Signature: [@ (anonymous namespace)::setup_wasapi_stream] [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type] → [@ cubeb_coreaudio::backend::CoreStreamData::setup] [@ cubeb_coreaudio::backend::device_property::get_device_transport_type]

Merging in 1764574, not hard to uplift I think.

Depends on: 1764574

Changing the priority to P1 as the bug is tracked by a release manager for the current beta.
See Triage for Bugzilla for more information.
If you disagree, please discuss with a release manager.

Priority: P3 → P1

Comment on attachment 9272283 [details]
Bug 1762563 - Update cubeb-coreaudio-rs to 39f7e69 on beta to fix an assert crash. r?#cubeb-reviewers

Beta/Release Uplift Approval Request

  • User impact if declined: Firefox crashes when an audio output device is removed.
  • 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 essentially reverts part of the code to what it currently is in release.
  • String changes made/needed:
Attachment #9272283 - Flags: approval-mozilla-beta?

:padenot this hasn't landed yet, is this still something we want to get into b8?

Flags: needinfo?(padenot)

(In reply to Dianna Smith [:diannaS] from comment #11)

:padenot this hasn't landed yet, is this still something we want to get into b8?

Certainly yes, it's not risky and fixes a crash.

Flags: needinfo?(padenot)

Comment on attachment 9272283 [details]
Bug 1762563 - Update cubeb-coreaudio-rs to 39f7e69 on beta to fix an assert crash. r?#cubeb-reviewers

Approved fro 100.0b8

Attachment #9272283 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

There are also crash reports for v101 with this signature. Should those get a new bug?

Flags: needinfo?(padenot)

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #15)

There are also crash reports for v101 with this signature. Should those get a new bug?

Yes please.

Flags: needinfo?(padenot)

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

(In reply to Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout) from comment #15)

There are also crash reports for v101 with this signature. Should those get a new bug?

Yes please.

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1765969 and attached the patch there.

You need to log in before you can comment on or make changes to this bug.