GMPChild should finish IPC setup before calling Crashreporter IPC method

RESOLVED FIXED in Firefox 33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jesup, Assigned: gfritzsche)

Tracking

(Blocks 1 bug, {crash})

unspecified
mozilla34
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox32 unaffected, firefox33+ fixed, firefox34 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Posted file stack
+++ This bug was initially created as a clone of Bug #1039575 +++

Bug 1039575 broke --enable-crashreporter builds (Nightly) due to an assert when registering the crashreporter ID during GMP initialization:

[time:1405803882621308][19039->-1][PGMPChild] Sending Msg_PCrashReporterConstructor([TODO])
[19039] ###!!! ABORT: not on worker thread!: 'mWorkerLoopID == MessageLoop::current()->id()', file ../../dist/include/mozilla/ipc/MessageChannel.h, line 362

I believe the assert is erroneous here, and is caused by incomplete setup before it tries to send the ID.
Whiteboard: [leave-open][webrtc-uplift]
Comment on attachment 8459235 [details] [diff] [review]
disable crashreporter in GMP plugins until it's ready

Tested on m-c with --enable-crashreporter
Attachment #8459235 - Flags: review?(georg.fritzsche)
Comment on attachment 8459235 [details] [diff] [review]
disable crashreporter in GMP plugins until it's ready

Sign-off for quick unbustage for jesup, deeper look tomorrow or monday.
Attachment #8459235 - Flags: review?(georg.fritzsche) → review+
Landed on m-c and inbound because it was a hidden requirement of the stuff we landed on m-c last night.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9383ccd71289
https://hg.mozilla.org/mozilla-central/rev/1e7f2af8d929
See Also: → 1041244
Georg -- I believe you said you'd take this since bsmedberg is on PTO
Assignee: nobody → georg.fritzsche
(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #5)
> Georg -- I believe you said you'd take this since bsmedberg is on PTO

Correct.
I'm torn for the points without spending time investigating first, so i'll just lean towards over-estimating.
Points: --- → 8
Flags: firefox-backlog+
Status: NEW → ASSIGNED
Iteration: --- → 33.3
QA Whiteboard: [qa?]
Hi Georg, can you mark this bug as [qa+] or [qa-] for verification.
Flags: needinfo?(georg.fritzsche)
Blocks: 1038961
(In reply to Marco Mucci [:MarcoM] from comment #8)
> Hi Georg, can you mark this bug as [qa+] or [qa-] for verification.

qa- as this path will be covered by the other OpenH264/... testing once we can re-enable the crash reporting.
QA Whiteboard: [qa?] → [qa-]
Flags: needinfo?(georg.fritzsche)
Does this problem only happen with debug builds?
For what it's worth, this bug doesn't happen on the Mac when I start the sandbox (call sandbox_init()) in GMPChild::OnChannelConnected() -- after IPC is fully set up.  I tested with a debug build.

We may want to consider doing this on all platforms.
I believe the proximate cause is an assertion; however I'm not sure if it will work or not if debug assertions are off.

Holding off on stuff like this until IPC is fully up makes sense....
Duplicate of this bug: 1041244
Iteration: 33.3 → 34.1
Summary: Crashreporter init in GMP plugin triggers assert in IPC → GMPChild should finish IPC setup before calling Crashreporter IPC method
Comment on attachment 8460226 [details] [diff] [review]
Open IPC channel before using IPC methods

Nicolas, i heard you may be able to give input on IPC things?
It looks like our IPC peer availability is not good right now.
Attachment #8460226 - Flags: feedback?(nical.bugzilla)
Comment on attachment 8460226 [details] [diff] [review]
Open IPC channel before using IPC methods

Checking through mozilla::ipc::Open() et al, i'm fairly sure this should be fine.
Attachment #8460226 - Flags: review?(rjesup)
Points: 8 → 5
Attachment #8460226 - Flags: review?(rjesup) → review+
Whiteboard: [leave-open][webrtc-uplift] → [leave-open][openh264-uplift]
Attachment #8460226 - Flags: feedback?(nical.bugzilla) → feedback+
QA Contact: drno
Blocks: 1043531
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa4338c4f0cc
Whiteboard: [leave-open][openh264-uplift] → [openh264-uplift]
Target Milestone: --- → mozilla34
https://hg.mozilla.org/mozilla-central/rev/aa4338c4f0cc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8460226 [details] [diff] [review]
Open IPC channel before using IPC methods

Forgot to list this with the other openh264 uplift requests to bring it up to inbound state.

Approval Request Comment
[Feature/regressing bug #]: bsmedberg GMP crashreporter landing

[User impact if declined]: Crashreporter doesn't get inited for GMP plugins

[Describe test coverage new/current, TBPL]: WIll be covered in the general GMP crashreporting coverage under development

[Risks and why]: Very low risk; on central for some time

[String/UUID change made/needed]: none
Attachment #8460226 - Flags: approval-mozilla-aurora?
Attachment #8460226 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.