Each window leaks memory (only) when Vimperator is enabled

[anderspapitto]

With vimperator enabled, opening a new new window and then closing it again leaks memory, which about:memory categorizes under Explicit Allocations -> explicit -> window-objects -> top(none)/detached. Likely there are other leaks as well, but this one I have confirmed explicitly.

I am running Linux, Firefox 39.0 and Vimperator 3.9.1-signed.

To Reproduce

Disable all addons except for Vimperator, and restart Firefox. Go to about:memory

then, open a new window (OS-level window, not tab), and close it again. For more dramatic results open multiple - I ran "for i in {0..10}; do firefox & done" from a shell.

now, in the about:memory window, click "Free Memory -> Minimize memory usage", then "Show memory reports -> Measure". Look at Explicit Allocations -> explicit -> window-objects -> top(none)/detached. This memory (as a lower bound) is leaked due to Vimperator.

Expected Behavior

Disable all addons, this time including Vimperator, and restart Firefox. Go through the same steps as the "To Reproduce" section above. This time, after "Minimize memory usage", "Measure" should not have any "top(none)/detached" entry.

To me, this indicates that Vimperator is probably preventing some page-level objects from being gc-ed. However I don't know much about Firefox and/or Vimperator implementations, so I'm happy to take any further measurements that help clarify.

Severity - this depends on use pattern, I guess. I always use windows instead of tabs, and do tabbing at the os/window-manager level. For me, I commonly hit about 2G of memory leak and OOM after about a day. For someone who rarely opens new windows, probably this is less of an issue.

[gkatsev]

You're probably correct. Unfortunately, we probably won't be able to address this immediately since we have issues just trying to keep the plugin available in new version of firefox :)
However, hopefully, I'll be becoming more and more familiar with the codebase and will be able to fix things up. However, if you feel like trying to figure things out yourself, that would be pretty awesome as well.
This is probably related to vimperator incompatibilities with firefox's e10s.

[nagisa]

This is probably related to vimperator incompatibilities with firefox's e10s.

The issue happens without e10s. I can reproduce the issue.

[amccreight]

I work on Firefox and I've been investigating the window leaks related to Vimperator ( https://bugzilla.mozilla.org/show_bug.cgi?id=873163 ). As far as I can tell, there are at least 3 things that are keeping windows alive after they are closed.

1. The bookmarks Cache registers itself with the nsINavBookmarksService as a strong observer. This means that the service keeps alive the Cache for a window that has been closed. I think that changing the second argument to addObserver to "true" should fix this.
   

   vimperator-labs/common/content/bookmarks.js

   Line 184 in 144894b

   bookmarksService.addObserver(observer, false);

2. In StatusField, there is code that adds a reference to a XUL element:
   statusline._statuslineWidget.appendChild(this.node);
   I think this _statuslineWidget is a global data structure, but it is being given a reference to an element form a particular window, so this leaks thewindow.
   

   vimperator-labs/common/content/statusline.js

   Line 32 in c0ce5ca

   statusline._statuslineWidget.appendChild(this.node);

3. There's some field commandline._completionList._items.editor (or something like that) where the editor is some kind of nsIEditor thing for the window that has been closed, which is holding it alive.

I think maybe for 2 and 3 you might be able to convert them to using weak references, though then you'd have to deal with checking for thing that went away.

[nagisa]

Thanks for all the work you’ve put into it, @amccreight!

[gkatsev]

@amccreight that's awesome. Thanks for your work. I'll definitely work on bringing these in over the weekend.
Do you have any details on how I can try and track this myself? For example, when making these changes?

[gkatsev]

1. I'm currently investigating whether doing change for the bookmarks observer causes any issues (probably not)
2. The particular line doesn't seem to be called directly from vimperator. It possibly gets called if a user adds an addon item to the vimperaor statusline. I'll investigate.

But yeah, for 2 and 3, we probably should either convert to weak references or be super diligent in cleaning up after ourselves. Thanks @amccreight.

[amccreight]

To investigate things like this yourself, first reproduce the leak. Then go to about:memory and click on "Save verbose". This will give you CC and GC logs. Grep through the CC logs for nsGlobalWindow for browser.xul. One of these will be the actual window, but the ones with a lower number in rc={some number} is going to be the leaking windows.

Once you have the address of one of those leaking windows, you need my find_roots script from here:
https://github.com/amccreight/heapgraph
Then you do something like
find_roots.py

and that will show you at least one reason why the window is alive, as a path from some object to the window. If the object keeping the window alive is a marked GC object, you have to run
find_roots.py
to find out why the GC is keeping that object alive.