Closed Bug 633427 Opened 14 years ago Closed 13 years ago

Clearing cookies launches instance of plugin-container for each plugin installed

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(firefox10 affected, firefox11 affected, firefox12 verified, firefox13 verified, firefox-esr1012+ verified, blocking2.0 -)

RESOLVED FIXED
mozilla13
Tracking Status
firefox10 --- affected
firefox11 --- affected
firefox12 --- verified
firefox13 --- verified
firefox-esr10 12+ verified
blocking2.0 --- -

People

(Reporter: KWierso, Assigned: jaas)

References

Details

(Keywords: regression, Whiteboard: [MemShrink:P2] [Snappy:P1][qa+])

Attachments

(4 files, 1 obsolete file)

In recent nightlies (maybe starting last night, didn't pay attention until today), clearing Cookies will start an instance of plugin-container for almost all of the plugins installed on the system, regardless of enable/disable state for each one.

I say "almost all", because I currently have 13 plugins, but clearing the cookies only starts up 11 plugin-containers.

These plugin-container instances stay open until Firefox is shut down.

Setting Firefox to erase cookies on shutdown will have the plugin-containers started and closed at shutdown.
Attached image installed plugins —
Screenshot of installed plugins and their versions.
Attached image process explorer capture —
And what Process Explorer says about each plugin-container.
So, probably from one or more of the following:
Bug 625496 - Clear Adobe Flash Cookies (LSOs) when Cookies is selected in Clear Recent History
Bug 625497 - Clear Adobe Flash Cookies (LSOs) when "Forget This Site" is selected
Bug 508167 - NPAPI additions for clearing recent history (e.g. for "flash cookies")

Expected results:
Either don't fire up each plugin to clear Flash cookies, or close the plugins after everything is cleared. (Or, if this is all being done just for Flash cookies, only start up plugin-container for Flash plugins?)

Actual results:
Firefox seems to iterate through all installed plugins, firing off each one in order to clear the Flash cookies, with each plugin remaining open for the rest of the session.
blocking2.0: --- → ?
Keywords: regression
Blocks: 633433
Josh, if I'm reading the docs correctly, the right fix here would be to check the disabled flag on the plugin tag before passing it to clearSiteData (or maybe doing the check inside clearSiteData).

Does that sounds sane to you?
Clearing private data requires loading the plugin. For the in-process case the plugins will probably have to remain loaded. For the OOPP case we can probably keep track of what was loaded just for the sake of clearing and shut it down when clearing is done.  We shouldn't be loading disabled plugins at all.
Assignee: nobody → joshmoz
blocking2.0: ? → final+
Summary: Clearing cookies launches instance of plugin-container for each plugin installed → Clearing cookies loads disabled plugins
Whiteboard: [hardblocker]
I'm not sure that we shouldn't load disabled plugins.  If I used Java for a site, then disabled it due to a security issue and cleared history, I would be very surprised if Java state was still around once I got a security update and re-enabled it.

That said, I don't think "we don't clear cookies for disabled plugins" would be a hardblocker, so let's get there.
Yeah, after I wrote that I started changing my mind and I think I agree now.
Returning this bug's summary to the reporter's summary, the complaint is about the fact that clearing cookies creates too many processes that stick around.
Summary: Clearing cookies loads disabled plugins → Clearing cookies launches instance of plugin-container for each plugin installed
In that case, this is totally not a blocker.
blocking2.0: final+ → -
Whiteboard: [hardblocker]
Do all plugins get private data cleared when this happens, or just Flash/Java? 

If it's just Flash/Java, why does Firefox have to launch (almost) all plugins to do this?
Any plugin that implements the interface will do it.  We don't know which those are without loading them; after 4 we can do something to cache it or otherwise improve things.
With the Flash plugin-container running while the plugin is disabled, I can't get YouTube to play a Flash video while I have it disabled, so it still respects the plugin's setting in not doing anything beyond clearing the private data.
not only plugin-container.exe, but also AcroRd32.exe (Adobe Reader).
related issue ?
Same issue here. This problem occurs even in safe mode.
Windows 7x64
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110214 Firefox/4.

Plugins installed:
Adobe PDF Plug-In For Firefox and Netscape 10.0.1 10.0.1.434
Silverlight Plug-In 4.0.60129.0
Java Deployment Toolkit 6.0.230.5 1.6.0.23
Java(TM) Platform SE 6 U23 1.6.0.23 1.6.0.23
Shockwave Flash 10.2 r152
Microsoft® Windows Media Player Firefox Plugin 1.0.0.8
Google Earth Plugin GEPlugin
I assume you know this but also occurs on OSX.
I'm having the same problem, Windows 7 Pro (32bit), with Firefox 4.
The same problem on Windows XP Home, with firefox 4.
This bug has been around from 3.6 I hoped that you have already fixed it.

The thing that's ironic about this bug is that clearing history should make firefox run smoother - not the other way around.
It is not only the problem of a amount of "plugincontainer"-processes slowing down or freezing system for some seconds. Clearing cookies script sometimes also crashes! If this happens I normally cancle the request to restart launch. But afterwards I try again "Clear cookies", then it works fine and only takes a second. I think it is the amount of data to be cleared, whats causing major problems. So you (developers) should change the way, this clearing is done. In my opinion the algorithm used now is not the best, it seems Firefox wants to be fast and launches too much threads.
This is a showstopper for me. I distribute a Firefox+Tor package (not the "official" one by the Tor project), and of course it comes with settings twisted for anonymity. Which is why the history is cleared after each session (with the AskForSanitize add-on used to signal this to the user, but that's not relevant here). With this bug, things like an Adobe Reader process can be launched when history is cleared. With the increasing number of drive-by infections that use PDF files, you can imagine what security-minded users will think if a Reader process is started when they didn't do anything to explicitly trigger it. I'm sure many users of "vanilla" Firefox will have the same doubts.
This is a pretty annoying problem. I don't understand the discussion about not loading disabled plug-ins during the clearing process - okay I do understand it, but it misses the point. The point seems to be either to only load plugins that have cookies to clear (e.g. Flash) or conversely to specifically avoid loading some common plug-ins which do not have plug-in cookies to clear (note: The QuickTime plugin is broken down into 6 separate plugins - Thanks Apple!)

A ~40 MB jump in memory use from loading plugins that aren't normally needed is a big penalty for clearing cookies.
Whiteboard: [MemShrink:P2]
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
Sounds like we should record in a file whether each plug-in implements the "clear cookie" interface.  And then consult that file before loading each plug-in when cookies are cleared.  (I.e. what comment 11 said.)

Josh, you are assigned to this bug, does that sound like the right approach?  Any ideas on when you might be able to get to this?
Seems to be a lot better in FF 5.0 Have you changed something regarding this problem?
This appears to continue in Firefox 9 (on Windows XP). Using Mozilla/5.0 (Windows NT 5.1; rv:9.0a1) Gecko/20110819 Firefox/9.0a1 ID:20110819030749 I note if I disable plugins and then clear cookies whilst process explorer is running I do see the disabled plugins  become listed in process explorer.
I'm pretty ignorant on all this stuff, but my experience with this issue is through multiple versions, but only with 5.0 forward did I experience up to 16 plugin container modules each of them using my resources when clearing my personal data (cookies etc) So the issue is getting worse since I reported it months ago. And I'd like to note, that I have no extra plugins or anything installed, it is like it comes to me in the download. Also, the amount of plugin containers that open is varied, from 8-16, so with me not having any plugins installed, why would one be opening for "each plugin Installed" as suggested in this conversation? it makes no sense. Also, when is the discussion going to stop and the fixing take place? I reported this issue six months ago and have been living with it ever since through 3 more FF instals, each time hoping it would be fixed.
but instead of it being fixed in 5.01, I get more problems with saving passwords. If I have more than 2 passwords on a site, I have to repeatedly enter and save them many times for all pf them to actually be saved and start appearing in the dropdown list regularly. So my question is if in 5.01, is FF now in a contest with IE to be the shittiest browser with the least reliable password manager?
rubrtoe:  please read https://bugzilla.mozilla.org/page.cgi?id=etiquette.html, especially the items "No obligation" and "No abusing people".
What a hoot, RE:"Open Source" is not the same as "the developers must do my bidding." Everyone here wants to help, but the only person who has any obligation to fix the bugs you want fixed is you. Therefore, you should not act as if you expect someone to fix a bug by a particular date or release. Aggressive or repeated demands will not be received well and will almost certainly diminish the impact and interest in your suggestions." it is suggested that I review this. So I did.
Here is what I think: You software types live in a fantasy world of your own making that is completely disconnected with all reality, especially any form of work ethic and taking responsibility for your mistakes. #1 this passage trying to foist on the world the preposterous idea that  is my software and someone is working for me on it. Like I said, FANASYLAND. But last time I checked, Mozilla employs people whose job depends on the success of the software they make, That makes this product their responsibility not mine, that is if they like to keep their jobs and feed their families. My family and I will do just find if Firefox died today. This makes it Mozilla's responsibility not mine because I could give a rats's ass about FF. it just **** me off as does software hacks in general. But if you morons don't start building **** right that works, you'll be out of a job and 3 payments behind on your crystal fantasy palaces.Living in your mortgages fantasy palaces. Therefore MOZILLA IS THE ONLY ENTITY Which SHOULD HAVE THE COMMON SENSE TO KNOW THAT NOT ONLY ARE THE BUGS IT'S ENGINEERS REPEATEDLY INTRODUCE INTO THIS SOFTWARE EVERY TIME THEY FIX OR INTRODUCE SOMETHING THAT IT IS IN MOZILLA"S ECONOMIC INTEREST TO START TAKING RESPONSIBILITY for  it's work and quit living under the fantasy that it's mine. It's COMMON SENSE: follow the buck. who get's paid for building this software? Not me. I don't feed my family by writing bad software full of modules that don't work half the time. Mozilla's paid engineers are solely responsible and it is in their best interests to fix these issues with the software. The food on your table is put there by your job, so start doing it and quit trying to put it off on me.
A few notes:

1) Comment 28 didn't abuse anyone, just Firefox (albeit unhelpfully). I suggest that pointing people straight at the etiquette document at the first sign of anger may be in some cases counter-productive. A more mild rebuke might get better results. "A gentle answer turns away wrath, but a harsh word stirs up anger." (Proverbs 15:1)

2) Comment 30 is well out of line. Profanivore FTW.

