Closed Bug 729761 (LA-hyperwords) Opened 12 years ago Closed 12 years ago

Liquid Words add-on creates zombie compartments

Categories

(WebExtensions :: General, defect)

Product:
WebExtensions
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: kmag, Unassigned)

References

(
URL
)

Details

(Whiteboard: [MemShrink:P3]) 

The Liquid Words add-on (formerly HyperWords) leaks content compartments after its functionality has been used. I visited http://enwp.org/Mahler, selected a number, and chose Convert > Currency > GBP > EU, closed the tab, and the compartment was leaked. I then went to a Google search page, selected a number, did the same, and two compartments were leaked.

Oddly, and this may be because I'm misunderstanding something, visiting other Wikipedia pages never created additional compartments after that point.

Author CCed and notified via AMO.
frode@liquid.info - are you looking into this and do you understand the issue?
Hi Kris, this is what I get from my developer, Alex. Does it help clarify the issue? He has worked on this for quite a while and if this doesn't help, please give us some tips as to how to fix this:

Please see https://developer.mozilla.org/en/Zombie_Compartments#Proactive_checking_of_add-ons

"Return to about:memory?verbose and hit the "minimize memory usage" button near the bottom, multiple times if necessary."

So you should try to click "Minimize memory usage" several times.
I would like to add the following: It can take some time (according to https://developer.mozilla.org/en/Zombie_Compartments#Proactive_checking_of_add-ons) to release memory. It can be just a few seconds. And mempry released for our addon after 2-3 clicks to the button 'Minimize memory usage'. That means our addon does not create zombie compartments because zombie compartments are not released after any number of clicks on that button.
frode: How long does it take in your experience to clear the reference?  I've just left Firefox for 5 minutes after closing the wikipedia tab and then hit the minimize memory button a few times but the compartment still persists.  If these aren't zombie compartments what are they?
Our standard procedure is to click Minimize memory multiple times before we classify a compartment as a zombie. I can confirm that the compartments created by your add-on when following the STR above are indeed zombies. While I would normally be willing to expend some effort looking for your leak, in a 1.2MB, 30KLOC JS file that's just not an option.
We're looking into this.
We found the issue with the zombies and have submitted a new version. We could not find the issue in 10.0 but we saw it on ff 10.0.2 and 11. We look forward to hearing what you find. Hopefully nothing...   :-)
Unfortunately I'm still getting a zombie compartment on https://addons.mozilla.org/ after a reference|wikipedia search. Also on http://www.mozilla.org/about (I was trying the difference between the lightbox on http and new tab on https). These aren't always consistently created however. Sometimes the compartment closes correctly, other times it remains.
Please review this soon since we are currently not compatible with Firefox 12. Why in the world is there such a fast upgrade cycle for Firefox?
Component: General → Add-ons
Product: Core → Tech Evangelism
The latest update did not receive full review because I can still reproduce the leak as mentioned in comment #8. The add-on will be downgraded to preliminarily reviewed if no progress is shown by 04/18.
Really? Another threat? This is too much, really too much. I'll see if I can get a coder to look into this within what I can afford to pay. Really, seriously uncool Jorge, seriously. An arbitrary cut of date? If it's really bad, cut it now. If it's not, why on the 18th? You make developing for Firefox a real and expensive pain.
Creating zombie compartments is a sufficient reason not to give full review to an extension. The "arbitrary" cut off date was chosen to give you sufficient time to work on this. If don't intend to fix this in your add-on, I can just downgrade it now, and you won't need to worry about this problem anymore. Preliminarily reviewed add-ons are allowed to have non-critical leaks such as this one.
Jorge, as I say in my reply, I'm working on finding someone to fix this 5 year old issue, which is, according to previous reviewer, significantly improved.
So far we cannot reproduce the bug at all. Do you have any more details as to how we can reproduce it? I'll have to hire a tester for this.
I reproduced it following these steps:

1) Go to https://addons.mozilla.org and look for some add-on page.
2) Select some text and wait for the context icon to appear.
3) Select Reference > Wikipedia and launch the wikipedia search.
4) Close both tabs.

The result is that one of the tabs (I think the AMO one) stays as a zombie compartment if you look in about:memory, and stays after minimizing memory and waiting for several minutes.

Please confirm that you have tested this and can't reproduce the problem.
Hi all.

