Skip to content

IE8 memory leak loading mootools in an iframe #2329

Closed
kguelzau opened this Issue Mar 13, 2012 · 4 comments

4 participants

@kguelzau

Here is another memory leak.
Memory usage in increases in Process Explorer on every reload of index.html

IE: 8.0.6001.18702
Moo: 1.4.5 / trunk

index.html:

<!DOCTYPE HTML>
<html>
  <body>
    <iframe src="iframe.html"></iframe>
  </body>
</html>

iframe.html:

<!DOCTYPE HTML>
<html>
  <head>
    <script src="mootools.js" type="text/javascript"></script>
  </head>
  <body>
    iframe
  </body>
</html>

Adding another cleanup (this.Window) to the unload event fixes the issue for me:

var unloadEvent = function(){
  this.detachEvent('onunload', unloadEvent);
  document.head = document.html = document.window = this.Window = null;
};
@fakedarren fakedarren closed this Jul 28, 2012
@kguelzau
kguelzau commented Aug 6, 2012

No comment or commit? Just closed?

@ibolmo ibolmo reopened this Aug 6, 2012
@ibolmo
MooTools member
ibolmo commented Aug 6, 2012

@fakedarren did you intend to move it?

@cpojer
MooTools member
cpojer commented Aug 6, 2012

Can someone set this.Window and this.Document to null, make sure it doesn't throw in IE and send a pull-request? Seems simple.

@okolvik okolvik added a commit to okolvik/mootools-core that referenced this issue Mar 3, 2013
@okolvik okolvik Set this.Window and this.Document to null on unload.
Fix for issue #2329

Tested in IE7, 8, 9 and 10, and the latest releases Firefox and Chrome.
Frees up large amounts of memory when navigating away from a page.
76e3219
@kguelzau

Just an Update on this Issue.

I modified the iframe.html (ugly onload):

<!DOCTYPE HTML>
<html>
  <head>
    <script src="mootools.js" type="text/javascript"></script>
    <script type="text/javascript">
     function init() { 
       //nothing
     }
  </script>
  </head>
  <body onload="init()">
    iframe
  </body>
</html>

And expanded the cleanup in the unload event (Core.Browser):

var unloadEvent = function(){
  this.detachEvent('onunload', unloadEvent);
  document.head = document.html = document.window;
  // cleanup scope vars
  window = this.Window = document = null;
};

AND modified domready() in DOMReady

var domready = function(){
  clearTimeout(timer);
  if (!ready) {
    Browser.loaded = ready = true;
    document.removeListener('DOMContentLoaded', domready).removeListener('readystatechange', check);
    document.fireEvent('domready');
    window.fireEvent('domready');
  }
  // cleanup scope vars
  document = window = testElement = null;
};

This fixes this issue for me. again... ;-)

@okolvik okolvik added a commit to okolvik/mootools-core that referenced this issue Mar 28, 2013
@okolvik okolvik Incorporating changes by kguelzau
Comment thread: #2329
Pull request: #2476
f4b1c51
@ibolmo ibolmo modified the milestone: 1.5.1, 1.5 Mar 3, 2014
@SergioCrisostomo SergioCrisostomo added a commit to SergioCrisostomo/mootools-core that referenced this issue Jul 11, 2014
@okolvik okolvik Set this.Window and this.Document to null on unload.
Fix for issue #2329

Tested in IE7, 8, 9 and 10, and the latest releases Firefox and Chrome.
Frees up large amounts of memory when navigating away from a page.
435d206
@SergioCrisostomo SergioCrisostomo added a commit to SergioCrisostomo/mootools-core that referenced this issue Jul 11, 2014
@okolvik okolvik Incorporating changes by kguelzau
Comment thread: #2329
Pull request: #2476
bb5fdbc
@SergioCrisostomo SergioCrisostomo added a commit that referenced this issue Jul 22, 2014
@okolvik okolvik Set this.Window and this.Document to null on unload.
Fix for issue #2329

Tested in IE7, 8, 9 and 10, and the latest releases Firefox and Chrome.
Frees up large amounts of memory when navigating away from a page.
9c74273
@SergioCrisostomo SergioCrisostomo added a commit that referenced this issue Jul 22, 2014
@okolvik okolvik Incorporating changes by kguelzau
Comment thread: #2329
Pull request: #2476
4783b9e
@SergioCrisostomo SergioCrisostomo added a commit that closed this issue Jul 22, 2014
@SergioCrisostomo SergioCrisostomo Merge PR-2476: Fix for IE8 iFrame leak
closes #2476, fixes #2329

* pr/2476:
  Spaces to tabs
  Add missing null.
  Incorporating changes by kguelzau
  Set testElement to null on domready
  Set this.Window and this.Document to null on unload.
1d38828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.