CODEWITHSUNDEEP
javascriptjquery
Nik
Nik

Reputation: 7273

jQuery Memory Leak Suspicion

I am making an AJAX request for XML. I am doing this every second. I notice that my memory usage grows into the hundreds of megabytes. As you might imagine, the customer is not happy with this. After doing some reading in various places, I suspect that function closures are causing my headache. I am looking for any verification that can be had as well as any help for how to fix it.

function PageManager () {
    var self = this;

    self.timeoutHandler = function () {
        $.ajax ({
            url: 'URLtoXML',
            type: 'post',
            cache: false,
            context: self,
            success: function (data) {
                var slf = this;
                var xmlDoc = $($.parseXML (data));
                xmlDoc.find ("tag_label").each (function () {
                    self.val = parseInt ($.trim ($(this).text ()));
                }
                setTimeout (slf.timeoutHandler, 750);
            }
        });
    }
}

var pm = new PageManager ();
pm.timeoutHandler ();

EDIT I have incorporated some people's ideas and a little of the success handler internals. I see a smaller growth rate, but not by much.

Upvotes: 6

Views: 3051

Answers (4)

Tom Bell
Tom Bell

Reputation: 499

Your timeout function

setTimeout(self.timeoutHandler, 750);

may be causing memory leak. Although the success: callback is able to exit after finished, there may not be an empty context in a setTimeout callback causing a circular loop. Youl should take out the anonymous function to end up something like this:

var pManager = new PageManager ();
pManager.timeoutHandler();

function PageManager () {
var ret  = (function(self) { 
    return function() { self.timeoutHandler(); };
})(this);

this.timeoutHandler = function () {
    $.ajax ({
        url: '/echo/json/',
        type: 'post',
        cache: false,
        success: function (data) {
            setTimeout (ret, 750);
        }
    });
};
}

This should only call back once.

Upvotes: 0

J. Holmes
J. Holmes

Reputation: 18546

I'm just going to throw my two cents in but i agree jAndy (+1). However, I would generate a single bound callback once, rather than creating a new closure on ever iteration of the callback (which in theory could keep the scope and the xml data alive for a lot longer than you want. I would suggest another alternative like this: 

function PageManager () {
    var callback  = (function(self) { 
        return function() { self.timeoutHandler(); };
    })(this); // only one callback

    this.timeoutHandler = function () {
        $.ajax ({
            url: '/echo/json/',
            type: 'post',
            cache: false,
            data: {a:Math.random() },
            success: function (data) {
                //var xmlDoc = $($.parseXML (data));
                // Processing of XML
                //alert("data");
                setTimeout (callback, 750);
            }
        });
    };
}

var pm = new PageManager ();
pm.timeoutHandler();

Edit

Here is a jsFiddle with the above code, and I watched the memory management for a little while, but not nearly enough to be conclusive in any sense.

Upvotes: 0

Halcyon
Halcyon

Reputation: 57709

Memory grows when you keep references to things. It's a leak when you accidentally do this.

In this case your function timeoutHandler calls itself, and again, and again ... It can never clean up the call stack. I'll bet this is your leak.

To avoid this problem use setInterval. It works exactly like setTimeout but it will keep calling the function every milliseconds until you clear it with clearTimeout (or until the world ends).

The down side is that you can't time it very well, your current implementation always waits 750 milliseconds after each call. I suppose you could do something fancy that will still allow you time it well (using Date to check the time), but it's not something I could write in 10 seconds :P

Upvotes: 0

jAndy
jAndy

Reputation: 235962

To avoid that any newly created function (context) is closing over its parent scope here, you would just need to get rid of the anonymous function in setTimeout there. So

 setTimeout(self.timeoutHandler, 750);

However, even if that closure would clouse over the parent(s) contexts, any half decent garbage collector (like any modern browser have) will notice it and free the memory after the method has fired. The very important thing you didn't mention is on which browser you noticed the behavior. The Firefox garbage collector for instance, works .. pretty unpretictable (at least to me). It'll allow for more and more memory to get used and then on some point, it will release a huge chunk again.

To see what is going on, use Firefox and have a look into about:memory while your script is running. There you will see where the memory goes. I would not be worried if the memory usage increases for a while. Look at it, if that is all of your code, the memory should get freed sooner or later.

Upvotes: 6

Related Questions