crash in mozilla::ipc::MessageChannel::Send(IPC::Message*)

RESOLVED FIXED in Firefox 39

Status

()

Core
Plug-ins
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: aklotz, Assigned: billm)

Tracking

({crash})

39 Branch
mozilla40
crash
Points:
---

Firefox Tracking Flags

(firefox37 unaffected, firefox38 unaffected, firefox39+ fixed, firefox40+ fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
This bug was filed from the Socorro interface and is 
report bp-f9f27a7a-dc84-45e9-abcf-055022150414.
=============================================================
This looks like fallout from bug 1093693.
(Reporter)

Updated

3 years ago
Flags: needinfo?(wmccloskey)
Assignee: nobody → wmccloskey
Flags: needinfo?(wmccloskey)

Comment 1

3 years ago
[Tracking Requested - why for this release]:
This is the #3 browser crash on Dev Edition right now, still ranking in front of the OOM|small that in stable versions is always the leader of the pack.
status-firefox39: --- → affected
tracking-firefox39: --- → ?
I see reports for 37+ (strangely 37.0b1 tops the list for the last 7 days). I'm tracking for 38+.
status-firefox37: --- → wontfix
status-firefox38: --- → affected
status-firefox40: --- → affected
tracking-firefox38: --- → +
tracking-firefox39: ? → +
tracking-firefox40: --- → +
Bill, we just have a few more beta left for the 38 cycle (ESR release btw), any chance you could provide a fix for this? Thanks
Flags: needinfo?(wmccloskey)
Lawrence, I looked for crashes on beta and didn't find any. Can you link to the ones you found? There are different kinds of crashes grouped under MessageChannel::Send, and this bug is only about the ones that include PluginModuleChromeParent::CachedSettingChanged.
Flags: needinfo?(wmccloskey) → needinfo?(lmandel)

Comment 5

3 years ago
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #2)
> I see reports for 37+ (strangely 37.0b1 tops the list for the last 7 days).
> I'm tracking for 38+.

We had a different bug on 37 beta with the same signature (bug 1120331). It was crazy huge volume so I'm not surprised to see residual crashes still coming in.

It's hard to search on the stack so I recommend differentiating them by address. The old bug crashed on address 0x69696956. This one crashes on 0x0.

As far as I can tell, this crash only affects Aurora 39 currently.
status-firefox37: wontfix → unaffected
status-firefox38: affected → unaffected
tracking-firefox38: + → ?
Thanks.
Flags: needinfo?(lmandel)
(In reply to Bill McCloskey (:billm) from comment #4)
> Lawrence, I looked for crashes on beta and didn't find any. Can you link to
> the ones you found? There are different kinds of crashes grouped under
> MessageChannel::Send, and this bug is only about the ones that include
> PluginModuleChromeParent::CachedSettingChanged.

That's my mistake. I was looking at the overall list of crashes for the signature as you expected.
tracking-firefox38: ? → ---
Created attachment 8598333 [details] [diff] [review]
patch

We seem to be properly unregistering the offline observer when the PluginModuleChromeParent is destroyed (or when ActorDestroy happens). I don't think that's the problem though.

Instead, I think we have a PluginModuleChromeParent that never actually loads a plugin and so never opens an IPC channel. I think maybe an object like this just gets leaked forever, given that it exists at shutdown.

Since we register the offline observer in PluginModuleChromeParent's constructor, we might still try to send a message even if no plugin has been loaded yet. The fix is to wait to register the observer until we've definitely opened a channel.

Aaron, this problem is Aurora-only, which leads me to suspect that it's async init-related. It might be worthwhile looking for places where we create a PluginModuleChromeParent and then fail to destroy it if an error happens during initialization.
Attachment #8598333 - Flags: review?(aklotz)
(Reporter)

Comment 9

3 years ago
Comment on attachment 8598333 [details] [diff] [review]
patch

Review of attachment 8598333 [details] [diff] [review]:
-----------------------------------------------------------------

This is definitely the best place to be registering callbacks.
Attachment #8598333 - Flags: review?(aklotz) → review+
https://hg.mozilla.org/mozilla-central/rev/f21414161734
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(Reporter)

Comment 12

3 years ago
Comment on attachment 8598333 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1093693 + async plugin init
[User impact if declined]: crashes (#4 topcrash on Aurora)
[Describe test coverage new/current, TreeHerder]: On m-c, plugin tests
[Risks and why]: Low, trivial patch
[String/UUID change made/needed]: None
Attachment #8598333 - Flags: approval-mozilla-aurora?
Comment on attachment 8598333 [details] [diff] [review]
patch

Approved for uplift to aurora. Another async plugin init fix and a topcrash.
Attachment #8598333 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Comment 14

3 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/9b759b93773c
status-firefox39: affected → fixed
You need to log in before you can comment on or make changes to this bug.