Closed Bug 1154898 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Plug-ins, defect, critical)

39 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 --- unaffected
firefox39 + fixed
firefox40 + fixed

People

(Reporter: aklotz, Assigned: billm)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-f9f27a7a-dc84-45e9-abcf-055022150414.
=============================================================
This looks like fallout from bug 1093693.
Flags: needinfo?(wmccloskey)
Assignee: nobody → wmccloskey
Flags: needinfo?(wmccloskey)
[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.
I see reports for 37+ (strangely 37.0b1 tops the list for the last 7 days). I'm tracking for 38+.
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)
(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.
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.
Attached patch patchSplinter Review
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)
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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
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+
You need to log in before you can comment on or make changes to this bug.