Closed Bug 731906 Opened 12 years ago Closed 12 years ago

Video DownloadHelper 4.9.8 add-on causes zombie compartments

Categories

(Core :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: justin.lebar+bug, Unassigned)

References

Details

Reported in the greataddontest's etherpad [1]:

-----------------------------------------------------------------------------
+ Video DownloadHelper - https://addons.mozilla.org/addon/3006
-----------------------------------------------------------------------------
- Tested by:       Archaeopteryx
- Where obtained:  Add-ons Manager (AMO)
- Firefox version: 10.0.2
- OS version:      Windows XP SP 3
- Add-on version:  4.9.8
- Testing:         
1. Open a video url, let's say  http://www.youtube.com/watch?v=ZB6ryZ1x8rY (you don't need to watch it  in full length, just let it start). 
2. Download the video by using the extension. 
3. Close the tab.
4. Open about:memory?verbose. 
5. Run 'Minimize memory usage' three times.
Actual result: Zombie comparment for youtube
Add-on developer has been informed about the bug in early October 2011 (add-on version was 4.9.5)
Sometimes, I can reproduce without downloading (just starting the video and then closing the tab)

[1] https://etherpad.mozilla.org/hSOVNzKZen
cc Michel Gutierrez, whom I presume is a developer of this add-on per bug 678811.
Blocks: LeakyAddons
No longer depends on: LeakyAddons
Michel - is there any further information/help you need with this?

If we don't get an updated version we'll have to downgrade the listing on addons.mozilla.org to preliminary reviewed to reduce the users it impacts.

(sent via info request also)
I made some tests on this problem and the results are not very conclusive:

- most of the times, after downloading the video and closed the tab, about:memory?verbose (after reload or minimize mem) does not show any youtube compartment.
- some times, there was a youtube compartment persisting to minimize mem, but which disappeared after a few minutes (5 to 10)

I also noticed (but not 100% sure) that the youtube compartment never showed up when the video was taken from the cache (as per Firefox download manager action). Could this simply be due to the Firefox download manager (invoked by VDH) just keeping a reference for a few minutes to the page holding the downloaded file ?

Can you confirm/infirm that in any case the memory compartment is released after, say, 10 minutes ?

Also, when doing the test, did you have the History feature of VDH enabled (by default it's disabled, and can be enabled from the add-on preferences, tab Miscellaneous) ?
I've just tested now

(In reply to Michel Gutierrez from comment #3)
> I made some tests on this problem and the results are not very conclusive:
> 
> - most of the times, after downloading the video and closed the tab,
> about:memory?verbose (after reload or minimize mem) does not show any
> youtube compartment.
> - some times, there was a youtube compartment persisting to minimize mem,
> but which disappeared after a few minutes (5 to 10)

I left Firefox for 10 minutes and then pressed minimize memory usage a few times but the compartment remained.  Incidentally, just viewing the page didn't leave the whole compartment for me but did leave a reference in the DOM section of about:memory (less than 1k though)

> I also noticed (but not 100% sure) that the youtube compartment never showed
> up when the video was taken from the cache (as per Firefox download manager
> action). Could this simply be due to the Firefox download manager (invoked
> by VDH) just keeping a reference for a few minutes to the page holding the
> downloaded file ?

Can you suggest a way to confirm/deny this idea?  A direct url to a video that can be downloaded via the download manager?

> Can you confirm/infirm that in any case the memory compartment is released
> after, say, 10 minutes ?

Its still there.

> Also, when doing the test, did you have the History feature of VDH enabled
> (by default it's disabled, and can be enabled from the add-on preferences,
> tab Miscellaneous) ?

No.  All settings were left as the defaults.
(In reply to Andrew Williamson [:eviljeff] from comment #4)

> > - most of the times, after downloading the video and closed the tab,
> > about:memory?verbose (after reload or minimize mem) does not show any
> > youtube compartment.
> > - some times, there was a youtube compartment persisting to minimize mem,
> > but which disappeared after a few minutes (5 to 10)
> 
> I left Firefox for 10 minutes and then pressed minimize memory usage a few
> times but the compartment remained.  Incidentally, just viewing the page
> didn't leave the whole compartment for me but did leave a reference in the
> DOM section of about:memory (less than 1k though)

I ran another test and this time, the compartment remains 2 hours after the download.

More strangely, it also keeps a compartment for another youtube page i visited and closed without downloading.

But i just noticed something interesting: when i switch back to the about:memory tab, VDH thinks i'm still in the youtube page and animates the icon with the youtube download offers. This is obviously a bug and it explains why the memory is not freed (at least it gives someting to investigate upon). I'm working on this.

> > I also noticed (but not 100% sure) that the youtube compartment never showed
> > up when the video was taken from the cache (as per Firefox download manager
> > action). Could this simply be due to the Firefox download manager (invoked
> > by VDH) just keeping a reference for a few minutes to the page holding the
> > downloaded file ?
> 
> Can you suggest a way to confirm/deny this idea?  A direct url to a video
> that can be downloaded via the download manager?

I made a test on http://ftp.mozilla.org/ and downloaded a firefox release (if that was a download manager issue, this shouldn't make a difference whether it's a video or any other file). However, the corresponding compartment was released quickly, so i don't think anymore the download manager is involved in the issue.
The compartments are _not_ page/tab specific, the compartment usually gets its url from the page which created it and tabs opened later can also use it, even after the initial opening tab has been closed.
Michel, have you made any progress in fixing this bug?
Jorge: Not much, i had to finish launching a new service: done today http://www.jocly.com/ 
Fixing this addon issue is now my top priority.

Sebastian: Thanks for the information. But this means my previous assumptions irrelevant. So, how do we know a compartment is a zombie if it's simply being used by another tab ?

I haven't yet being able to reproduce the about:memory/youtube mismatch. I keep trying.
I could reproduce the memory problem reliably so investigation is now possible.

The memory leak occurs when you open the VDH download menu. Something, somewhere, keeps a reference to the menuitem elements used to construct the first level download menu (other levels are not a problem). This might be in VDH, but i don't think so, or simply some optimisation from the Firefox. The point is that each menuitem has a listener that contains a reference to the download entry properties. In the case of youtube, those properties contain a reference to the original page document. As a consequence, even when the menu is properly cleared by VDH, the youtube page document remains in memory.

We now know where the problem is, i must find the best way to fix it.
I fixed the zombie compartment issue (as i could see it in my setup) by using a weak reference in the click listener to the download entry properties object.

I made a test version available at http://www.downloadhelper.net/install-beta.php?version=4.9.9a3

Can someone confirm the fix ?
Confirmed fixed with linked file and Firefox 11.0 32-bit on Windows XP SP 3 using the steps from comment #0.
Once the fixed version has been uploaded to AMO and reviewed we can close this bug.
I should be able to submit a new VDH update to amo with the fix by the middle of this week.
DownloadHelper version 4.9.9 with this memory fix has been submitted to AMO.
Version 4.9.9 has been reviewed now on AMO so closing bug!
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
FWIW, it would have been nice to have a MemShrink tag on this, since Video DownloadHelper is the 2nd most popular add-on on AMO! :)
You need to log in before you can comment on or make changes to this bug.