Memory leak at Dean Edwards' IE7 site

RESOLVED WORKSFORME

Status

()

defect
--
major
RESOLVED WORKSFORME
13 years ago
a month ago

People

(Reporter: schapel, Unassigned)

Tracking

({memory-leak})

Trunk
x86
Windows XP
Points:
---
Bug Flags:
blocking1.9 -
wanted1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
Reproducable: Always

Steps to Reproduce:
1. Start Firefox
2. Go to http://dean.edwards.name/IE7/
3. Click the Back button
4. Close Firefox

Expected Results: no memory leak
Actual Results: leak gauge reports leak of 3 DOM Windows and 3 documents

I've reproduced this with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060120 Firefox/1.6a1 with a new profile and SeaMonkey trunk build 2006011909 with an old profile. Going to the page multiple times by selecting it from the URL bar dropdown and clicking the Back button makes even more objects leak, but going back to the page clicking the Forward and Back buttons does not make more objects leak.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060120 Firefox/1.6a1 ID:2006012005
I see this leak too.

Leaked inner window 16bdb38 (outer 2018e88) at address 16bdb38.
 ... with URI "http://dean.edwards.name/IE7/".
Leaked outer window 2018e88 at address 2018e88.
Leaked document at address 2873678.
 ... with URI "http://dean.edwards.name/my/behaviors/moz-behaviors.xml".
Leaked document at address 2871b28.
 ... with URI "http://dean.edwards.name/my/behaviors/bindings.xml".
Leaked document at address 266bf38.
 ... with URI "http://dean.edwards.name/IE7/".
(Reporter)

Updated

13 years ago
Summary: Memory leak at Dean Edward's IE7 site → Memory leak at Dean Edwards' IE7 site
So.. I hate to say this, but would someone be willing to minimize this XBL/JS/etc?  As things stand, this thing is so huge I have no idea where to even start.

I _think_ the thing that's making us leak is:

b3dc9320 object 0x86c77b8 HTMLDocument via XPCWrappedNativeProto::mJSProtoObject(XPC_WN_ModsAllowed_Proto_JSClass @ 0xb3dc96d0).XPCWrappedNativeScope::mPrototypeJSObject(Object @ 0xb3dc9688).constructor(Function @ 0xb3dc9680).__proto__(Function @ 0xb3dc96b8).caller(Function @ 0x0895c8c8).__parent__(With @ 0x0895c3f0).__proto__(HTMLPreElement @ 0xb3ce37e8).__parent__(HTMLDocument @ 0xb3dc9320).

but I'm really not sure whether that's true and what the rest of the cycle is if so....

I also see us leaking an nsJSContext via the ref taken in NS_NewJSEventListener.  Not sure whether that's relevant here....
Here, I tried to minimise it:
http://wargers.org/mozilla/bug324145/
http://wargers.org/mozilla/bug324145/_IE7_.htm
Somehow, it only leaks 2 windows locally, not on the website, so this already might be a wrong minimalisation.
When I remove this code, the leak disappears:
            // load the behavior
            _httpRequest.open("GET", $url, false);
            _httpRequest.send(null);
            return _httpRequest.responseText;

Maybe this is (partly) related to bug 323454, because in this case, the string that I used for the xmlhttprequest, is also obtained by referencing an property from a dom node ( getComputedStyle(this, null).getPropertyValue("-moz-binding") ).
That test case doesn't leak over here.  :(
Posted file zipped testcase, not minimal (obsolete) —
Well, at least I managed it reducing it to this. It's still terribly complicated, though. This leaks for me 2 dom windows and 3 documents.
A few more reduction steps.  The salient points now are:

1)  The node involved has an on* event attr.  Doesn't matter so much what the value is; "void(1)" leaks.

2)  The XBL loads a script via XMLHttpRequest and turns it into actual JS using new Function().

3)  The function body is completely wrapped in "with (this) { }".

4)  Whatever is going on, the call to parse() is needed to leak.
Attachment #212903 - Attachment is obsolete: true
So there are clearly two things here -- the rigmarole that actually executes this script, and the script itself.  I'm starting to wonder whether the XBL binding per se is even needed.... I'm also starting to wonder whether we're close to being able to nix the XMLHttpRequest silliness and just stick all this stuff into a string, even if the XBL binding is needed.

Martijn, do you think you can take a crack at removing whatever else you can in the "behavior" file?
Posted file Minimised testcase
Thanks Boris! I admit, I wasn't too keen on minimising, because it's really tedious work (more tedious than html cases).

This leaks 2 docs and 2 windows for me. It doesn't involve xbl nor xmlhttprequest.
OK, I've done some more tracking, and I'm really not sure how we're managing to leak this stuff.  Here's what I know so far:

1)  The HTMLAnchorElement holds a strong ref to the outer window (via the
    nsJSEventListener object, which holds a strong ref to the nsJSContext, which
    locks the JSObject of the window, which keeps the window alive).
2)  The Window's scope seems to be cleared so it's not entraining anything that
    I can see.
3)  The XPCWrappedNativeProto that's entraining the HTMLAnchorElement and the
    HTMLDocument is the proto for HTMLDocument.

I'm really not sure how the window keeps the XPCWrappeNativeProto alive (which is what has to be happening).  Especially since we do NOT leak an XPCWrappedNativeScopes.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Using the minimized testcase I don't see window or document leaks anymore.
Yeah, doesn't seem to leak anymore. I tried the minimized testcase, the " zipped testcase, not minimal" and the site. All of them don't seem to leak anymore.
Marking worksforme?

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → WORKSFORME
I bet the cycle collector fixed this.  Most excellent!
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.