41276 - [crash] arbitrary limit to number of plugins allowed in a page

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[sean echevarria]

Mozilla has a hardcoded limit of 10 simultaneously embedded plugins.
If that limit is exceeded, plugin instances are prematurely destroyed (see the 
empty boxes in the attachment).
Leave or reload a page that has a prematurely destroyed plugin and mozilla will 
crash.

nsPluginHostImpl::AddInstanceToActiveList in
http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginHostImpl.c
pp#1751 is responsible for premature destruction of plugins that are not 
Stopped (plugins embedded in the currently viewed page).  A plugin should not 
be Stopped if it is still visible.

Similar problem if you have multiple browser windows open with fewer plugins in 
each since nsPluginHostImpl (and mActivePluginList) is app-global.

Comment 1

[sean echevarria]

Attachment: testcase - this will probably crash mozilla just by viewing it

Comment 2

[sean echevarria]

added crash, testcase and 4xp keywords

Comment 3

[sean echevarria]

MAX_ACTIVE_PLUGINS is defined as 10 at 
http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginHostImpl.h
#64

Was there a hardcoded MAX in 4x?

Comment 4

[sean echevarria]

changed wording of summary: artificial->arbitrary
nominating nsbeta2

Comment 5

[leger]

Putting on [nsbeta2+] radar for beta2 to fix the crash.  

After beta2, can we make it so that can handle up many plugins and work 
properly. Can we bump the number, and give a warning when app cannot handle more 
plug-ins?  10 is too small of a number.  Recommend rejecting newest plug in, not 
replacing installed current plug-ins as more are added.

Comment 6

[av (gone)]

Does anybody think that the array of ten should be replaced by a linked list? I 
don't think this is hard thing to do.

Comment 7

[sean echevarria]

Linked list will work, but need a way to reduce size of list to a reasonable 
amount once the user doesn't have X active plugins.  

That is, the number of active plugins should be unlimited, but the number of 
cached, stopped plugins should be limited.

Comment 8

[av (gone)]

Attachment: Using list of instances instead of array

Comment 9

[av (gone)]

Sean, would you review the list vs. array change. There is no limiting list size 
mechanism yet but I want to get this one first.

Comment 10

[av (gone)]

Fix checked it over the last three days. Thanks Sean for review and comments.

Comment 11

[shrirang khanzode]

marking verif.

Comment 12

[Jesse Ruderman]

Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/5a6def05ccbc