Last Comment Bug 674535 - Lastpass causes immortal zombie compartments
: Lastpass causes immortal zombie compartments
Status: RESOLVED FIXED
[MemShrink:P3]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: ---
Assigned To: general
:
Mentors:
Depends on: hueyfix
Blocks: LeakyAddons ZombieCompartments
  Show dependency treegraph
 
Reported: 2011-07-27 08:02 PDT by Daniel Einspanjer [:dre] [:deinspanjer]
Modified: 2012-03-08 15:40 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Daniel Einspanjer [:dre] [:deinspanjer] 2011-07-27 08:02:18 PDT
+++ This bug was initially created as a clone of Bug #669730 +++

STR:

 * Install LastPass (I have 1.74.0).
 * Visit a site that has a sign-in and save the credentials (I used bugzilla.mozilla.org for this test case).
 * Restart Firefox.
 * Open about:memory, review compartments.
 * Visit the saved site again.
 * Let LastPass sign you in.
 * Close the site tab.
 * Refresh about:memory.
 * Click <minimize memory usage>.  Notice that the compartment for the site is still listed.


I don't know how popular LastPass is, but I am filing because I found the STR.  I found one bugzilla user with a lastpass.com domain so I am CC'ing him (Sorry if you are the wrong person to ping, Joe).  Will e-mail support@lastpass.com as well to loop them in.
Comment 1 Daniel Einspanjer [:dre] [:deinspanjer] 2011-07-27 08:03:10 PDT
Oops, forgot to change the summary when I cloned the bug. :/
Comment 2 Joe Siegrist 2011-07-27 08:15:19 PDT
Thanks Daniel, we'll take a look
Comment 3 Joe Siegrist 2011-08-05 14:24:40 PDT
Thank you for reporting this Daniel, it has been resolved in our prebuilds for Firefox: https://lastpass.com/lpre.php 

And should be released generally soon. 

This was somewhat painful to track down even with the help of this post:  http://blog.mozilla.com/sfink/2011/08/02/zombie-hunting/

Adding more info to the verbose version of about:memory would be a great improvement.
Comment 4 Nicholas Nethercote [:njn] 2011-08-05 14:38:03 PDT
Joe: great work, thanks!  Can you briefly explain why it was painful and how you found the leak?

> Adding more info to the verbose version of about:memory would be a great
> improvement.

What info would have helped you?
Comment 5 Nicholas Nethercote [:njn] 2011-08-05 14:40:00 PDT
(In reply to Joe Siegrist from comment #3)
> 
> And should be released generally soon. 

I guess we can close this bug once the new version is released.
Comment 6 David Rees 2011-11-05 16:39:11 PDT
Was it released? Because lastpass sure still seems to really bump Firefox's memory usage...

PS, The https://lastpass.com/lpre.php link is now bad, maybe because its released?
Comment 7 Joe Siegrist 2011-11-05 17:42:11 PDT
It's been released -- 1.80.0  -- https://lastpass.com/dl  (it was in 1.75.0 as well but we never got it through AMO's approval there). 

The amount of memory utilized by the extension appears to stable and proportional to the number of sites you have.
Comment 8 Nicholas Nethercote [:njn] 2011-11-07 19:38:05 PST
I don't think this has been fixed.  I tried the steps from comment 0 on v1.74.0 and v1.80.0 and both of them left behind the bugzilla.mozilla.org zombie compartment.
Comment 9 Joe Siegrist 2011-11-07 21:20:17 PST
With LastPass 1.75 or above it doesn't show the compartment 2 seconds after closing the tab for me unless I choose 'More Verbose'.

