19
function(foo, cb) {
  var bigObject = new BigObject();
  doFoo(foo, function(e) {
     if (e.type === bigObject.type) {
          cb();
          // bigObject = null;
     }
  });
}

The above example shows a classic, accidental (or maybe not) memory-leaking closure. The V8 garbage collector can't determine if it's safe to remove the bigObject because it's being used in the callback function which can be called several times.

One solution is to set bigObject to null when the job in the callback function is over. But if you are using many variables (imagine there is n variables like bigObject, and they are all used in callback) then cleaning this becomes an ugly problem.

My question is this: is there any other way to clean those used variables?

EDIT Here's another (real world) example: So I get application from mongodb and compare it to some other application. Callback from mongodb uses variable application that is defined out of that callback. After I get result from mongodb I return it also as a callback (because it is all async and I cant just write return ). So actually it can happen that i propagate callback all the way to the source...

function compareApplications(application, condition, callback) {

    var model = database.getModel('Application');
    model.find(condition, function (err, applicationFromMongo) {
        var result = (applicationFromMongo.applicationID == application.applicationID)
        callback(result)        
    }
}
ᴍᴇʜᴏᴠ
  • 4,804
  • 4
  • 44
  • 57
Ivan Longin
  • 3,207
  • 4
  • 33
  • 42
  • Let me ask you this - why is this a problem? The `change` handler is meant to be called several times. So how will you (or the GC) ever know when it's really the end of using `bigObject` unless you unbind the `change` event? You seem to want one instance of `bigObject` so that the handler can compare types. You instantiate it once, which reduces the load for every time the handler runs. If you want it to be cleaned up, instantiate it inside the handler every time, or expect it to "leak" memory because that's how it works. – Ian May 08 '13 at 13:47
  • How about using .one() instead of .on()? – frenchie May 08 '13 at 13:59
  • Please notice that I have changed the example. In my real world program I don't use .on. I pass callback function to another. – Ivan Longin May 08 '13 at 14:17
  • Thanks. I believe methods like `model.find` are supposed to automatically release the reference to your callback once the async operation completes. This small code snippet should not leak anything (assuming `model.find` doesn't have a bug in it). Are you sure it is actually leaking? You should do some heap analysis to see what objects are actually leaking and what is retaining a reference to them. – Brandon May 08 '13 at 15:08
  • 1
    What concerne me is that I can potentialy have chain of callbacks...for example business function calls data function which calls mongodb and all communication between these is in callbacks. Maybe callback from mongodb realy clean everything but what with these other callbacks in business and data layers. I suppose that GC only see callback and what is underneath him he doesn't look. I haven't yet run analysis for leaks but I can clearly see that my node.js workers are using too much rss(RAM) memory. – Ivan Longin May 08 '13 at 15:27
  • 1
    Please notice that I don't use jQuery and I don't bind or register to some event. I just pass callback function to another. I use NODE.JS ! – Ivan Longin May 08 '13 at 14:25
  • callback chains are typical. They almost always clean themselves up correctly after the callback is fired. They are usually pinned to memory by the lowest level call that has actually registered a callback with some I/O operation. Once that operation completes that lowest callback is released and once the callback chain finishes executing, all of the callbacks are released. You only get in trouble if someone along the chain decides to store a reference to the callback somewhere that is more durable than the async operation itself. With practice, you can spot code which does this. – Brandon May 08 '13 at 15:52
  • 1
    Yes but in every that kind of callback (in business or data layer) you can have a closure and are you really sure that they will clean up? ... listen to this example. In some other part of code I also have reading from database but in stream. I have stream.on(data){...} event and every time when I collect 1000 records I fire up callback for business layer (in my example in first post it will be this one : callback(result)) ... that implementation of callback function in business layer is actually a closure and it works perfectly fine. Why didn't GC cleaned up that closure after first callback? – Ivan Longin May 08 '13 at 20:12
  • And how can GC even know that that implementation of callback in business layer will only be called once? ... you can fire up callback as many time you want...so in my opinion it cannot clean variables like bigObject in example unless you explicitly clean it by yourself...correct me if I am wrong in some part of this kind of thinking... – Ivan Longin May 08 '13 at 20:16
  • If this is true, then there is bug in DB driver software. Function 'find' should clearly release callback after it is called, because it calls it only once. It doesn't make sense that reference to callback passed to 'find' is kept somewhere after it is called. – Marko Jul 06 '18 at 15:05
  • I don't have time to post an answer but you can use a [weakRef](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef#examples) – Flavien Volken Mar 16 '23 at 14:24

2 Answers2

2

If your callback function is only supposed to be called once, then you should unsubscribe after it is called. That will release your callback + closure to the GC. With your closure released, bigObject will also be free to be collected by the GC.

That's the best solution - as you noted, the GC doesn't magically know your callback will only be called once.

Brandon
  • 38,310
  • 8
  • 82
  • 87
  • 2
    Thanks for answering but can you give some example of unsubscribing from callback? – Ivan Longin May 08 '13 at 13:55
  • 1
    It depends on what mechanism you are using to subscribe the callback. You don't provide enough details on what `doFoo()` does. Most frameworks either provide a `undoFoo()` method, or `doFoo()` itself returns a method you can call to unsubscribe. If you were using jQuery, you'd subscribe with `$("...").on("change", cb);` and unsubscribe with `$("...").off("change", cb)`. In fact, they have a method for the exact situation where you only want to subscribe for 1 event: `$("...").one("change", cb)` which will automatically unsubscribe after a single call. – Brandon May 08 '13 at 14:07
  • 1
    Please notice that I have changed the example in first post. This is now more like my actual problem. I pass callback function to another function. I use node.js. Foo is just a example but the problem is same as in my real world functions. Callback function is using variables in scope and GC cannot clean it implicitly :( ... my rss memory goes up very quickly because I have lots of traffic. – Ivan Longin May 08 '13 at 14:13
  • But what is `doFoo`? Somewhere you are using some API someone else wrote that lets you register a callback to receive events. That API will also provide a way to _unregister_ your callback when you no longer wish to receive events. You need to use the API to unregister your change callback when you do not want it anymore. That will free your memory. If you only ever register callbacks but never unregister them, and you do this ALOT then yes you are going to have problems. Setting `bigObject` to null will just delay the inevitable by slowing down the speed at which you leak. – Brandon May 08 '13 at 14:17
  • 3
    The problem is that I don't know how to unregister from callback in this specific problem! As I said, I use node.js and I'm not binding to events like in jQuery or something like that, I just pass callback function to another function (doFoo in this example) and doFoo does some IO operation for example and then in some point call callback function that I passed. – Ivan Longin May 08 '13 at 14:23
  • 1
    Node.Js does not have a function called `doFoo`. If you do not want to give us a specific example then all I can give is general advice, which is basically RTFM. Post a specific example using a specific node.js function and you'll get specific advice. – Brandon May 08 '13 at 14:30
  • 1
    I've added some real world example in first post. – Ivan Longin May 08 '13 at 14:39
0

To build on Brandon's answer: If (for some terrible reason) you are unable to unsubscribe your callback you could always handle deleting the callback yourself:

function createSingleUseCallback(callback)
{
    function callbackWrapper()
    {
        var ret = callback.apply(this, arguments);
        delete callback;
        return ret;
    }
    return callbackWrapper;
}

function compareApplications(application, condition, callback)
{
    var model = database.getModel('Application');
    model.find(condition, createSingleUseCallback(function (err, applicationFromMongo)
    {
        var result = (applicationFromMongo.applicationID == application.applicationID);
        callback(result);
    })
}
JosiahDaniels
  • 2,411
  • 20
  • 37