6

I was reading this article ( http://www.ibm.com/developerworks/web/library/wa-memleak/ ) on IBM's website about memory leaks in JavaScript when I came across a memory leak that didn't quite look liked it leaked:

<html>
<body>
<script type="text/javascript">
document.write("Program to illustrate memory leak via closure");
window.onload=function outerFunction(){
   var obj = document.getElementById("element");
   obj.onclick=function innerFunction(){
      alert("Hi! I will leak");
   };
   obj.bigString=new Array(1000).join(new Array(2000).join("XXXXX"));
   // This is used to make the leak significant
};
</script>
<button id="element">Click Me</button>
</body>
</html>

I understood all the other leaks but this one stood out. It says there's a circular reference between the DOM and JavaScript object but I don't see it.

Can anyone shed some light on this?

EDIT: The link seems to have been taken down (I even refreshed the page I had up and it was down). Here's Google's cache (for as long as that lasts: http://webcache.googleusercontent.com/search?q=cache:kLR-FJUeKv0J:www.ibm.com/developerworks/web/library/wa-memleak/+memory+management+in+javascript&cd=1&hl=en&ct=clnk&gl=us&client=firefox-a )

Adam
  • 3,063
  • 5
  • 35
  • 49
  • It's not just you. I dont see a circular reference here either. – Tejs Sep 22 '11 at 17:57
  • 1
    Your link doesn't work? Your DOM object `obj` references `innerFunction`. `innerFunction`, in turn, has a reference to the obj through the closure. You should be able to avoid the memory leak by setting obj.onclick to null onunload. – Ruan Mendes Sep 22 '11 at 18:07
  • Thanks for pointing that out Nico and Juan. I looks like the site was taken down within the last 20 minutes. I even refreshed the tab I had open from before and it was down. I posted the Google Cache version for what it's worth. Sorry about that. – Adam Sep 22 '11 at 18:11
  • 1
    See my answer at http://stackoverflow.com/questions/1077840/javascript-memory-leaks-after-unloading-a-web-page/1886359#1886359 combined with my comment above – Ruan Mendes Sep 22 '11 at 18:23
  • Awesome, thanks Juan! It looks like the link is back up now, btw. – Adam Sep 22 '11 at 18:35

2 Answers2

6

The assignment to onclick of the innerFunction creates a function closure that preserves the value of the variables that are in scope of innerFunction. This allows the code in innerFunction to reference variables above it and is a desirable feature of javascript (something some other languages don't have).

Those variables that are in scope of innerFunction include obj. So, as long as there is a reference to this closure, the variables in that closure are preserved. It's a one-time piece of memory usage so it doesn't accumulate over time and thus isn't usually significant. But, if you put big data into one of those variables, then and expected it to be freed, then "yes" you would be using more browser memory than you expected.

Where this can cause problems is in this particular example, you have a circular reference between JS <==> DOM. In JS, you have preserved (in the function closure) a reference to the DOM object in the obj variable. In the DOM object, you have preserved a reference to the JS code and the function closure with the assignment to the onclick attribute. Some older browsers are dumb enough that even if you remove the "element" object from the DOM, the circular reference will keep the garbage collector from ever freeing the memory. This is not a problem in modern browsers as they are smart enough to see this circular reference and still free the object if there are no outside references to it.

In your particular code, you haven't actually created a leak because the element is still in the DOM. bigString is just a big chunk of data you've attached to the DOM element and it will stay there until you remove that attribute or remove the DOM object. That's not a leak, that's just storage.

The only way this would become a leak in IE6 is if you did this:

<html>
<body>
<script type="text/javascript">
document.write("Program to illustrate memory leak via closure");
window.onload=function outerFunction(){
   var obj = document.getElementById("element");
   obj.onclick=function innerFunction(){
      alert("Hi! I will leak");
   };
   obj.bigString=new Array(1000).join(new Array(2000).join("XXXXX"));
   // This is used to make the leak significant

};

// called later in your code
function freeObject() {
   var obj = document.getElementById("element");
   obj.parentNode.removeChild(obj);  // remove object from DOM
}

</script>
<button id="element">Click Me</button>
</body>
</html>

Now, you've removed the object from the DOM (sometime later in your code) and probably expected all memory associated with it to be freed, but the circular reference keeps that from happening in IE6. You could work-around that by doing the following:

<html>
<body>
<script type="text/javascript">
document.write("Program to illustrate memory leak via closure");
window.onload=function outerFunction(){
   var obj = document.getElementById("element");
   obj.onclick=function innerFunction(){
      alert("Hi! I will leak");
   };
   obj.bigString=new Array(1000).join(new Array(2000).join("XXXXX"));
   // This is used to make the leak significant

};

// called later in your code
function freeObject() {
   var obj = document.getElementById("element");
   obj.onclick=null;                 // clear handler and closure reference
   obj.parentNode.removeChild(obj);  // remove object from DOM
}
</script>
<button id="element">Click Me</button>
</body>
</html>

or, if you don't need the obj reference in the closure, you could null the reference from the closure entirely with this:

<html>
<body>
<script type="text/javascript">
document.write("Program to illustrate memory leak via closure");
window.onload=function outerFunction(){
   var obj = document.getElementById("element");
   obj.onclick=function innerFunction(){
      alert("Hi! I will leak");
   };
   obj.bigString=new Array(1000).join(new Array(2000).join("XXXXX"));
   // This is used to make the leak significant
   obj = null;   // kills the circular reference, obj will be null if you access if from innerFunction()
};
</script>
<button id="element">Click Me</button>
</body>
</html>

In practice, this is only a meaningful issue in a few cases when the memory usage of a leak could be significant.

  1. If you have large chunks of data that you store as DOM attributes, then a single object leak could leak a large amount of data. That's usually solved by not storing large chunks of data on DOM objects. Store them in JS where you control the lifetime explicitly.
  2. If you have a long lived page with lots of javascript interaction and an operation that creates/destroys DOM objects can be done many, many times. For example, a slideshow that runs unattended may be creating/destroying DOM objects over and over and over again. Even small amounts of memory leakage per item could eventually add up and cause a problem. In a slideshow I wrote, I just made sure that I didn't put any custom attributes on these particular DOM objects that I was add/removing and that all event handlers were removed before removing the object from the DOM. This should assure that there could be no circular references to them.
  3. Any kind of DOM operation you're doing over and over in a big loop.
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • 1
    The workaround removed the click handler, and you lose functionality. The solution is to remove it on unload. You don't need to remove both references, just one of them. That is, you don't need to remove the element from the DOM – Ruan Mendes Sep 22 '11 at 18:10
  • 1
    The IBM site (before they took it down) mentioned that a workaround would be to set obj=null after setting everything. – Adam Sep 22 '11 at 18:13
  • The work-around removed the click handler only when I was removing the object from the DOM and intending for the object to be destroyed. If you want the object to stay around, then the memory associated with it should stay around - that is not a leak. – jfriend00 Sep 22 '11 at 18:14
  • @Adam - setting obj to null in your example above doesn't accomplish much with relation to bigString. You still have bigString as an attribute on your object and all that memory is still being used. It would remove the circular reference between JS<==>DOM so if you later removed obj from the DOM, then it would get freed appropriately. That could protect against that. But, the issue that I wanted to make clear is this is only a leak if you later remove the object from the DOM and expect it to be freed. – jfriend00 Sep 22 '11 at 18:16
  • @jfriend00: The code wants a click handler do to something. You are simply adding a handler and removing it immediately, that doesn't make sense. Like I mentioned before, the way to address the issue is to remove `obj.onclick` when the page is unloaded, so that you still get your alert when clicking the element, but the leak doesn't persist when you reload the page – Ruan Mendes Sep 22 '11 at 18:18
  • @Juan Mendes - I was just trying to illustrate later freeing the object. I've amended my example to make that more clear and added the obj=null option too. I understand your point. – jfriend00 Sep 22 '11 at 18:21
  • @jfriend00: `obj = null` does seem like it would break the cyclical reference and not remove the click handler. The real problem comes when the closure must access the object in question and you couldn't set it to null. In which case, you'd have to resort to the unload handler. – Ruan Mendes Sep 22 '11 at 18:28
  • Use `addEventListener` and don't bother about leaks. – katspaugh Sep 22 '11 at 18:38
  • @katspaugh - I'm under the impression that IE6 is dumb enough that attachEvent (which is what IE6 supports) can have the same issue. – jfriend00 Sep 22 '11 at 19:14
  • @katspaugh - when I read that article, it says that the only way to not have the memory leak is to NOT use a closure with attachEvent. It specifically shows a closure with attachEvent as still having a memory leak. Did I read it wrong? – jfriend00 Sep 22 '11 at 19:29
  • @jfriend00, yes, might be. The resolution to the bug they propose (http://support.microsoft.com/kb/830555) seems to be concerned mainly with not having the reference to the element in the handler. – katspaugh Sep 22 '11 at 19:30
  • @jfriend00, yes, you're right. Wasn't fast enough to type a new comment before you answered. – katspaugh Sep 22 '11 at 19:34
0

This leaks memory due to an IE6 bug.

obj references innerFunction from onclick, and innerFunction references obj because it captures obj from the outer scope.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • Is that only an IE6 bug? Also, would this still be a leak if obj was passed in as a parameter, rather than referencing the outer global? – vol7ron Sep 22 '11 at 18:14
  • @vol7ron: Yes. Passed as a parameter to what? `obj` _isn't_ an outer global; that's why it leaks. – SLaks Sep 22 '11 at 18:17
  • 1
    It's not an IE6 only bug, IE claims to have fixed the issue but it still lingers. It still uses a separate garbage collector for DOM and JS, which is the reason that cyclical references can't be detected. See http://stackoverflow.com/questions/1077840/javascript-memory-leaks-after-unloading-a-web-page/1886359#1886359 That has links that explain the issue and that IE still has the problem. @vol7ron: Yes, the leak would still be there.... This many not be true in IE9, I'm not sure – Ruan Mendes Sep 22 '11 at 18:22