This is what I had to write in the review note for our latest version, 9.11, which is improved, but not for all scenarios:

This has some of the issues with zombies as we have had in previous versions. The advice I am getting by coders who are looking at this (I am not a coder, I am a designer only) is that to fix it thoroughly would be quite a huge re-write and I simply cannot afford this now. I am committed to Liquid Words but I am launching Liquid Words for Mac OS X and therefore I can not spend much money on Firefox at this point. If you feel you have to pull Liquid Words, that would be a real shame as it's currently a featured add-on and downloads are growing and uses seem to like it (5/5 stars).
The add-on has been downgraded to preliminarily reviewed, pending the resolution of the memory leak. All future updates can continue to have the leak as long as they aren't nominated to be fully public. This leak might disappear in Firefox 15, with the resolution of bug 695480.

Feel free to ping me if you have a fixed version and I'll make sure it gets a quick review.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
I would love to have a look at Firefox 15 when it's available. I will not be able to work on fixing this bug in the short term though, I simply cannot afford it.
Why exactly is this RESOLVED WORKSFORME? Because the add-on has been downgraded?

(In reply to Jorge Villalobos [:jorgev] from comment #17)
> This leak might disappear in Firefox 15, with the resolution of bug 695480.

Did somebody test this? Bug 695480 isn't a silver bullet making all leaks impossible.
(In reply to Dão Gottwald [:dao] from comment #19)
> Why exactly is this RESOLVED WORKSFORME? Because the add-on has been
> downgraded?

Yes. Zombie compartments are allowed for add-ons with preliminary review, since the main criteria to pass preliminary review is security.

> (In reply to Jorge Villalobos [:jorgev] from comment #17)
> > This leak might disappear in Firefox 15, with the resolution of bug 695480.
> 
> Did somebody test this? Bug 695480 isn't a silver bullet making all leaks
> impossible.

No, no one has tested this yet. That's the reason I said 'might'.
(In reply to Jorge Villalobos [:jorgev] from comment #20)
> (In reply to Dão Gottwald [:dao] from comment #19)
> > Why exactly is this RESOLVED WORKSFORME? Because the add-on has been
> > downgraded?
> 
> Yes. Zombie compartments are allowed for add-ons with preliminary review,
> since the main criteria to pass preliminary review is security.

It still affects users, so I would think that the bug tracking this should stay open. Downgrading the add-on to preliminarily reviewed is an attempt to limit the damage, it's not a solution.
The question is whether the purpose of the Bugzilla leaky add-on bugs are to bring add-ons into conformance with policies or whether it's to eliminate leaks in popular add-ons to enhance user perception of Mozilla performance. Arguably there's some of the latter, given the drive to test the most popular add-ons no matter the hosting, so it's a question of where to draw the line. With 17K users and preliminary review status, are we concerned about tracking this issue as long as it doesn't violate AMO policies?
Yes, I don't see any point in keeping this open. It'll just be an inconvenience for Andrew and I who go through all pending leaks on a weekly basis. For those interested in looking for leaky add-ons with preliminary approval, you can just go to bug 700547 and look for dependencies with the WFM resolution.
(In reply to Jorge Villalobos [:jorgev] from comment #23)
> Yes, I don't see any point in keeping this open.

It still affects users and should get resolved properly. It doesn't "work for me" in any meaningful way. If you really want to close this now, the most sensible resolution would be WONTFIX.

> It'll just be an
> inconvenience for Andrew and I who go through all pending leaks on a weekly
> basis. For those interested in looking for leaky add-ons with preliminary
> approval, you can just go to bug 700547 and look for dependencies with the
> WFM resolution.

This seems backwards. Add a [AMO-preliminarily-reviewed] whiteboard tag and filter out these bugs when you're not interested in them? The idea that all leaky-add-on bugs resolved as WFM refer to add-ons that actually still leak doesn't make any sense -- based on <https://bugzilla.mozilla.org/page.cgi?id=fields.html#status>, one would assume that the leaks in those bugs couldn't be reproduced anymore without it being clear that anyone actually fixed the leak (e.g. the author never responded or no new version has been released).
We agreed on using WONTFIX for these cases.
Resolution: WORKSFORME → WONTFIX
Component: Add-ons → General
Product: Tech Evangelism → WebExtensions
You need to log in before you can comment on or make changes to this bug.