1269472 - Hard crash in WebRTC when disconnecting headset

Status: RESOLVED FIXED

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

Description

[Ian Bicking (:ianbicking)]

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

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

This is   assert(r == noErr); in cubeb's callback
Probably we shouldn't assert that, but instead handle errors.

Comment 2

[Alex Chronopoulos [:achronop]]

This is the pull request pushed to cubeb github repo that handles the error case above: 
https://github.com/kinetiknz/cubeb/pull/107

Comment 3

[Matthew Gregan [:kinetik]]

libcubeb update containing fix landed in bug 1270004.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

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?

Comment 6

[Matthew Gregan [: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.

Comment 7

[Matthew Gregan [:kinetik]]

I filed bug 1273349 to cover uplifting all of the changes to aurora.

Comment 8

[Matthew Gregan [:kinetik]]

(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.

Comment 9

[Matthew Gregan [:kinetik]]

Attachment: bug1269472.patch

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

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8753215 [details] [diff] [review]
bug1269472.patch

Fix a crash, taking it.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/049347855964

should this also be landed on m-c ?

Comment 12

[Matthew Gregan [:kinetik]]

(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.

Comment 13

[Alex Chronopoulos [:achronop]]

The fix is landed on m-c.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

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