6 ms·

The Javascript memory leak we all do

92 points by ra 15y ago

nkassis 15y ago
I don't really get the difference, in both cases the a,b will be stored somewhere no? I mean, when you return that function it references a and b as the original function would have and you've done absolutely nothing different. Can someone explain this a little better?
- neilk 15y ago
  The argument is that because in the first case, the closure is created in a scope where element, a, and b all exist. So element is available in the closure even if it's never used. Since now element has a property which closes over element, he argues this is a circular reference, which will eventually become a memory leak. It seems to me that a smart JS engine could avoid this by noticing that there's no possibility of using element within the first closure, but I'm not an expert. Also it seems like a small price to pay unless you are constantly creating and destroying such widgets; surely anything associated with an unloaded page can be GC'ed regardless of circular references?
  - deleted 15y ago
    [deleted]
  - victorNicollet 15y ago
    And all of it stems from the fact that, JavaScript being a dynamic language, there's no way to statically determine whether the function will use 'element' or not (it could always eval() something evil and sneaky). Most functional languages would avoid packing 'element' at compile time, and I suspect that modern JavaScript compilers also handle this optimization whenever they can afford it.
  - masklinn 15y ago
    > It seems to me that a smart JS engine could avoid this by noticing that there's no possibility of using element within the first closure, but I'm not an expert. They don't even need that. Any garbage collection strategy (including refcounting) have no trouble reclaiming cycle (the only GC type which may have problems are refcounting GCs, and they usually have a cycle detector for exactly that purpose). The only way for this issue to arise is to involve two runtimes, each with its own garbage collector, able to create references between one another. A cycle crossing the boundary will not be detectable by either GC, and will lead to a leak. This is exactly what happens in Internet Explorer up to (and including) version 7: JavaScript lives in the jscript engine and the DOM lives in COM, each has its own GC, and they can have references to objects living in the other runtime.
  - keturn 15y ago
    I've found that when I use the webkit inspector, those unused values don't show in the Closure section of the Scope Variables display, and if I put a breakpoint in there and try to reference the unused variables I get a Reference Error.
wzdd 15y ago
I'm not a JavaScript expert, and so I didn't understand this bit: > create a circular reference and thus, a memory leak It was confusing because JavaScript uses mark-and-sweep collection, which can handle circular references just fine. A bit of googling turns up this link, which explains it: http://www.ibm.com/developerworks/web/library/wa-memleak/ http://www.ibm.com/developerworks/web/library/wa-memleak/ The problem is that the DOM is reference counted (at least in some browsers, at least back when that article was written). So, you can get memory leaks in cases where JS intersects with the DOM.
- stanleydrew 15y ago
  There is a better explanation at the very end of this awesome description of javascript's closures and scope resolution: http://jibbering.com/faq/notes/closures/ http://jibbering.com/faq/notes/closures/
- masklinn 15y ago
  > so I didn't understand this bit: That's because it's wrong. > It was confusing because JavaScript uses mark-and-sweep collection, which can handle circular references just fine. I don't think that's mandated, you could probably implement a refcounted JS interpreter. > The problem is that the DOM is reference counted (at least in some browsers, at least back when that article was written). So, you can get memory leaks in cases where JS intersects with the DOM. And the main culprit was, as is usually the case, Internet Explorer.
rst 15y ago
The OP is confused --- a proper garbage collector will collect cyclic structures. The situation that he describes was a problem in versions of IE prior to ie8 because of limitations on its garbage collector --- specifically that it was unable to collect cycles if the cycle involved a DOM element. This is fixed in ie8 (and, presumably, subsequent versions), and even in ie7 the situation gets cleaned up when the user navigates away from the page (although JS that creates a lot of "throwaway" DOM elements could still get into trouble). So, most programmers really have to worry about this only if still targeting ie6. (By the way, if you are afflicted with ie6, the jQuery 'data' APIs work around this misbehavior; other libraries often have similar tricks.) The official scoop from Microsoft: http://msdn.microsoft.com/en-us/library/dd361842%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/dd361842%28v=vs.85%2...
- victorNicollet 15y ago
  Indeed. Last I checked, jQuery breaks the cycle when elements are removed from the DOM by going through all their event handlers and data members and dropping them.
  - bentruyman 15y ago
    I don't think jQuery is necessarily relevant in this discussion. This post is talking about JavaScript closures in general, regardless of whether or not it's DOM events or plain old callbacks.
