Closed Bug 1041226 Opened 10 years ago Closed 10 years ago

GMPChild should finish IPC setup before calling Crashreporter IPC method

Categories

(Core :: WebRTC, defect)

defect
Not set
normal
Points:
5

Tracking

()

RESOLVED FIXED
mozilla34
Iteration:
34.1
Tracking Status
firefox32 --- unaffected
firefox33 + fixed
firefox34 --- fixed

People

(Reporter: jesup, Assigned: gfritzsche)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Attachments

(2 files, 1 obsolete file)

Attached 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....
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
Attachment #8459235 - Attachment is obsolete: true
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+
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: 10 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.

Attachment

General

Created:
Updated:
Size: