380837 - leaking the Firefox window when run with about:blank or file: command line parameters

Status: RESOLVED DUPLICATE of bug 383269

(Core :: DOM: Core & HTML, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If I start up Firefox from the command line with an "about:blank" command-line parameter, and then immediately quit, 2 nsGlobalWindows are leaked. These are the inner and outer nsGlobalWindows for browser.xul.

Interestingly, if I start up with no command-line URLs, or with any set of command line URLs including at least one http: URL, we don't leak. If I start up with any positive number of command-line URLs consisting entirely of file: or about:blank, we leak. If I start up with about:config, we don't leak, but if I start up with about:config and about:blank, we do leak...

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If I turn new-textframe on, we crash in a cairo assert on shutdown instead of just leaking; the leak prevents nsLayoutStatics::Shutdown from running, so my cleanup code never gets to run. I could work around this but crashing when we leak is perhaps no bad thing...

Comment 2

[Johnny Stenback (:jst)]

Obviously good to get this fixed, but not a blocker as this wouldn't be much of a problem for average users.

Roc, do you know if this is a regression, and if so, any clue when it started happening? Related to the fix for bug 378987?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No idea when it started, sorry.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Are we pretty sure the about:blank thing is the only way to trigger this leak?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Not at all. That's just the only way I know to trigger it right now.

Comment 6

[WADA:World Anti-bad-Duping Agency]

Similar leak is reported to Bug 381992 for Tb 2.0 & latest-trunk of Tb on MS Win.
When Tb, the leak is produced by "simply start Tb & close Tb immediately".
Is this bug Mac OS X only issue?

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v1

I'm seeing a leak when starting up and shutting down with a profile with the homepage set to about:blank. This patch fixes it for me. The safebrowsing component keeps a reference to the tabbrowser, which ends up keeping a XUL document alive. By the time the component shuts down and releases its reference, the cycle collector has already shut down and so we don't collect any cycles involving the XUL document. I have no idea why this only happens for about:blank or local files.

Roc: could you try this patch and let me know if it fixes the leak?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This seems to fix things for me.

Is there a way to change shutdown so that the cycle collector runs after all JS components have shut down? It would be nice for JS components and extensions to not be vulnerable to leaks in this way.

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to comment #8)
> Is there a way to change shutdown so that the cycle collector runs after all JS
> components have shut down? It would be nice for JS components and extensions to
> not be vulnerable to leaks in this way.

Bsmedberg?

(This isn't really specific to JS components. though it's probably more likely to happen there because it's harder to spot strong references).

Comment 10

[Benjamin Smedberg]

Define "shut down"... JS components really shouldn't be holding XPCOM references forever, they should be nulling them out at xpcom-shutdown *or* holding them alive via XPCOM objects that can be cycle-collected.

We can't really run cycle/garbage collection after we start releasing modules, because you don't have any guarantees that the other module still exists.

See http://wiki.mozilla.org/XPCOM_Shutdown for the "correct" sequence: the only part of that which is unimplemented is xpcom-shutdown-gc.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Could we tear down the JS roots for JS components before we run cycle collection for the last time? Without actually unloading those components?

Comment 12

[Benjamin Smedberg]

Do you mean to remove all properties of the global object, or something else?

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, that. I think. I'm shooting in the dark

Comment 14

[Tony Chang (Google)]

Comment on attachment 266468 [details] [diff] [review]
v1

review+, but I don't understand why the XUL tabbrowser causes an xpcom leak.

Random guess: maybe the web progress listeners aren't getting removed properly on DOMWindowClose.

Comment 15

[Tony Chang (Google)]

Better random guess:  it may have something to do with about: and file: being flagged as spurious (not checked by phishing) in browser/components/safebrowsing/src/nsDocNavStartProgressListener.cpp.
Does starting firefox with chrome: URLs also trigger the leak?