Open Bug 922597 Opened 11 years ago Updated 1 year ago

browser.xul windows leak if closed before load event fires

Categories

(Firefox :: General, defect)

x86
Linux
defect

Tracking

()

People

(Reporter: santiago.gimeno, Unassigned)

References

Details

(Whiteboard: [MemShrink:P2])

I have a very simple addon that just closes / opens a window every X seconds. The module 'sdk/windows' is not really needed in this module but if you require it the memory grows with every iteration of the interval whereas it doesn't grow if you don't require it. I'm observing this behaviour using addon-sdk-1.14 w FF24 and addon-sdk-1.12 w FF17.

The main.js of my addon is:

var page_mod = require('sdk/page-mod');
var timers = require('timers');
var win_utils = require('sdk/window/utils');
var windows = require("sdk/windows");

var main_page_mod = page_mod.PageMod({
    include : ['*'],
    onAttach : function(worker) {
        var alert_window;
        timers.setInterval(function() {
            if (alert_window) {
                console.log('closing window');
                alert_window.close();
                alert_window = undefined;
            }

            alert_window = win_utils.open('chrome://browser/content/browser.xul', {
                name : 'simple addon',
                features: {
                    width : 100,
                    height : 100,
                    chrome : true,
                    left : 100,
                    top : 100
                }
            });
        }, 6000);

        worker.on('detach', function() {
            main_page_mod.destroy();
        });
    }
});
I'm not sure why I'm needinfo'ed on this, someone needs to investigate if why is this happening. If memory
usage constantly growing without being reclaimed that's a serious problem.
Flags: needinfo?(rFobic)
Jordan, can you try to investigate this?
Assignee: nobody → jsantell
Confirmed, spamming window opens and closes causes memory to shoot up, mostly from window-objects -> top(none)/detached -> window([system]) -> js-compartment([SystemPrincipal], about:blank), judging from about:memory -- will look into it more
Not sure what's happening here quite yet. Looks like when opening/closing windows in scratchpad using the window watcher directly, it reclaims memory. When running inside jetpack code, it is not reclaimed (either using the jetpack module, or the WW directly)

https://gist.github.com/jsantell/7091737
How big is this leak, and does it go away after GC?
Around 200MB alone for creating 100 windows, and GC does not clear it
When rapid fire open/closing windows (using WindowWatcher xpcom, none of our Window modules are used) in an addon, the memory is never recollected (although recollected when in scratchpad with or without addons installed).

When opening and closing windows via waiting for ready event, or waiting about a second, the memory is successfully collected.

It appears that closing a window before it's completely opened, only in an addon, causes this issue. We suspect something in bootstrap or addon is holding on (doesn't look like it), or possibly the platform doesn't correctly trash objects that were 'touched' by an addon and closed correctly
Unassigning myself from bugs I haven't gotten around to
Assignee: jsantell → nobody
Whiteboard: [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P2]
Priority: P1 → --
This is CustomizableUI's gListeners holding on to a TabsInTitlebar instance:

0000022E9A73B060 [BackstagePass 22e990b3a00]
    --[gListeners]--> 0000022E9A72DE80 [Set 22e995753a0]
    --[key]--> 0000022EA2959220 [Proxy <no private>]
    --[private]--> 0000022EB5FC00E0 [Object <no private>]
    --[uninit]--> 0000022EB5FEDE00 [Function TabsInTitlebar.uninit]
    --[fun_environment]--> 0000022EB5AFA070 [Block <no private>]
    --[enclosing_environment, **UNKNOWN SLOT 1**]--> 0000022EB5AF9060 [Window <no private>]

I think the easiest thing to do here is use a WeakSet in CustomizableUI.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Seems the bigger issue is that we aren't firing unload events for these windows.  So browser.js isn't running any of its cleanup code.
So, I can't reproduce the part from comment 0 that says:

> The module 'sdk/windows' is not really needed in this module but if you require it the memory grows with every iteration of the interval whereas it doesn't grow if you don't require it.

Testing the gist in comment 4 still leaks, but I don't think its an addon issue.  We simply do a lot of resource initialization in browser/base/* code before the load event fires.  We then fail to clean this stuff up because unload is not fired (we try not to fire unload if load didn't fire).

I think fixing this would require a fair amount of work to fix many issues in browser/base/*.  Its not clear its worth it, though, since opening and closing a window quickly is sort of a corner case.  It doesn't really happen in practice that often.  Maybe it happens in our tests sometimes causing intermittent leaks, though.

Anyway, I'd rather focus on things our users are likely to hit in real use, so I'm not going to work this right now.
Assignee: bkelly → nobody
Status: ASSIGNED → NEW
Product: Add-on SDK → Firefox
Summary: Memory leak when requiring 'sdk/windows' module → browser.xul windows leak if closed before load event fires
Lets re-triage this since the current leak here is different from the original leak.
Whiteboard: [MemShrink:P2] → [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P2]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.