3) Account disabled.

Gerv
One thing that is NOT mentioned in Comment 30 is that almost everyone who works on Firefox and Mozilla products in general are people who don't get paid for this and do this because they want to help out with this awesome software.

And the people who do work for Mozilla(I think) are working on big things like security and really big features like new UI, HTML 5, CSS 3, probably...

BTW: Anyone notice that he got frustrated for this bug not being fixed in one day?
I think he forgot that Firefox(Open Source) wasn't built in a day.

----BACK TO REPORTED BUG----
That has nothing to do with whether the programmers are paid or not, that is simply bad style to add misprogrammed new feature and let people going crazy with them for month. Why didn't simply remove this feature until it works how it should?
Josh: what do you think about comment 23?
(In reply to Ehsan Akhgari [:ehsan] from comment #35)
> Josh: what do you think about comment 23?

At a glance that seems like a reasonable strategy but I'm about three weeks away from being able to put serious thought into this.
(In reply to Josh Aas (Mozilla Corporation) from comment #36)
> (In reply to Ehsan Akhgari [:ehsan] from comment #35)
> > Josh: what do you think about comment 23?
> 
> At a glance that seems like a reasonable strategy but I'm about three weeks
> away from being able to put serious thought into this.

Can you recommend another person who knows the code in question for this bug?  If not, I guess we'll just wait.  :-)
I don't know if they're able to work on this, but people who probably have relatively strong knowledge of the code in question include jst, bsmedberg, jmathies.
I'm guessing we create the plugin instance in the same way we would if content were creating it? Seems like we should be able to flag the instance in this special case so that when this instance is destroyed if it's the only one running it takes the modules/process down with it.

Josh, any pointers on where this clear cookie plugin logic is based in the code?
(In reply to Jim Mathies [:jimm] from comment #39)
> I'm guessing we create the plugin instance in the same way we would if
> content were creating it? Seems like we should be able to flag the instance
> in this special case so that when this instance is destroyed if it's the
> only one running it takes the modules/process down with it.

I'm not sure what you mean by this.

> Josh, any pointers on where this clear cookie plugin logic is based in the
> code?

nsPluginHost::ClearSiteData is probably a good place to start.
Maybe bug 680609 is related.
I can reproduce the bug on firefox-9.0.1 (ubuntu 10.04, x86).
It does not trigger every time I quit firefox.  When it does, I can see one remnant plugin-container processes.  The plugins that are launched are never the same ones and can be some plugins that I have explicitly disabled in my firefox' configuration.  Unless I kill these processes, firefox is still alive even though its window is closed.

I was able to locate the problem in the code.  The issue stems from the mIsFlashPlugin flag (in nsPluginTag objects) not being explicitly reset in some cases.  This flag can be initialized to some garbage value different from 0 (it is quite likely to happen and indeed I experimentally observed it in a debugger); hence the !tag->mIsFlashPlugin condition in nsPluginHost::ClearSiteData() could be incorrectly evaluated to FALSE instead of TRUE, so that the corresponding plugin is incorrectly launched.  I propose the following patch:


--- mozilla.orig/dom/plugins/base/nsPluginTags.cpp      2011-12-21 09:54:06.000000000 +0100
+++ mozilla/dom/plugins/base/nsPluginTags.cpp   2012-01-30 15:06:30.000000000 +0100
@@ -199,6 +199,7 @@ mLibrary(nsnull),
 mCanUnloadLibrary(aCanUnload),
 mIsJavaPlugin(PR_FALSE),
 mIsNPRuntimeEnabledJavaPlugin(PR_FALSE),
+mIsFlashPlugin(PR_FALSE),
 mFileName(aFileName),
 mFullPath(aFullPath),
 mVersion(aVersion),


I have not been able to reproduce the bug when using this patch.
It should be possible to apply it against all recent firefox versions, yet I have only tested it on 9.0.1.

Note for the testers: the MOZ_DEBUG_CHILD_PROCESS environment variable can be useful to detect when firefox launches plugins (on unix systems with this variable set to 1, plugin-container starts by displaying its process id and then sleeps for 30 seconds before resuming execution).
I think bug 680609 is indeed a dup of this one.
Thank you, Khanh-Dang! Would you like to attach a patch to this bug so it can be properly reviewed?
Attached patch Fix detection of flash plugins (obsolete) — — Splinter Review
The attached patch (to be applied against 9.0.1) fixes the issue of incorrect detection of flash plugins, leading to unexpected behaviors.  In this patch, the code of the nsPluginTag::nsPluginTag() constructors are factorized and the mIsFlashPlugin flag is explicitly set to FALSE as the default value.

I tested the patch with "MOZ_DEBUG_CHILD_PROCESS=1 firefox" and observed that the flash plugin was launched as expected on firefox exit when user pref privacy.clearOnShutdown.cookies was set to true, and not launched, again as expected, when this user pref was set to false.  In both case, no other plugin was launched on exit, as expected.
Comment on attachment 592948 [details] [diff] [review]
Fix detection of flash plugins

Requesting review on behalf of Khanh-Dang.

Thanks for the patch! :-)
Attachment #592948 - Flags: review?(joshmoz)
Comment on attachment 592948 [details] [diff] [review]
Fix detection of flash plugins

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

Sorry for the delay but we've been busy with a number of other patches to our plugin code.

This patch needs to be updated due to a number of changes to this code in the past few weeks. You might want to wait for the fix in bug 501485 to land as well, because that will conflict with this.

Thanks for the patch, Khanh-Dang! I'll review as soon as the fix for bug 501485 lands (today?) and you've updated the patch.
Attachment #592948 - Flags: review?(joshmoz)
Assignee: joshmoz → kdntl
Bug 501485 has landed on mozilla-central so we're ready for this. Khanh-Dang - do you want to update your patch or should I?
I am quite busy these days.  I would be pleased if you could update the patch.  Thanks!
Assignee: kdntl → joshmoz
Attached patch fix v1.1 — — Splinter Review
Attachment #592948 - Attachment is obsolete: true
Attachment #598555 - Flags: review?(smichaud)
Attached patch fix v1.1 for Aurora — — Splinter Review
Attachment #599185 - Flags: review?(smichaud)
Comment on attachment 599185 [details] [diff] [review]
fix v1.1 for Aurora

This looks fine to me.

But why not initialize mIsFlashPlugin in all the nsPluginTag constructors?  We already do this for mIsJavaPlugin, and it might save us grief somewhere down the line.
Attachment #599185 - Flags: review?(smichaud) → review+
Attachment #598555 - Flags: review?(smichaud) → review+
> But why not initialize mIsFlashPlugin in all the nsPluginTag
> constructors?  We already do this for mIsJavaPlugin, and it might
> save us grief somewhere down the line.

Oops, I goofed.  With Josh's patch we're already doing this.
Comment on attachment 599185 [details] [diff] [review]
fix v1.1 for Aurora

[Approval Request Comment]
Regression caused by (bug #): Not sure, a long time ago
User impact if declined: Potential for massive shutdown delay.
Testing completed (on m-c, etc.): Will be on m-c today.
Risk to taking this patch (and alternatives if risky): 3/10
String changes made by this patch: None
Attachment #599185 - Flags: approval-mozilla-aurora?
Comment on attachment 599185 [details] [diff] [review]
fix v1.1 for Aurora

[Triage Comment]
We haven't heard that slow shutdown is a source of major user pain, so let's let this ride the trains.
Attachment #599185 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
> [Triage Comment]
> We haven't heard that slow shutdown is a source of major user pain, so let's
> let this ride the trains.

http://www.reddit.com/r/AdviceAnimals/comments/phx7f/scumbag_firefox/
(In reply to Nicholas Nethercote [:njn] from comment #58)
> > [Triage Comment]
> > We haven't heard that slow shutdown is a source of major user pain, so let's
> > let this ride the trains.
> 
> http://www.reddit.com/r/AdviceAnimals/comments/phx7f/scumbag_firefox/

I have to agree. I've even had people complain to me in person, and when it's this particular bug it can be very painful. Most people won't know how to diagnose it.
(In reply to Josh Aas (Mozilla Corporation) from comment #59)
> (In reply to Nicholas Nethercote [:njn] from comment #58)
> > 
> > http://www.reddit.com/r/AdviceAnimals/comments/phx7f/scumbag_firefox/
> 
> I have to agree. I've even had people complain to me in person, and when
> it's this particular bug it can be very painful. Most people won't know how
> to diagnose it.

This hadn't come up in our weekly SUMO sync-ups for sources of user pain, but that may have just been because people no longer report this issue (or know how to). Given this, I'm willing to take the extra risk and land this a cycle early in Firefox 12.

I'm also adding [Snappy] to the whiteboard to make sure this is a known pain point for Firefox performance investigations.
Whiteboard: [MemShrink:P2] → [MemShrink:P2] [Snappy]
Attachment #599185 - Flags: approval-mozilla-aurora- → approval-mozilla-aurora+
Whiteboard: [MemShrink:P2] [Snappy] → [MemShrink:P2] [Snappy:P1]
Thank you for fixing this and sorry for interrupting without knowing your development process. But I'd be interested to know if the patch will appear in Firefox 10 (ESR perhaps) - any chance for that?
I'd be happy to forward port this to the FF10 ESR if we can get approval for tracking.
> status-firefox13: 	fixed 

when ?
seems to be no "pushed to mozill-central"

(In reply to Josh Aas (Mozilla Corporation) from comment #55)
> pushed to mozilla-inbound
> http://hg.mozilla.org/integration/mozilla-inbound/rev/c4f9959cc9ac

(In reply to Josh Aas (Mozilla Corporation) from comment #61)
> pushed to mozilla-aurora
> http://hg.mozilla.org/releases/mozilla-aurora/rev/0bcfd4989ce9
(In reply to pal-moz from comment #64)
> > status-firefox13: 	fixed 
> 
> when ?
> seems to be no "pushed to mozill-central"

My bad, I jumped the gun there.
https://hg.mozilla.org/mozilla-central/rev/c4f9959cc9ac

\o/
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Is it OK to automatically clear cookies and/or other private data when disabling a plugin?  If this can happen, when users choose to clear coookies, FF does not need to launch instances of disabled plugin-container.
Tracking for the ESR that is released alongside FF12. Please hold on landing this change until mozilla-beta has version 12. https://wiki.mozilla.org/Release_Management/ESR_Landing_Process
wow..\0/ thanks for fixing this.. I hit this profile in use when I relaunch about a thousand times because the nightly is not shutdown yet.
still happens with most recent version 11.0 on Windows XP with latest patches applied
(In reply to Christoph-Erdmann Pfeiler from comment #70)
> still happens with most recent version 11.0 on Windows XP with latest
> patches applied

That is expected as this Fix landed for Firefox 12 what is in Beta Stage right now: http://www.mozilla.org/en-US/firefox/beta/
Comment on attachment 599185 [details] [diff] [review]
fix v1.1 for Aurora

[Triage Comment]
We're ready to start landing things on the ESR branch for the next release.  Please go ahead and land, see https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for details.
Attachment #599185 - Flags: approval-mozilla-esr10+
Whiteboard: [MemShrink:P2] [Snappy:P1] → [MemShrink:P2] [Snappy:P1][qa+]
The problem persists with Firefox 11.0
(In reply to Christoph-Erdmann Pfeiler from comment #74)
> The problem persists with Firefox 11.0

Please see comments 70 & 71
Mozilla/5.0 (Windows NT 6.1; rv:10.0.4esrpre) Gecko/20120416 Firefox/10.0.4esrpre
 
On Firefox 10.0.4 ESR I can still see one plugin-container process that is spawn after clearing all the cookies.

"C:\Program Files\Nightly\10.0.4ESR\plugin-container.exe" --channel=3484.8bd1200.1914406901 "C:\Windows\system32\Macromed\Flash\NPSWF32_11_2_202_233.dll" 1CCA150C7371694B -greomni "C:\Program Files\Nightly\10.0.4ESR\omni.ja" 3484 "\\.\pipe\gecko-crash-server-pipe.3484" plugin

Is this expected?
Yes. Flash supports clearing its cookies, which is why this feature exists in the first place. Most other plugins either don't have cookies or haven't implemented support for clearing them, so we should only be launching a plugin-container process for those plugins which have support.
Given comment 76 and 77, calling the verified for esr10.
Verified using Firefox 12 beta 6 that clearing cookies does not launch instances of plugin-container for each plugin installed.

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0 
Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0

Verified using Firefox 13 beta 2 that clearing cookies does not launch instances of plugin-container for each plugin installed.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: