Last Comment Bug 740085 - LoL add-on 1.5 creates zombie compartments
: LoL add-on 1.5 creates zombie compartments
Status: RESOLVED WONTFIX
:
Product: Tech Evangelism
Classification: Other
Component: Add-ons (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
https://addons.mozilla.org/en-US/fire...
Depends on:
Blocks: LeakyAddons ZombieCompartments
  Show dependency treegraph
 
Reported: 2012-03-28 11:57 PDT by Chris Pickett
Modified: 2013-03-20 14:28 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Chris Pickett 2012-03-28 11:57:31 PDT
Steps to reproduce:

1. Install LoL add-on 1.5: https://addons.mozilla.org/en-US/firefox/addon/lol/
2. Disable all other add-ons
3. Restart
4. Open https://www.facebook.com/home.php
5. Close facebook tab / window
6. Open about:memory
7. Click GC / CC / Minimize memory usage

Result: zombie facebook compartment
Expected: no zombies!

Snapshot of heap is below.  This happens with many other sites.  It is not dependent on hitting space to activate the user-visible part of this addon.  The developer's homepage [1] is dead but the root domain is still alive so I added his email to the CC list.

This blocks bug 668871 and bug 700547.  I don't see how to edit the blocks / depends fields when filing this report in bugzilla so can somebody else annotate it for me?

[1] http://elder-gods.org/lol

83.04 MB (100.0%) -- explicit
├──40.08 MB (48.26%) -- js
│  ├──12.60 MB (15.17%) -- compartment([System Principal], 0x1152e4000)
│  │  ├───7.93 MB (09.55%) -- gc-heap
│  │  │   ├──2.49 MB (03.00%) -- objects
│  │  │   │  ├──1.88 MB (02.26%) -- function
│  │  │   │  └──0.61 MB (00.74%) -- non-function
│  │  │   ├──2.34 MB (02.82%) -- shapes
│  │  │   │  ├──1.37 MB (01.65%) -- tree
│  │  │   │  ├──0.58 MB (00.70%) -- base
│  │  │   │  └──0.38 MB (00.46%) -- (1 omitted)
│  │  │   ├──1.61 MB (01.94%) -- arena
│  │  │   │  ├──1.46 MB (01.76%) -- unused
│  │  │   │  └──0.15 MB (00.18%) -- (2 omitted)
│  │  │   ├──1.33 MB (01.60%) -- scripts
│  │  │   └──0.17 MB (00.20%) -- (3 omitted)
│  │  ├───1.92 MB (02.31%) -- script-data
│  │  ├───1.45 MB (01.75%) -- shapes-extra
│  │  │   ├──0.61 MB (00.73%) -- tree-tables
│  │  │   ├──0.50 MB (00.60%) -- compartment-tables
│  │  │   └──0.35 MB (00.42%) -- (2 omitted)
│  │  ├───0.76 MB (00.91%) -- (5 omitted)
│  │  └───0.54 MB (00.65%) -- object-slots
│  ├───9.88 MB (11.90%) -- compartment(https://www.facebook.com/home.php)
│  │   ├──5.81 MB (07.00%) -- gc-heap
│  │   │  ├──2.68 MB (03.23%) -- arena
│  │   │  │  ├──2.57 MB (03.10%) -- unused
│  │   │  │  └──0.11 MB (00.13%) -- (2 omitted)
│  │   │  ├──1.20 MB (01.45%) -- objects
│  │   │  │  ├──0.71 MB (00.85%) -- function
│  │   │  │  └──0.49 MB (00.59%) -- non-function
│  │   │  ├──0.91 MB (01.09%) -- scripts
│  │   │  ├──0.76 MB (00.92%) -- shapes
│  │   │  │  ├──0.45 MB (00.54%) -- tree
│  │   │  │  └──0.32 MB (00.38%) -- (2 omitted)
│  │   │  └──0.26 MB (00.31%) -- (2 omitted)
│  │   ├──1.37 MB (01.64%) -- type-inference
│  │   │  ├──1.02 MB (01.22%) -- script-main
│  │   │  └──0.35 MB (00.42%) -- (2 omitted)
│  │   ├──1.08 MB (01.30%) -- script-data
│  │   ├──0.89 MB (01.07%) -- (4 omitted)
│  │   └──0.73 MB (00.88%) -- shapes-extra
│  │      └──0.73 MB (00.88%) -- (4 omitted)
│  ├───9.84 MB (11.85%) -- runtime
│  │   ├──8.52 MB (10.25%) -- threads
│  │   │  ├──8.00 MB (09.63%) -- stack-committed
│  │   │  └──0.52 MB (00.62%) -- (3 omitted)
│  │   ├──1.00 MB (01.20%) -- atoms-table
│  │   └──0.33 MB (00.40%) -- (2 omitted)
│  ├───4.10 MB (04.93%) -- gc-heap-decommitted
│  ├───2.15 MB (02.59%) -- compartment(atoms)
│  │   ├──1.71 MB (02.06%) -- gc-heap
│  │   │  ├──1.62 MB (01.95%) -- strings
│  │   │  └──0.09 MB (00.11%) -- (1 omitted)
│  │   ├──0.45 MB (00.54%) -- string-chars
│  │   └──0.00 MB (00.00%) -- (1 omitted)
│  ├───1.02 MB (01.23%) -- xpconnect
│  └───0.48 MB (00.58%) -- (5 omitted)
├──29.23 MB (35.20%) -- heap-unclassified
├───6.76 MB (08.15%) -- storage
│   ├──5.57 MB (06.71%) -- sqlite
│   │  ├──2.23 MB (02.69%) -- other
│   │  ├──2.05 MB (02.46%) -- places.sqlite
│   │  │  ├──1.78 MB (02.15%) -- cache-used [3]
│   │  │  └──0.26 MB (00.32%) -- (2 omitted)
│   │  ├──0.82 MB (00.98%) -- (9 omitted)
│   │  └──0.48 MB (00.58%) -- extensions.sqlite
│   │     └──0.48 MB (00.58%) -- (3 omitted)
│   └──1.19 MB (01.43%) -- prefixset
│      └──1.19 MB (01.43%) -- all
├───2.13 MB (02.56%) -- startup-cache
│   ├──2.13 MB (02.56%) -- data
│   └──0.00 MB (00.00%) -- (1 omitted)
├───2.13 MB (02.56%) -- layout
│   ├──0.85 MB (01.02%) -- shell(chrome://browser/content/browser.xul)
│   │  ├──0.50 MB (00.60%) -- arenas
│   │  └──0.35 MB (00.42%) -- (1 omitted)
│   ├──0.74 MB (00.89%) -- (4 omitted)
│   └──0.54 MB (00.65%) -- shell(about:addons)
│      └──0.54 MB (00.65%) -- (2 omitted)
├───1.18 MB (01.42%) -- xpti-working-set
├───1.02 MB (01.22%) -- images
│   ├──0.84 MB (01.01%) -- chrome
│   │  ├──0.84 MB (01.01%) -- used
│   │  │  ├──0.84 MB (01.01%) -- uncompressed-heap
│   │  │  └──0.00 MB (00.00%) -- (2 omitted)
│   │  └──0.00 MB (00.00%) -- (1 omitted)
│   └──0.18 MB (00.21%) -- (1 omitted)
├───0.42 MB (00.51%) -- dom
│   └──0.42 MB (00.51%) -- window-objects
│      ├──0.42 MB (00.51%) -- active
│      │  └──0.42 MB (00.51%) -- (5 omitted)
│      └──0.00 MB (00.00%) -- (1 omitted)
└───0.09 MB (00.11%) -- (5 omitted)
Comment 1 Jorge Villalobos [:jorgev] 2012-04-04 15:49:14 PDT
A message has been sent to the developer.
Comment 2 Andrew Williamson [:eviljeff] 2012-04-12 08:01:32 PDT
smoofra/Larry: are you making any progress in fixing the memory leak in lol?
Comment 3 Jorge Villalobos [:jorgev] 2012-04-26 15:24:13 PDT
An ultimatum message has been sent to the developer. The add-on should be downgraded if no progress is shown by next week.
Comment 4 Jorge Villalobos [:jorgev] 2012-05-02 16:57:21 PDT
The add-on has been downgraded. Given that there has been no new version releases in over a year, I don't expect anything else to happen here.
Comment 5 Chris Lawson (gone) 2012-05-02 17:00:37 PDT
Wouldn't "WONTFIX" be a more appropriate resolution for these bugs, since the developer is, well, refusing (either through inaction or deliberate choice) to fix the problem?
Comment 6 Jorge Villalobos [:jorgev] 2012-05-02 17:03:05 PDT
That was brought up in another leak bug and I'm indifferent about it. Whatever we choose, we should be consistent about it, so we would need to change the others with similar resolutions.

Andrew, what do you think?
Comment 7 Nicholas Nethercote [:njn] 2012-05-02 20:56:31 PDT
WONTFIX seems right to me.

I just tried this add-on, AFAICT it leaks every content compartment ever opened.  I.e. it's as bad as the original SiteAdvisor leak (bug 727938).
Comment 8 Andrew Williamson [:eviljeff] 2012-05-03 03:19:47 PDT
(In reply to Jorge Villalobos [:jorgev] from comment #6)
> That was brought up in another leak bug and I'm indifferent about it.
> Whatever we choose, we should be consistent about it, so we would need to
> change the others with similar resolutions.
> 
> Andrew, what do you think?

Yeah I'd agree with WONTFIX for addons where the developer hasn't fixed the leak.
Comment 9 Jorge Villalobos [:jorgev] 2012-05-03 09:25:36 PDT
(In reply to Nicholas Nethercote [:njn] from comment #7)
> I just tried this add-on, AFAICT it leaks every content compartment ever
> opened.  I.e. it's as bad as the original SiteAdvisor leak (bug 727938).

Given this new information, I disabled all existing versions.
Comment 10 Chris Pickett 2012-05-08 18:51:07 PDT
(In reply to Jorge Villalobos [:jorgev] from comment #9)
> (In reply to Nicholas Nethercote [:njn] from comment #7)
> > I just tried this add-on, AFAICT it leaks every content compartment ever
> > opened.  I.e. it's as bad as the original SiteAdvisor leak (bug 727938).
> 
> Given this new information, I disabled all existing versions.

Since the patch for Bug 695480 reportedly fixes this, can you undo this change?
Comment 11 Nicholas Nethercote [:njn] 2012-05-08 22:20:59 PDT
Bug 695480 won't be in a Firefox release until Firefox 15 (August 28) at the earliest.  AMO testing is done with release versions, as is sensible.   Once bug 695480 makes it into a release, there's an argument to be made for re-enabling this add-on, but given the lack of response from the author I don't think it would be a strong case.
Comment 12 Jorge Villalobos [:jorgev] 2012-05-09 09:22:12 PDT
(In reply to Nicholas Nethercote [:njn] from comment #11)
> Bug 695480 won't be in a Firefox release until Firefox 15 (August 28) at the
> earliest.  AMO testing is done with release versions, as is sensible.   Once
> bug 695480 makes it into a release, there's an argument to be made for
> re-enabling this add-on, but given the lack of response from the author I
> don't think it would be a strong case.

I agree with this.
Comment 13 Chris Pickett 2013-01-31 15:08:20 PST
Nicholas confirmed that LoL works with the patch here:

https://bugzilla.mozilla.org/show_bug.cgi?id=695480#c50

The LoL extension I have on my computer works great with the latest releases (presumably since Firefox 15 or whenever the patch landed, but I didn't have it enabled then).  I think it's still useful for keyboard accessibility, as there are few mouseless browsing extensions.

I got in touch with the developer.  The LoL website and repository are back online.  I was wondering what it would take to re-enable LoL on AMO.  Has it been deleted from the database along with all comments?  Is a simple version bump enough to satisfy you guys?  He says he would be happy to hand the extension off to a new maintainer, but at the same time, it seems like a fairly stable thing, aside from the (fixed) compartment leaking that is.

Or, is it official policy that leaky addons are not permitted on AMO now?
Comment 14 Jorge Villalobos [:jorgev] 2013-02-04 07:49:12 PST
The listing still exists, but all past versions are disabled. All the developer needs to do is submit a new version and nominate it for review.
Comment 15 Larry D'Anna 2013-03-19 16:41:39 PDT
Alright, I'm paying attention to this now :/ 

Got an email from a user asking why the addon was removed from mozilla.org

Figured that it was another instance of version numbers in the .xpi needing to be updated for no reason.

Re-submitted the addon, Leszek Życzkowski rejects it with "You not resolved problems with memory leaks mentioned by previous reviewer"

Previous review doesn't seem to be visible anywhere on addons.mozilla.org

Searched my email, found this bug.


As far as I can tell this isn't reproducible on 19.0.2.   From comments above I surmise this is because of a fix introduced in Firefox 15.


Well, what do I do now?
Comment 16 Jorge Villalobos [:jorgev] 2013-03-20 14:28:41 PDT
I added a comment for other reviewers to make sure that they test for the leaks and don't reject unless they are still present. Please upload again with a new version number.

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