10

I was reviewing the slides in this presentation: http://slid.es/gruizdevilla/memory

and on one of the slides, this code is presented with the suggestion that it creates a memory leak:

var buggyObject = {
   callAgain: function() {
     var ref = this;
     var val = setTimeout(function() {
        ref.callAgain();
     }, 1000);
   }
}

buggyObject.callAgain();
buggyObject = null;

Can someone explain the issue in more detail here? I might be missing some subtleties here.

Evan Davis
  • 35,493
  • 6
  • 50
  • 57
MedicineMan
  • 15,008
  • 32
  • 101
  • 146
  • Obviously, there will be a reference to `buggyObject` forever but I'm not sure if you would call that a memory leak in the traditional sense or not. They could be implying some sort of cycle since the closure references the timeout so each call would theoretically create a new long-lived closure, except that `val` is a handle not a reference. – Brian Nickel Aug 06 '13 at 19:12
  • This has me confused as well. I feel like I'm missing an important point and I'm not sure if I am. I guess the only point they are making is the very obvious one, that you'll have a piece of code indefinitely running and it will use memory enough for it to run (and keeping alive the original buggyObject in the process). But this seems super obvious. I thought they were trying to make some subtle point about an exponentially growing leak that would be created, but I don't see how. – Quinxy von Besiex Jan 07 '16 at 20:53
  • 1
    Can someone give a fixed version of this code? – Zlelik Jan 04 '21 at 21:10
  • @Zlelik a fixed version should just use setInterval, without an object or maintaining the reference to the object. – WillOw Aug 03 '23 at 18:50

3 Answers3

10

This is definitely a memory leak. However, the memory consumption is so small and cannot be measured. I did some small changes to the source code.

  • I put the entire code inside a loop to create the same scenario 100,000 times
  • I increased the timer interval to about 16 minutes. This prevents browser from crashing

Here is the code:

for (var i = 0; i < 100000; i++) {
    var buggyObject = {
       callAgain: function() {
         var ref = this;
         var val = setTimeout(function() {
            ref.callAgain(); 
         }, 1000000); //16m
       }
    }

    buggyObject.callAgain();
    buggyObject = null;
}

Memory leak in Chrome

My experiment:

I ran the code in Chrome Version 34.0.1847.116 m and captured memory changes using Developer Tools\Timeline.

As we can see in the picture, around 32 MB of memory has been consumed for running this code and after a while it's been decreased to about 30 MB and stayed unchanged (see #1). After several garbage collecting attempts by Chrome (see #2) and one manual forced garbage collection by me (see #3, #4), the memory consumption remains unchanged. There is no more buggyObject and there is nothing we can do to release the memory. The only possible way is to close the browser.

What causes this?

The main reason for this behavior is the timer. Timer callback and its tied object, buggyObject will not be released until the timeout happens. In our case timer resets itself and runs forever and therefore its memory space will never be collected even if there is no reference to the original object.

Hugo
  • 27,885
  • 8
  • 82
  • 98
advncd
  • 3,787
  • 1
  • 25
  • 31
8

There is another question that describes how setTimeout() looks like it has memory leaks, but in reality does not.

However, I think what the author is trying to say is that since buggyObject creates a setTimeout which calls itself, even if you change buggyObject to equal null (saying you are done with the object and it can be cleaned up), the object won't be garbage collected because there is still a reference to it in the setTimeout(). This is technically a memory leak because there is no longer any direct reference to the setTimeout function so that you could clear it later (kind of a zombie timeout if you will).

Community
  • 1
  • 1
Steven Lambert
  • 5,571
  • 2
  • 29
  • 46
2

As advncd pointed out, the timer gets executed and adds even more data on the stack. There is a conceptual view of what happens:

var a = 123;
// call the setTimeout.function
  var a = 123;
  // call the setTimeout.function
    var a = 123;
    // call the setTimeout.function
      var a = 123;
      // call the setTimeout.function
        var a = 123;
        // call the setTimeout.function
          var a = 123;
          // call the setTimeout.function
            var a = 123;
            // call the setTimeout.function
              var a = 123;
              // call the setTimeout.function
                var a = 123;
                ...etc...

So each time a new variable a is allocated on the stack which grows forever.

What advncd did not mention, though, is the fact that you have a setInterval() function to do what you need to do: have the same function called over and over again. Now you still have a "memory leak" but only the initialization parameters leak (i.e. it won't grow each time the timer times out.)

So conceptually, the calls are flat and you avoid the leak:

a = 123;
// call the setTimeout.function
// call the setTimeout.function
// call the setTimeout.function
// call the setTimeout.function
// call the setTimeout.function
// call the setTimeout.function
// call the setTimeout.function
...etc...
Alexis Wilke
  • 19,179
  • 10
  • 84
  • 156