- spjwebster 15y ago
  Indeed. Also, a slightly more succinct fix for IE6/7 would be: function foo(element, a, b) { element.onclick = function() { /* uses a and b */ }; element = null; // avoid memory leak } The closure contains a reference to foo's activation object, not to its properties. As a consequence, you can clear out the element reference after the event handler has been defined and this will break the circular reference. The above technique is also useful when creating anonymous functions for other purposes when element references happen to be lying around. Depending on how long these anonymous functions are kept alive and what you do with the elements in the meantime, this would normally lead to a memory leak (due to the dangling reference to the DOM element, rather than a circular reference as described in the article). See http://jibbering.com/faq/notes/closures/ http://jibbering.com/faq/notes/closures/ for a seriously nerdy deep-dive into closures in JavaScript.
wccrawford 15y ago
Has this actually been tested? Does it really create a leak, and does the second code really eliminate it? And with the way Chrome handles tabs (in separate processes) does it really matter, since closing the tabs releases the memory?
- masklinn 15y ago
  > Has this actually been tested? Does it really create a leak, and does the second code really eliminate it? it does not create a leak in any modern browser. The only older browser in which it does is Internet Explorer, up to (and including) version 7.
stanleydrew 15y ago
See the internet explorer memory leak section at the end of this article for a full explanation: http://jibbering.com/faq/notes/closures/ http://jibbering.com/faq/notes/closures/
theallan 15y ago
Just a quick point, but rather than using the DOM0 interface, use the DOM2 addEventListener method.
philwise 15y ago
Using jquery will avoid this issue, because it automatically removes event handlers when a node is taken out of the DOM tree.
mjijackson 15y ago
This is a well-known issue that we shouldn't still be discussing in 2011. Any sane JavaScript implementation already addresses this issue, and any responsible JavaScript framework will address this issue for you for the sake of older browsers (e.g. see the jQuery source at https://github.com/jquery/jquery/blob/master/src/event.js#L141 https://github.com/jquery/jquery/blob/master/src/event.js#L1...). To say that we all do this is more than a bit of an exaggeration.
- spjwebster 15y ago
  It's definitely a stretch to say we all run into these issues, but you'd be surprised how many beginner and intermediate-level developers don't know about this issue precisely because they've been insulated from it by frameworks. There are plenty of situations where developers can find themselves dealing with insane JavaScript implementations and irresponsible JavaScript frameworks. While the article doesn't explain exactly what the issue is and which browsers it affects (leading me to believe the author himself doesn't truly understand these memory leaks), the recommendation given is a sound one.
  - masklinn 15y ago
    > It's definitely a stretch to say we all run into these issues, but you'd be surprised how many beginner and intermediate-level developers don't know about this issue precisely because they've been insulated from it by frameworks. Or by most browsers not being broken anymore. > which browsers it affects Internet Explorer up to version 7.
jrockway 15y ago
So the issue is only with DOM elements in IE 6, right? If I write: function foo(huge) { some_global_thing.call_later = function() { 42 } } And call foo with "huge", huge is not saved in call_later's closure, right? And it doesn't change if I do: function foo(huge, x){ blah_blah_blah.whatever = function() { x + 1 }; } We're still only closing over "x", right?
- Twisol 15y ago
  Unfortunately, eval() complicates things. Consider: function foo (x) { return function(what) { return eval(what); }; } var theClosure = foo(42); var theX = theClosure("x"); document.write(theX); // displays 42 Even though "x" is never explicitly used, it's still accessible through the eval'd expression.

