CODEWITHSUNDEEP
javascriptjquery
jonagoldman
jonagoldman

Reputation: 8754

Memory leak when performing setTimeout and Ajax every x seconds

I get new data every x seconds using jQuery Ajax and setTimeout() and it results in a memory leak. The browser memory goes up and up on each Ajax call until the browser crashes.

$(document).ready(wp.state.init);

wp.state = {

    data: {},

    init: function () {
        $.PubSub('state:refresh').subscribe(function () {
            window.setTimeout(function () {
                wp.state.refresh();
            }, 1000);
        });

        this.refresh();
    },

    refresh: function () {

        $.ajax({
            url: 'http://url_here/',
            cache: false,
            dataType: "json",
            success: function (data) {
                wp.state.data = data;
                $.PubSub('state:refresh').publish();
            }
        });
    }
}

UPDATE (based on @dez answer)

wp.state = {

    data: {},

    init: function () {
        $.PubSub('state:refresh').subscribe(function () {
            window.setTimeout(wp.state.refresh, 1000);
        });

        this.getState();
    },

    refresh: function () {
        wp.state.getState();
    },

    onSuccess: function (data) {
        wp.state.data = data;
        $.PubSub('state:refresh').publish();
    },

    getState: function () {

        $.ajax({
            url: 'http://url_here/',
            cache: false,
            dataType: "json",
            success: this.onSuccess
        });
    }
}

Notes:

Upvotes: 4

Views: 1868

Answers (1)

dezfowler
dezfowler

Reputation: 1258

I can see a couple of issues here, some of them may be causing your memory leak.

For starters, the first line:

$(document).ready(wp.state.init);

This is not going to execute init in the correct scope and as a result the value of this will be undefined on the line:

this.refresh();

You should do something like this instead:

$(document).ready(function(){wp.state.init();});

or amend your this.refresh() to be explicitly wp.state.refresh().

The bits I think are likely to be causing a leak are...

Every time you make the $.ajax call in refresh you're create a new function to handle the success callback. You should define the callback as another function on your wp.state object instead and pass that in as success in the same manner as you pass wp.state.init to the ready() method currently.

Similarly every time you call window.setTimeout in the function subscribed to your PubSub you're creating a function to handle that callback too. You should do the same here and extract that out into a function on wp.state.

Upvotes: 1

Related Questions