Closed Bug 1269472 Opened 8 years ago Closed 8 years ago

Hard crash in WebRTC when disconnecting headset

Categories

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

48 Branch
x86_64
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 --- affected
firefox48 --- fixed
firefox49 --- affected
firefox-esr45 --- affected
firefox50 --- affected

People

(Reporter: ianbicking, Assigned: achronop)

References

Details

Crash Data

Attachments

(1 file)

Twice (reproducible?) I got a hard crash in Firefox when disconnecting my Logitech G930 wireless headset on Mac, using Dev Edition/48.

Crashes:
https://crash-stats.mozilla.com/report/index/c0dda81f-323a-4f91-9a01-d531a2160502
https://crash-stats.mozilla.com/report/index/9d1a70e6-3523-4bd6-9189-fa43b2160502
Flags: needinfo?(mreavy)
See Also: → 1267288
This is   assert(r == noErr); in cubeb's callback
Probably we shouldn't assert that, but instead handle errors.
Rank: 10
Component: Client → Audio/Video: cubeb
Flags: needinfo?(mreavy)
Priority: -- → P1
Product: Hello (Loop) → Core
Assignee: nobody → achronop
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
Version: unspecified → 48 Branch
This is the pull request pushed to cubeb github repo that handles the error case above: 
https://github.com/kinetiknz/cubeb/pull/107
libcubeb update containing fix landed in bug 1270004.
Depends on: 1270004
Crash Signature: [@ libsystem_kernel.dylib@0x16f06]
We should cherry-pick the fix for this from upstream, or (if you prefer and it makes sense - quite possibly not) we could import all of upstream into 48, or (more possibly but still more work than a cherry-pick) make an upstream branch to include the cherry-pick release and import that.

Kinetik, your preference/suggestion?
Flags: needinfo?(kinetik)
(In reply to Randell Jesup [:jesup] from comment #5)
> We should cherry-pick the fix for this from upstream, or (if you prefer and
> it makes sense - quite possibly not) we could import all of upstream into
> 48, or (more possibly but still more work than a cherry-pick) make an
> upstream branch to include the cherry-pick release and import that.
> 
> Kinetik, your preference/suggestion?

Uplifting just this fix is the safest, but looking at the aurora to central diff, if we take everything we also get several important WASAPI fixes and a few other minor changes (non-fatal WinMM input stream failure, PulseAudio minimum latency reduction).  We certainly want the WASAPI fixes in 48, so the simplest option is to uplift the current central code to aurora.  It has been baking there since May 6th, so it's probably pretty safe at this point.
Flags: needinfo?(kinetik)
Depends on: 1273349
I filed bug 1273349 to cover uplifting all of the changes to aurora.
(In reply to Matthew Gregan [:kinetik] from comment #7)
> I filed bug 1273349 to cover uplifting all of the changes to aurora.

After discussing this with jesup on IRC, we'll just cherry-pick the patch for this bug for now.
Attached patch bug1269472.patchSplinter Review
This has already been review and landed in central via bug 1270004.  This patch is to uplift this specific fix to aurora.

Approval Request Comment
[Feature/regressing bug #]: this bug
[User impact if declined]: crash on OS X when disconnecting audio hardware while active
[Describe test coverage new/current, TreeHerder]: manual testing required
[Risks and why]: very low, converts an assertion into an error return; baked on central since May 6th
[String/UUID change made/needed]: none
Attachment #8753215 - Flags: review?(rjesup)
Attachment #8753215 - Flags: approval-mozilla-aurora?
Attachment #8753215 - Flags: review?(rjesup) → review+
Comment on attachment 8753215 [details] [diff] [review]
bug1269472.patch

Fix a crash, taking it.
Attachment #8753215 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/049347855964

should this also be landed on m-c ?
Target Milestone: --- → mozilla48
(In reply to Carsten Book [:Tomcat] from comment #11)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/049347855964

Thanks!

> should this also be landed on m-c ?

It landed on m-c as part of the larger update in bug 1270004.
The fix is landed on m-c.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Crash volume for signature 'libsystem_kernel.dylib@0x16f06':
 - nightly (version 50): 5 crashes from 2016-06-06.
 - release (version 47): 15234 crashes from 2016-05-31.
 - esr     (version 45): 4298 crashes from 2016-04-07.

Crash volume on the last weeks:
             Week N-1   Week N-2   Week N-3   Week N-4   Week N-5   Week N-6   Week N-7
 - nightly          1          0          3          0          0          0          1
 - release       2588       2485       2421       2328       2411       2159        630
 - esr            499        463        412        427        413        377        241

Affected platform: Mac OS X
You need to log in before you can comment on or make changes to this bug.