Closed Bug 1093693 Opened 10 years ago Closed 10 years ago

[e10s] Plugin hangs with new plugin IPC code

Categories

(Core Graveyard :: Plug-ins, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(e10sm4+)

RESOLVED FIXED
mozilla36
Tracking Status
e10s m4+ ---

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(1 file)

Since bug 641685 I've seen a couple hangs caused by deadlocks between all our different processes. I looked at one today and it works like this:

1. chrome sends CPOW message to content; this message sits in the queue and is never handled
2. content process asks plugin process to start a new plugin instance (this is an intr message)
3. plugin process sends NPN_GetValue_WithBoolReturn intr message to chrome process

At this point we deadlock. Jim pointed out this scenario to me, but I dismissed it because CPOWs aren't supposed to be able to trigger plugin code. However, in this case, the CPOW doesn't need to do anything. It just needs to make the chrome process block.

I think we'll have to go through all the sync messages from the plugin process to the chrome process and eliminate them. We can either pass them to the content process instead (which could then forward them to the chrome process) or we can cache the results at startup.
We should discuss tomorrow. I am a little skeptical that we'll ever be able to prevent the plugin process from blocking on the chrome process because of SendMessage message related to focus/window moving.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> We should discuss tomorrow. I am a little skeptical that we'll ever be able
> to prevent the plugin process from blocking on the chrome process because of
> SendMessage message related to focus/window moving.

Can I be looped in on said discussion, please? I'd like to be able to determine how bug 998863 will be affected by this (and come to think of it, those patches could probably mitigate this problem).
Blocks: 1095807
I believe I have seen such hangs:
Symptoms: Window gets (not responding) in Title, white overlay.
Workaround: kill plugin_container.exe hosting the plugins, this will make Nightly recover
BuildID: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141108030205 CSet: eb0d3b3c0b22

How do I debug such situations?
Blocks: 1095930
Attached patch plugin-settingsSplinter Review
This patch removes sync chrome <-> plugin interactions for NPN_UserAgent, NPN_GetValue_WithBoolReturn, and GetNativeCursorsSupported. The one where I most commonly see deadlocks is NPN_GetValue_WithBoolReturn.

The main piece of work left is fixing NP_Shutdown. I sometimes get deadlocks caused by that as well. I have a plan for that and I think I'll be able to get something up for that soon.
Attachment #8521555 - Flags: review?(benjamin)
Blocks: 1090573
No longer blocks: 1095807
Blocks: e10s-plugins
Bill... is there an ETA on this patch?  I'm hoping it fixes my LastPass induced Flash crashes so I can turn on e10s permanently.  LP is essential for me.
Comment on attachment 8521555 [details] [diff] [review]
plugin-settings

These hangs are affecting a lot of people. I'll take a review from whoever can do it sooner.
Attachment #8521555 - Flags: review?(jmathies)
Comment on attachment 8521555 [details] [diff] [review]
plugin-settings

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

Looks good to me. Since bsmedberg is really backed up right now i'd suggest we land this and address any nits as a follow up.

::: dom/plugins/ipc/PPluginModule.ipdl
@@ +25,5 @@
> +  bool vasdEnabled;
> +  bool isOffline;
> +  bool supportsXembed;
> +  bool supportsWindowless;
> +

nit - extra line here
Attachment #8521555 - Flags: review?(jmathies) → review+
Attachment #8521555 - Flags: review?(benjamin)
Is Bill on vacation or away on business?  I would really like to see this hit inbound asap.
I started seeing strange test failures with this patch when I pushed it to try yesterday. It can't land until those are resolved.
Gotcha.
Silly problem: I wasn't initializing an unused field and IPDL was complaining.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c37c5083952
Running with this patch and LastPass no longer 'breaks' Flash.  Nice job Bill.
https://hg.mozilla.org/mozilla-central/rev/8c37c5083952
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.