If I do that compartment ultimately goes away too, logging into any other site with LastPass makes it go away immediately in LP1.75+.
Comment 10 Nicholas Nethercote [:njn] 2011-11-07 22:04:31 PST
(In reply to Joe Siegrist from comment #9)
> With LastPass 1.75 or above it doesn't show the compartment 2 seconds after
> closing the tab for me unless I choose 'More Verbose'.

about:memory has to be in verbose mode to accurately diagnose zombie compartments, otherwise a zombie could be hidden in one of the aggregate "N omitted" nodes.

> If I do that compartment ultimately goes away too, logging into any other
> site with LastPass makes it go away immediately in LP1.75+.

I didn't log into any subsequent sites, which probably explains it.  So it sounds like LastPass now holds onto one zombie compartment, instead of N?  If so, that's an improvement, but not a complete fix.
Comment 11 Nicholas Nethercote [:njn] 2012-01-24 19:16:17 PST
Bug 720236 indicates that Lastpass also adds 20MB of memory consumption to Firefox's start-up, which seems excessive.
Comment 12 Joe Siegrist 2012-01-24 19:59:03 PST
I'm surprised at that number, have any additional tools been added to help track these issues down?  

Using nightly I show 6MB increase on start up to 37Mb at startup.  Still higher than anticipated though.

The one area clearly added when LastPass loads is the following, if you have tips for making progress here we'd appreciate them.

├─────292,651 B (00.79%) -- dom
│     ├──292,651 B (00.79%) -- window-objects
│     │  └──292,651 B (00.79%) -- active
│     │     ├──240,723 B (00.65%) -- top=1 (inner=2)
│     │     │  ├──237,701 B (00.64%) -- inner-window(id=2, uri=chrome://browser/content/browser.xul)
│     │     │  ├────1,511 B (00.00%) -- inner-window(id=10, uri=about:blank)
│     │     │  └────1,511 B (00.00%) -- inner-window(id=9, uri=about:blank)
│     │     ├───41,639 B (00.11%) -- top=12 (inner=14)
│     │     │   ├──39,544 B (00.11%) -- inner-window(id=14, uri=about:addons)
│     │     │   ├───1,695 B (00.00%) -- inner-window(id=18, uri=about:blank)
│     │     │   └─────400 B (00.00%) -- inner-window(id=17, uri=[system])
│     │     ├────5,731 B (00.02%) -- top=7 (inner=19)
│     │     │    └──5,731 B (00.02%) -- inner-window(id=19, uri=about:memory?verbose)
│     │     ├────2,800 B (00.01%) -- outer-windows [7]
│     │     └────1,758 B (00.00%) -- top=3 (inner=4)
│     │          └──1,758 B (00.00%) -- inner-window(id=4, uri=resource://gre-resources/hiddenWindow.html)
│     └────────0 B (00.00%) -- workers()
│              └──0 B (00.00%) -- worker(chrome://lastpass/content/lpctypesworker.js, 0xc06f8000c06f800)
│                 ├──0 B (00.00%) -- runtime
│                 │  ├──0 B (00.00%) -- runtime-object
│                 │  ├──0 B (00.00%) -- atoms-table
│                 │  ├──0 B (00.00%) -- contexts
│                 │  └──0 B (00.00%) -- threads
│                 │     ├──0 B (00.00%) -- normal
│                 │     ├──0 B (00.00%) -- temporary
│                 │     └──0 B (00.00%) -- stack-committed
│                 ├──0 B (00.00%) -- gc-heap-chunk-dirty-unused
│                 ├──0 B (00.00%) -- gc-heap-chunk-clean-unused
│                 ├──0 B (00.00%) -- gc-heap-decommitted
│                 └──0 B (00.00%) -- gc-heap-chunk-admin
Comment 13 Jorge Villalobos [:jorgev] 2012-02-22 13:57:50 PST
Joe, have you been able to address the zombie compartment problem?
Comment 14 Joe Siegrist 2012-02-22 14:14:34 PST
(In reply to Jorge Villalobos [:jorgev] from comment #13)
> Joe, have you been able to address the zombie compartment problem?

No, I haven't been able to find a way to see what's hanging on to the single compartment that's still being held in some cases.
Comment 15 Jorge Villalobos [:jorgev] 2012-02-23 10:15:34 PST
I can still reproduce the leak with Bugzilla.

On an unrelated note, the Firefox logos on this page: https://lastpass.com/misc_download.php doesn't follow our identity policies (http://www.mozilla.org/en-US/firefox/brand/identity/). I recommend you replace them soon.
Comment 16 Joe Siegrist 2012-03-08 12:04:52 PST
Jorge:

Version 1.90.4 fixes this immortal zombie.  This has been released on our side and is awaiting review on the AMO side. 

Logos on https://lastpass.com/misc_download.php  have been updated, thanks for the head's up.

Joe
Comment 17 Jorge Villalobos [:jorgev] 2012-03-08 12:08:32 PST
Andrew, please have a look when you have the time. Thanks.
Comment 18 Jorge Villalobos [:jorgev] 2012-03-08 15:40:46 PST
Nvm, I reviewed it due to security urgency. The memory leak has been fixed in this version.

Note You need to log in before you can comment on or make changes to this bug.