Prototype

Comments and changes to this ticket

• 

  John-David Dalton September 4th, 2008 @ 08:32 AM

  • → Title changed from “Prototype 1.6 - new Element() constructor” to “Prototype 1.6 - memory leak in new Element() constructor”
• 

  John-David Dalton September 15th, 2008 @ 08:52 PM

  Here is my Screenshot of IE 6/7 using your test and Prototype 1.6.0.2 (I don't see the stair step memory leak signs).

  • IE7__no_leak_for_me_.gif 20.2 KB
• 

  John-David Dalton September 15th, 2008 @ 08:51 PM

  • no changes were found...

  • IE6-_no-leak-for-me_.gif 20.9 KB
• 

  John-David Dalton September 15th, 2008 @ 08:53 PM

  Prototype 1.6.0.2 did reintroduce a memoryleak in some unpatched versions of IE6, but that is in the event observer code and not the Element constructor.

• 

  John-David Dalton September 15th, 2008 @ 10:29 PM

  I have been able to reproduce the JScript Leaks Detector leak alert. Though no trace on the memory footprints.

  Leaks !

  
  (function() {
    var backup = this.Consume;
    this.Consume = function(tagName) {
      Consume.cache = document.createElement('div');
      Consume.cache._extendedByPrototype = String.prototype.slice;
    };
   this.Consume.cache = { };
  })();
  
  Consume();
  

  A) No Leak: Remove the function assignment

  
  (function() {
    var backup = this.Consume;
    this.Consume = function(tagName) {
      Consume.cache = document.createElement('div');
      // Consume.cache._extendedByPrototype = String.prototype.slice;
    };
   this.Consume.cache = { };
  })();
  
  Consume();
  

  B) No Leak: Remove "this"

  
  (function() {
    var backup = this.Consume;
    Consume = function(tagName) { //<-- remove "this"
      Consume.cache = document.createElement('div');
      Consume.cache._extendedByPrototype = Prototype.emptyFunction;
    };
   this.Consume.cache = { };
  })();
  
  Consume();
  

  
  In both cases I am not sure why it would avoid a memory leak.
  I am not seeing the circular reference.
  
  In "A" assigning a method (not in the scope chain) shouldn't cause an issue.
  The "this" of the method should point to the "element" and not the "window" global scope.
  
  In "B" removing "this.Consume =" and replacing with "Consume =" still keeps the global scope ie:
  Consume = function() { alert(this.location.href) }; // alerts correctly <- global scope by default
  this.Consume = function() { alert(this.location.href) }; // alerts correctly <- "this" is global scope
  

• 

  Tobie Langel October 5th, 2008 @ 12:46 PM

  • → Milestone changed from “” to “1.6.1”
  • → State changed from “new” to “bug”
• 

  Michal October 14th, 2008 @ 04:31 PM

  Leaks is present because new Element() caches created elements into the Element.cache object. However, the cache is not cleaned before page unload, and that is why I am experiencing memory leak (IE6, Windows XP, JavaScript Memory Leak Detector add-in).

  Proposed fix

  Clean Element.cache object on page unload. Modify destroyCache() function to clean the cache:

  
    function destroyCache() {
      for (var id in cache)
        for (var eventName in cache[id])
          cache[id][eventName] = null;
      for(var id in Element.cache)
        Element.cache[id] = null;
    }
  

  • I Added two last lines into function.
• 

  John-David Dalton October 14th, 2008 @ 04:31 PM

  I don't know if its a real problem if it's not impacting the memory footprint. The JScript Leaks Detector does some funny things to the document and event objects.

  For example:

  
  document.test = function() {
    alert(this === document);
  };
  
  // when leak detector is enabled it will return false instead of true
  document.test();
  

• 

  Andrew Dupont October 16th, 2008 @ 10:56 PM

  Nonetheless, we need to consolidate and minimize all the element creation we do inside the library — and make sure they all get cleaned up on page unload.

• 

  ykphuah November 4th, 2008 @ 08:39 AM

  I prefer the tool in http://home.wanadoo.nl/jsrosman/ when it comes to debugging memory leaks, the IE plugin just serves as an extra tool to show the element in question.

  Sieve seems to think that your Default2.html test program does not leak!

• 

  ykphuah November 4th, 2008 @ 10:14 AM

  Having said the above, I went on to do more testing, and to my amusement, just calling document.createElement will make IE's memory usage shoot up like nobody's business, one hour later, my IE usage is already 700 Meg!

  • testnewelement.html 442 Bytes
• 

  John-David Dalton November 4th, 2008 @ 03:38 PM

  I am not getting the memory increase on IE6 when using document.createElement.