1171496 - crash in mozilla::gmp::GMPParent::ChildTerminated()

Status: RESOLVED FIXED

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

Description

[Ada [:adalucinet]]

This bug was filed from the Socorro interface and is 
report bp-9f896f96-44e5-4c04-ae13-2b7942150604.
=============================================================

Crash encountered while closing Firefox (38.0.5 build 4 and latest 41.0a1) after performing some calls via CiscoSparks webapp (https://web.ciscospark.com):
bp-9f896f96-44e5-4c04-ae13-2b7942150604
bp-9d3aa3cb-9178-40d1-9c8f-3b8e32150604

More reports:
https://crash-stats.mozilla.com/signature/?signature=mozilla%3A%3Agmp%3A%3AGMPParent%3A%3AChildTerminated%28%29&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&page=1

Comment 1

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

Ethan - I think this is a dup of a bug you had a wallpaper patch for; am I correct?

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

I think this is different. It looks like what I encountered in bug 1169129, which I fixed in https://bugzilla.mozilla.org/attachment.cgi?id=8615089 .

Comment 3

[Ethan Hugg [:ehugg]]

To me this does look the same as what I wallpapered in bug 1163239 which is only in FF41.  The report sounds like it is on shutdown after a successful Spark call.

Comment 4

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

(In reply to Chris Pearce (:cpearce) from comment #2)
> I think this is different. It looks like what I encountered in bug 1169129,
> which I fixed in https://bugzilla.mozilla.org/attachment.cgi?id=8615089 .

Can we try that again in current nightly, since cpearce's patch landed on 6/5 in m-c?  If the problem is gone in Nightly, let's try a build of 39 with and without the patch (https://bugzilla.mozilla.org/attachment.cgi?id=8615089).  If we have confirmation this resolves the problem, let's get it uplifted to 40 and 39 if possible.

Comment 5

[Ada [:adalucinet]]

(In reply to Randell Jesup [:jesup] from comment #4)
> (In reply to Chris Pearce (:cpearce) from comment #2)
> > I think this is different. It looks like what I encountered in bug 1169129,
> > which I fixed in https://bugzilla.mozilla.org/attachment.cgi?id=8615089 .
> 
> Can we try that again in current nightly, since cpearce's patch landed on
> 6/5 in m-c?  

Unable to reproduce the crash with latest Nightly (from 2015-06-08).

Comment 6

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

Chris: can you spin a try build for adalucient to verify in 39, and if it verifies this fixes it mark for uplift?  thanks.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Randell Jesup [:jesup] from comment #6)
> Chris: can you spin a try build for adalucient to verify in 39, and if it
> verifies this fixes it mark for uplift?  thanks.

Sure. I was hoping to uplift all of bug 1169129, but there have been e10s related regressions and edge cases which have delayed it.

Alexandra, can you please test this build, it should be complete by the time you start work:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6a0350749420

Comment 8

[Ada [:adalucinet]]

(In reply to Chris Pearce (:cpearce) from comment #7)
> Alexandra, can you please test this build, it should be complete by the time
> you start work:
> https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/
> [e-mail]-6a0350749420

I confirm that with the provided try build, the crash is no longer reproducible after performing various calls. Tested with CiscoSparks webapp, under Windows 7 64-bit and Ubuntu 14.04 64-bit, and the CiscoSparks native client on Mac OS X 10.9.5 (because no build was available for OS X).

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1169129 Patch 5: Make GMPParent hold a self ref while child process is alive

Bringing Bug 1169129 patch 5 over here, so that we can keep the uplift flags in Bug 1169129 for the other patches in Bug 1169129, and use the flags in this bug to track the issue this bug cares about; the crash.

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8623303 [details] [diff] [review]
Bug 1169129 Patch 5: Make GMPParent hold a self ref while child process is alive

(This is Bug 1169129 patch 5, but I'm requesting uplift in this bug as the rest of the patches in Bug 1169129 are not ready to be uplifted, and I *do* want to uplift the rest of the patches in Bug 1169129 so I don't want Bug 1169129's flags to be marked as fixed in aurora/beta when all the patches aren't uplifted.)

Approval Request Comment
[Feature/regressing bug #]: WebRTC with OpenH264 and EME

[User impact if declined]: There are some cases when using EME and OpenH264 where Firefox can crash on shutdown; this patch fixes that.

[Describe test coverage new/current, TreeHerder]: We have lots of EME and OpenH264 tests, and a few of the gtests we had were failing without this patch (particularly with the other patches in bug 1169129 applied).

[Risks and why]: We're messing with object life cycles here, so there's always some risk when you do that. However this patch lengthens the object's life, not shortens it, and this patch has been on Nightly for a couple of weeks, so I think it's safe enough.

[String/UUID change made/needed]: None

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch rebased onto Aurora

Patch rebased onto aurora 40.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a02c444eedc

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

[Tracking Requested - why for this release]: Since this is an OpenH264/WebRTC and EME crash, we should track this to ensure it doesn't slip through the cracks.

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch rebased onto Beta

Patch rebased onto Beta 39.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa517d43b31d

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8623303 [details] [diff] [review]
Bug 1169129 Patch 5: Make GMPParent hold a self ref while child process is alive

Approved for uplift to aurora and beta; WebRTC/EME shutdown crash fix.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/95a2427334b7

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/46b6c71bb1c1