Closed Bug 1099410 Opened 5 years ago Closed 5 years ago

[e10s] Plugin loading is unexpectedly re-entrant


(Core :: Plug-ins, defect)

Not set



Tracking Status
e10s m5+ ---


(Reporter: billm, Assigned: billm)




(1 file)

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:
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.
Attachment #8523218 - Flags: review?(benjamin)
Comment on attachment 8523218 [details] [diff] [review]

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.
Attachment #8523218 - Flags: review?(benjamin) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Backed this out since it seems to be causing bug 1105896.

I'll fix this in a better way in bug 1109883.
Depends on: 1105896
Resolution: FIXED → WONTFIX
You need to log in before you can comment on or make changes to this bug.