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

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 1

[Robert Kaiser]

[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.

Comment 2

[Lawrence Mandel [:lmandel] (use needinfo)]

I see reports for 37+ (strangely 37.0b1 tops the list for the last 7 days). I'm tracking for 38+.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

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

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 5

[(Away)]

(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.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Thanks.

Comment 7

[Lawrence Mandel [:lmandel] (use needinfo)]

(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.

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: 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.

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

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.

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f21414161734

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f21414161734

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 13

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

Comment on attachment 8598333 [details] [diff] [review]
patch

Approved for uplift to aurora. Another async plugin init fix and a topcrash.

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9b759b93773c