92 points by ra 15y ago

I don't really get the difference, in both cases the a,b will be stored somewhere no? I mean, when you return that function it references a and b as the original function would have and you've done absolutely nothing different. Can someone explain this a little better?

neilk 15y ago
The argument is that because in the first case, the closure is created in a scope where element, a, and b all exist. So element is available in the closure even if it's never used. Since now element has a property which closes over element, he argues this is a circular reference, which will eventually become a memory leak. It seems to me that a smart JS engine could avoid this by noticing that there's no possibility of using element within the first closure, but I'm not an expert. Also it seems like a small price to pay unless you are constantly creating and destroying such widgets; surely anything associated with an unloaded page can be GC'ed regardless of circular references?
- deleted 15y ago
  [deleted]
- victorNicollet 15y ago
  And all of it stems from the fact that, JavaScript being a dynamic language, there's no way to statically determine whether the function will use 'element' or not (it could always eval() something evil and sneaky). Most functional languages would avoid packing 'element' at compile time, and I suspect that modern JavaScript compilers also handle this optimization whenever they can afford it.
- masklinn 15y ago
  > It seems to me that a smart JS engine could avoid this by noticing that there's no possibility of using element within the first closure, but I'm not an expert. They don't even need that. Any garbage collection strategy (including refcounting) have no trouble reclaiming cycle (the only GC type which may have problems are refcounting GCs, and they usually have a cycle detector for exactly that purpose). The only way for this issue to arise is to involve two runtimes, each with its own garbage collector, able to create references between one another. A cycle crossing the boundary will not be detectable by either GC, and will lead to a leak. This is exactly what happens in Internet Explorer up to (and including) version 7: JavaScript lives in the jscript engine and the DOM lives in COM, each has its own GC, and they can have references to objects living in the other runtime.
- keturn 15y ago
  I've found that when I use the webkit inspector, those unused values don't show in the Closure section of the Scope Variables display, and if I put a breakpoint in there and try to reference the unused variables I get a Reference Error.

wzdd 15y ago

I'm not a JavaScript expert, and so I didn't understand this bit: > create a circular reference and thus, a memory leak It was confusing because JavaScript uses mark-and-sweep collection, which can handle circular references just fine. A bit of googling turns up this link, which explains it: http://www.ibm.com/developerworks/web/library/wa-memleak/ http://www.ibm.com/developerworks/web/library/wa-memleak/ The problem is that the DOM is reference counted (at least in some browsers, at least back when that article was written). So, you can get memory leaks in cases where JS intersects with the DOM.

stanleydrew 15y ago
There is a better explanation at the very end of this awesome description of javascript's closures and scope resolution: http://jibbering.com/faq/notes/closures/ http://jibbering.com/faq/notes/closures/
masklinn 15y ago
> so I didn't understand this bit: That's because it's wrong. > It was confusing because JavaScript uses mark-and-sweep collection, which can handle circular references just fine. I don't think that's mandated, you could probably implement a refcounted JS interpreter. > The problem is that the DOM is reference counted (at least in some browsers, at least back when that article was written). So, you can get memory leaks in cases where JS intersects with the DOM. And the main culprit was, as is usually the case, Internet Explorer.

rst 15y ago

The OP is confused --- a proper garbage collector will collect cyclic structures. The situation that he describes was a problem in versions of IE prior to ie8 because of limitations on its garbage collector --- specifically that it was unable to collect cycles if the cycle involved a DOM element. This is fixed in ie8 (and, presumably, subsequent versions), and even in ie7 the situation gets cleaned up when the user navigates away from the page (although JS that creates a lot of "throwaway" DOM elements could still get into trouble). So, most programmers really have to worry about this only if still targeting ie6. (By the way, if you are afflicted with ie6, the jQuery 'data' APIs work around this misbehavior; other libraries often have similar tricks.) The official scoop from Microsoft: http://msdn.microsoft.com/en-us/library/dd361842%28v=vs.85%29.aspx http://msdn.microsoft.com/en-us/library/dd361842%28v=vs.85%2...

