1099410 - [e10s] Plugin loading is unexpectedly re-entrant

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

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

Attachment: plugin-load-order

I saw an assertion today in a debug build that pointed out this possibility. Here's what can happen:

1. content process calls PluginModuleContentParent::LoadModule
2. it sends an intr message to chrome asking for the plugin
3. while waiting for a response, the content process dispatches various incoming messages, which can include HTTP data
4. the new data causes us to try to load more plugins while LoadModule is already on the stack

The eventual outcome is that we assert here:
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PluginModuleParent.cpp#140
We're assuming that only one LoadModule call will be on stack at once, and that's being violated.

The fundamental problem is that we have a crummy way of creating the plugin bridge:
1. content process sends LoadModule to chrome using intr message
2. chrome process creates bridge between plugin and content
3. a new AllocPPluginModuleContentParent message is sent to the content process while it's waiting for the LoadModule response
4. the LoadModule response comes in and we pick out the allocated PluginModuleContentParent somehow

IPDL gives us no help in linking the allocated PluginModuleContentParent to the LoadModule call that asked for it. There's no obvious way to fix this in IPDL that I can think of. If we want to allow multiple LoadModule calls on stack at once (as we have to) then we need a better way of linking this stuff. The one advantage we have is that LoadModule calls are received in the order sent, and the corresponding AllocPPluginModuleContentParent messages are also received in the order sent. So I set up a scheme that connects things together based on this ordering. Hopefully the comments are adequate to explain it.

Comment 1

[Benjamin Smedberg]

Comment on attachment 8523218 [details] [diff] [review]
plugin-load-order

I do think that this should be handled by IPDL (creating a bridge in response to a specific message should somehow be reflected in the signature somehow), but there's no reason to delay this patch for it.

Comment 2

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/982fd37469cf

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/982fd37469cf

Comment 4

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

Backed this out since it seems to be causing bug 1105896.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b55317fb623

I'll fix this in a better way in bug 1109883.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5b55317fb623