Closed Bug 740085 Opened 12 years ago Closed 12 years ago

LoL add-on 1.5 creates zombie compartments

Categories

(WebExtensions :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: chris.pickett, Unassigned)

References

()

Details

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)
Assignee: general → nobody
Component: JavaScript Engine → Add-ons
Product: Core → Tech Evangelism
QA Contact: general → addons
Version: Trunk → unspecified
A message has been sent to the developer.
smoofra/Larry: are you making any progress in fixing the memory leak in lol?
An ultimatum message has been sent to the developer. The add-on should be downgraded if no progress is shown by next week.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
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?
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?
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).
Resolution: WORKSFORME → WONTFIX
(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.
(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.
(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?
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.
(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.
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?
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.
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?
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.
Component: Add-ons → General
Product: Tech Evangelism → WebExtensions
You need to log in before you can comment on or make changes to this bug.