Memory Leak In Javascript from shared closures

TonyStarkissi · August 1, 2019, 6:54pm

Iam new to javascript and i found an article about memory and a specific example .article

In the example says that :

The important thing is that once a scope is created for closures that
are in the same parent scope, that scope is shared. In this case, the
scope created for the closure someMethod is shared by unused

I changed it to the fixed version but the same happens. The someMethod is not in the same parent scope, but the memory leak remains. What am i missing?

    var theThing = null;
    var replaceThing = function () {
      var originalThing = theThing;
      var unused = function () {
        if (originalThing)
          console.log("hi");
      };
      theThing = {
        longStr: new Array(1000000).join('*'),
        someMethod: function () {
          console.log(someMessage);
        }
      };
    };
    setInterval(replaceThing, 1000);


Fixed Version:


    var theThing = null;
    var replaceThing = function () {
      var originalThing = theThing;
      var unused = function () {
        if (originalThing)
          console.log("hi");
      };
      (function(){
      theThing = {
        longStr: new Array(1000000).join('*'),
        someMethod: function () {
          console.log(someMessage);
        }
      };
    })()
    };
    setInterval(replaceThing, 1000);

snigo · August 1, 2019, 8:09pm

Hi @TonyStarkissi, your code has one memory leak - unreferenced setInterval. Other than that I see no closures and no memory leaks. If you want to introduce closure you need to declare someMessage variable inside replaceThing function

TonyStarkissi · August 1, 2019, 9:18pm

Check the example in here https://auth0.com/blog/four-types-of-leaks-in-your-javascript-code-and-how-to-get-rid-of-them/#Leaks-in-JavaScript

snigo · August 2, 2019, 2:40am

Hmm… OK, so apparently JS encloses someMethod and originalThing variable based on the fact that the variable is ‘used in higher scope’ and doesn’t bother to check whether someMethod is that higher scope or not… Well, that’s not the weirdest thing JS does and I’m pretty sure they have concrete explanation why. Anyway @TonyStarkissi, thanks for this - everyday you know something new

I would definitely refactor example code, as there are too many unrelated to problem stuff that confuses people, just present it like this if you’re thinking about writing a Medium article:

const a = {};
const addMethod = () => {
  const b = null;
  const unused = () => b;
  a.f = () => 'This will create unnecessary closure!';
};
addMethod(); // Closure created and JS just has got a little weirder...

If anyone knows why this is happening, feel free to share the knowledge

husseyexplores · August 2, 2019, 4:56am

An interesting read indeed!

So to answer your question regarding your fixed version, it basically does nothing different. You can say that it is the exact same as your ‘unfixed’ version, hence no change in garbage collection.

In this example, the key problem that was explained in the article was that unused has a reference to theThing.
I’ll quote it here again from the blog post:

At the same time, the variable unused holds a closure that has a reference to originalThing ( theThing from the previous call to replaceThing )

That’s not obvious at a first glance, because clearly, the unused function uses originalThing, not theThing.

Answer to this is references.

In Javascript, if you assign an object to a variable, a new copy of the object will not be created, but a reference to the object is stored.
For example:

// Make a simple person object
var personOne = {
  name: 'Alex',
  age: 45
};

// Assign it to different variable
var personTwo = personOne;

At this point, we have created an object and then assigned it to another variable.
If you console.log both variables, you’ll get the same output.

Now we will change the name of personTwo:

// Just changing the name, nothing much
personTwo.name = 'Mark'

Now, if you console.log, what do you expect the output? If you’re not 100% sure, please first try logging this in your browser developer console to see this in action and teach this yourself.

Here is the spoiler:

console.log(personTwo)
// Output: { name: 'Mark', age: 45 }

// The fun part!
console.log(personeOne)
// Output: { name: 'Mark', age: 45 }

You may ask… but why? We didn’t change the personOne’s name.
We did actually. Again, because of reference.
Here’s the breakdown:

First we created a brand new object and stored in a variable.
Then we made a new REFERENCE to that object in a variable.
Note that, we did not create a new object. We only created a new variable that points to the same object in memory.
Then we changed the name of that object using the second variable. (Again, we only had one object and we changed it its property)
Then we logged it, and sure enough, that object’s name property was changed in the memory, that’s why it reflected on both variable references, we though we changed on only one variable.

Now back to your question. Here is the breakdown of the unfixed/fixed version (they’re same):
The very first time replaceThing is called, these things happen:

originalThing becomes null as well, since theThing is null
unused function uses originalThing which is null.
we create a new object and assign it to theThing

The second time it runs, these things happen:

We assign originalThing to theThing and now originalThing is a reference to the same object as theThing is.
unused function uses originalThing, again, which is an existing object which also contains someMethod as well.
we create a new object and assign it to theThing.
(The previous object is still stored in originalThing though, it is still in use. It will be reassigned on next setInterval execution)

And the same thing goes on and on.
(And in your ‘fixed’ version, it does nothing, except assigning theThing to a new object, which we were already doing previously. It is also IIFE so it is not adding any further memory leaks)

So hopefully, now you can see how someMethod is still in scope in unused function and is causing preventing the garbage collector free the memory.

I’d say that read the blog post again, it explains in a better way than I could. I only tried to explain why your fixed version doesn’t do anything.
And also be sure to read the blog post on the Meteor page, which is also linked in the above article. It goes into way more detailed explanation of the exact problem and using the same example.

A side note: Google for Javascript primitive values vs reference values etc and you’ll find a bunch of resources that cover this topic in detail.
And also, get to know which types are references and which are values so you’ll also be able to spot these things in the future.

snigo · August 2, 2019, 10:48am

In my example const b doesn’t have reference to const a but still closure is created, so it looks like assigning originalThing to theThing was made exclusively to present an excessive memory leak and has nothing to do with the problem. If you want you can assign huge string to the b in my example and despite the fact a never touches b you’ll still have the same leak going on.

husseyexplores · August 2, 2019, 12:48pm

I’m not 100% sure about this tbh, but the article pointed out the memory leak because of a closure. I just tried the original example and your example in the chrome dev tools.
To keep the results similar, I used This will create unnecessary closure! string in the original example as well intead of new Array.join('*')

The original example shows this after 2 mins:

(A bunch of someMethod()s in the closure memory heap)

And your example shows this after two mins (running it in a setInterval for two mins, not just one function call)

(addMethod() is inside closure dropdown just like in the first screenshot. I’ve scrolled down here to see the function in the list)

It only shows only one call in in closure dropdown, as I think the rest of them are garbage collected. Your example doesn’t have closure memory leak at least, as the test shows. You may give this a try yourself and let us know your thoughts.

snigo · August 2, 2019, 6:22pm

Yes sure, I understand. What I meant by memory leak is anything that unnecessarily referenced or enclosed and therefore cannot be garbage-collected. In my example enclosed variable will be rewritten with every invoke, aka classic case. What they showed in article is a catastrophic case and indeed for this case you need to const b = a;. My point was to demonstrate that variable will be enclosed even without step.