victorNicollet 15y ago
Indeed. Last I checked, jQuery breaks the cycle when elements are removed from the DOM by going through all their event handlers and data members and dropping them.
- bentruyman 15y ago
  I don't think jQuery is necessarily relevant in this discussion. This post is talking about JavaScript closures in general, regardless of whether or not it's DOM events or plain old callbacks.
spjwebster 15y ago
Indeed. Also, a slightly more succinct fix for IE6/7 would be: function foo(element, a, b) { element.onclick = function() { /* uses a and b */ }; element = null; // avoid memory leak } The closure contains a reference to foo's activation object, not to its properties. As a consequence, you can clear out the element reference after the event handler has been defined and this will break the circular reference. The above technique is also useful when creating anonymous functions for other purposes when element references happen to be lying around. Depending on how long these anonymous functions are kept alive and what you do with the elements in the meantime, this would normally lead to a memory leak (due to the dangling reference to the DOM element, rather than a circular reference as described in the article). See http://jibbering.com/faq/notes/closures/ http://jibbering.com/faq/notes/closures/ for a seriously nerdy deep-dive into closures in JavaScript.

wccrawford 15y ago

Has this actually been tested? Does it really create a leak, and does the second code really eliminate it? And with the way Chrome handles tabs (in separate processes) does it really matter, since closing the tabs releases the memory?

masklinn 15y ago
> Has this actually been tested? Does it really create a leak, and does the second code really eliminate it? it does not create a leak in any modern browser. The only older browser in which it does is Internet Explorer, up to (and including) version 7.

stanleydrew 15y ago

See the internet explorer memory leak section at the end of this article for a full explanation: http://jibbering.com/faq/notes/closures/ http://jibbering.com/faq/notes/closures/

theallan 15y ago

Just a quick point, but rather than using the DOM0 interface, use the DOM2 addEventListener method.

philwise 15y ago

Using jquery will avoid this issue, because it automatically removes event handlers when a node is taken out of the DOM tree.

mjijackson 15y ago

This is a well-known issue that we shouldn't still be discussing in 2011. Any sane JavaScript implementation already addresses this issue, and any responsible JavaScript framework will address this issue for you for the sake of older browsers (e.g. see the jQuery source at https://github.com/jquery/jquery/blob/master/src/event.js#L141 https://github.com/jquery/jquery/blob/master/src/event.js#L1...). To say that we all do this is more than a bit of an exaggeration.

spjwebster 15y ago
It's definitely a stretch to say we all run into these issues, but you'd be surprised how many beginner and intermediate-level developers don't know about this issue precisely because they've been insulated from it by frameworks. There are plenty of situations where developers can find themselves dealing with insane JavaScript implementations and irresponsible JavaScript frameworks. While the article doesn't explain exactly what the issue is and which browsers it affects (leading me to believe the author himself doesn't truly understand these memory leaks), the recommendation given is a sound one.
- masklinn 15y ago
  > It's definitely a stretch to say we all run into these issues, but you'd be surprised how many beginner and intermediate-level developers don't know about this issue precisely because they've been insulated from it by frameworks. Or by most browsers not being broken anymore. > which browsers it affects Internet Explorer up to version 7.

jrockway 15y ago

So the issue is only with DOM elements in IE 6, right? If I write: function foo(huge) { some_global_thing.call_later = function() { 42 } } And call foo with "huge", huge is not saved in call_later's closure, right? And it doesn't change if I do: function foo(huge, x){ blah_blah_blah.whatever = function() { x + 1 }; } We're still only closing over "x", right?

Twisol 15y ago
Unfortunately, eval() complicates things. Consider: function foo (x) { return function(what) { return eval(what); }; } var theClosure = foo(42); var theX = theClosure("x"); document.write(theX); // displays 42 Even though "x" is never explicitly used, it's still accessible through the eval'd expression.