Opened 10 years ago

Closed 9 years ago

#10628 closed defect (fixed)

Memory Leak / IE / windows/frames

Reported by: Dustin Machi Owned by: James Burke
Priority: high Milestone: 1.6
Component: General Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

We have a report from a customer about some memory leaks we have also seen and I have talked with others about in the past when dojo is loaded in a child window or frame in IE.

From the developer: "We have found these particular cases where a page leaks (in IE 6, 7, and even 8 in some cases) - even if the page exhibits no normal circular reference leaks through regular page navigation:

  • opening and then closing a new tab or window (only a problem in IE 8 if it is configured to run in a single process)
  • running a page inside of an Iframe and reloading the parent frame
  • running IE embedded in a desktop app and opening/closing new windows (this is always a problem in IE 8)

Dustin is aware of this leak (same problem was saw in the test tool). This issue can reproduced with non-dojo code. For example, it is reproducible on MSN.com, which is using the prototype framework. I have narrowed it down to two places in the dojo framework: In dojo\_base\_loader\hostenv_browser.js, I adding setting the d and _w variables to null in the window.Unloaded function:"

This reduced the leaks, but does not completely eliminate them. The current code he is using looks like:

d.windowUnloaded = function() {
            // summary:
            //        signal fired by impending window destruction. You may use
            //        dojo.addOnWindowUnload() to register a listener for this
            //        event. NOTE: if you wish to dojo.connect() to this method
            //        to perform page/application cleanup, be aware that this
            //        event WILL NOT fire if no handler has been registered with
            //        dojo.addOnWindowUnload. This behavior started in Dojo 1.3.
            //        Previous versions always triggered dojo.windowUnloaded. See
            //        dojo.addOnWindowUnload for more info.
            var mll = d._windowUnloaders;
            while (mll.length) {
                (mll.pop())();
            }
            d = onunload = onbeforeunload = null;
            delete window.onunload; 
            delete onbeforeunload; 
}

Does this look ok to you and or do you think there is a better way to fix it?

Change History (11)

comment:1 Changed 10 years ago by James Burke

Milestone: tbd1.5

I expect that the only thing that helps in that code is setting d = null? We do not set onunload or onbeforeunload directly? It would be good to verify if just setting d = null is sufficient.

comment:2 Changed 10 years ago by Shane O'Sullivan

Will this make it into 1.5? We're seeing the same issue at my company, and making this change definitely helps.

comment:3 Changed 10 years ago by James Burke

shaneosullivan: can you confirm that setting d = null; above is sufficient to see the improvement you are noticing? I do not expect the rest of that code to make a difference. If d = null is enough, then I can add that in for 1.5.

comment:4 Changed 10 years ago by Shane O'Sullivan

Yes, setting d to null does the trick for us.

comment:5 Changed 10 years ago by James Burke

Resolution: fixed
Status: newclosed

(In [21994]) Fixes #10628, helps out IE memory management, thanks to shaneosullivan for confirming the simplified fix.

comment:6 Changed 10 years ago by Dustin Machi

Sorry I didn't notice this thread. I had asked my client to test this against 1.4.x (as their patches were against 1.3) and they had reported that they weren't seeing this issue anymore after all those memory leak fixes that Bill had put in place. In any case, I don't think the d=null hurts anything. Thanks James and Shane.

comment:7 in reply to:  6 ; Changed 9 years ago by ancaa

d=null is not enough for my project. I still have problems with my memory. IE rich 1.5 G only in one hour ( from 100 M)....

comment:8 in reply to:  7 Changed 9 years ago by ancaa

Resolution: fixed
Status: closedreopened

Is not normal that on all the files where is something like this: d.windowUnloaded=function(){ var mll=d._windowUnloaders; while(mll.length){ (mll.pop())(); } }; to put d = null;? I have looked in all js files and the problem seems to disappear. I will do more tests.

I have modify in hostenv_ff_ext.js and in dojo.js

comment:9 Changed 9 years ago by James Burke

Milestone: 1.51.6

ancaa: if you can be more specific with the changes you did, hopefully via a patch or a diff, that would be helpful.

comment:10 Changed 9 years ago by ancaa

After a long debugging, i think i have found the problem, i have put 2 bugs: http://bugs.dojotoolkit.org/ticket/11310 and http://bugs.dojotoolkit.org/ticket/11328, so you can close this ticket. Thank you

comment:11 Changed 9 years ago by Jared Jurkiewicz

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.