Last Comment Bug 731906 - Video DownloadHelper 4.9.8 add-on causes zombie compartments
: Video DownloadHelper 4.9.8 add-on causes zombie compartments
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on: TheGreatAddonTest
Blocks: LeakyAddons
  Show dependency treegraph
 
Reported: 2012-02-29 19:45 PST by Justin Lebar (not reading bugmail)
Modified: 2012-10-13 15:13 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Justin Lebar (not reading bugmail) 2012-02-29 19:45:36 PST
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
Comment 1 Justin Lebar (not reading bugmail) 2012-02-29 19:47:42 PST
cc Michel Gutierrez, whom I presume is a developer of this add-on per bug 678811.
Comment 2 Andrew Williamson [:eviljeff] 2012-03-14 05:40:29 PDT
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)
Comment 3 Michel Gutierrez 2012-03-14 07:16:37 PDT
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) ?
Comment 4 Andrew Williamson [:eviljeff] 2012-03-14 10:12:28 PDT
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.
Comment 5 Michel Gutierrez 2012-03-15 03:14:27 PDT
(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.
Comment 6 Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) 2012-03-15 09:40:45 PDT
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.
Comment 7 Jorge Villalobos [:jorgev] 2012-03-21 12:37:13 PDT
Michel, have you made any progress in fixing this bug?
Comment 8 Michel Gutierrez 2012-03-21 17:48:37 PDT
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.
Comment 9 Michel Gutierrez 2012-03-22 15:32:56 PDT
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.
Comment 10 Michel Gutierrez 2012-03-22 16:26:52 PDT
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 ?
Comment 11 Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) 2012-03-23 02:14:10 PDT
Confirmed fixed with linked file and Firefox 11.0 32-bit on Windows XP SP 3 using the steps from comment #0.
Comment 12 Andrew Williamson [:eviljeff] 2012-03-23 04:26:54 PDT
Once the fixed version has been uploaded to AMO and reviewed we can close this bug.
Comment 13 Michel Gutierrez 2012-03-26 06:36:56 PDT
I should be able to submit a new VDH update to amo with the fix by the middle of this week.
Comment 14 Michel Gutierrez 2012-03-29 05:41:24 PDT
DownloadHelper version 4.9.9 with this memory fix has been submitted to AMO.
Comment 15 Andrew Williamson [:eviljeff] 2012-03-29 07:04:11 PDT
Version 4.9.9 has been reviewed now on AMO so closing bug!
Comment 16 Nicholas Nethercote [:njn] 2012-03-30 02:31:52 PDT
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! :)

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