2
\$\begingroup\$

I built my first long running AJAX heavy app and memory leaks are eating me alive. I don't want to post too much code because it should be my job to fix it all, but I need some help.

Does this code leak? Where? Why? and if you'd be so kind as to suggest a fix.

function list_stuff(data, type, where, order) {
    var items = [];
    $.each(data, function (i, item) {
        items.push(
        //using jquery $data might make this look more readable
        '<li data-lat="' + item.lat + '" data-lng="' + item.lng + '" data-type="' + type + '" data-id="' + item.group_id + '" data-privacy="' + item.privacy + '" type = "li"> ' + '<div class ="li_title">' + item.name + '</div>' + '<div class ="li_description">' + item.description + '</div>' + '<div class ="li_footer">' + 'Miles: ' + item.distance + '  People: ' + item.people);

        if (item.privacy != 0) {
            items.push('  (Private group) ');
        }
        if (where == '#thelist') {
            items.push('</div>' + '<button data-action ="2" type ="button" class ="join_button" onclick="join_group(this)">Join</div></li>');
        } else if (where == '#thelist2') {
            items.push('</div>' + '<button data-action= "4" type ="button" class ="join_button" onclick="join_group(this)">Leave</div></li>');
        }
    });

    if (order === 'append') {
        $(where).append(items.join(''));
    } else {
        $(where).prepend(items.join(''));
    }

    scroll.refresh();
}

My AJAX looks like this:

function ajax(send,where) {
    $.ajax({
        url: where,
        type: 'POST',
        dataType: 'json',
        data: send,
        success: function(a) {
            if (a.success===1) {
                list_stuff(a.data, a.type, '#list1','append');
            } else {
                create_group_problem(a.msg);
            }
            if(a.msg.length >1 ) {
                msg_disp(a.msg);
            }
        },
        error: function(err){
            msg_disp('error');
        }
    });
}

I understand (tenuously) that JavaScript gets garbage collected, but doesn't this form a circular reference that may prevent it from being collected? Please correct me if I'm wrong, but here is my reasoning:

  1. list_stuff():

    '<button data-action ="2" type ="button" class ="join_button" onclick="join_group(this)">Join</div></li>'
    

    JS -> DOM

  2. Generated LI Element:

    onclick="join_group(this)"
    

    DOM ->JS

Last question for the sake of learning/understanding: if this is not a circular reference, that will be prevented from being garbage collected, how can I change it to become one?

function join_group(block) {
    var $block = $(block);    
    var pass   = 0;
    var c_id   = $block.closest("li").data("id");
    var p      = $block.closest("li").data("privacy");
    var action = $block.data("action");

    if(p !== 0 && action ==3){
        pass = prompt("This group requires a password to join.");
        if (pass === null || pass ==='') {
            return;
        }
    }

    var send ={
        group_id: c_id,
        group_action: action,
        privacy:p,
        password: pass,
    };

    ajax(send, 'group');
}
\$\endgroup\$
1
\$\begingroup\$

First of all, to answer your questions:

  1. I don't see anything in the code you've posted which would cause a "memory leak".
  2. Your reference to join_group inside of your HTML is within a string, and so does not create a reference in the JavaScript code. The browser will create a reference to that function when it is parsed. If you really want to create a reference immediately, you could, after your call to prepend or append, hook up the event handlers using jQuery with something like:

    $('.join_button').on('click', join_group);
    

    If you do that, you'll need to change the start of join_group to:

    function join_group() {
        var $block = $(this);
    

    You'll want to remove the onclick="join_group(this)" from the HTML as well.

On a different note, you can speed up your code by not concatenating strings you're pushing into an array and joining anyway. In other words, replace all of the +s inside a push() with a ,. You won't see much of a speed difference though.

\$\endgroup\$
1
\$\begingroup\$

javascript is garbage collected, unless you're making huge(tens of MB of XML/JSON) data transfers every couple of seconds and keep references to that data, there should be "no memory leak" unless the browser is doing a lousy job.

I would put "items" array as global variable so that with each request, you overwrite it, in this way, old data will be discarded, i.e.

// moved globally
var items = [];
function list_stuff(data, type, where, order) 
{
  // try to parse data as JSON
  try 
  {
    items = jQuery.parseJSON(data);
    var index;
    var item;
    for ( index = 0; index < items.length; index++ )
    {
      item = items[index];
      // do something with "item"
    }
  }
  catch(e)
  {
    // handle error here
  }
}
\$\endgroup\$
  • \$\begingroup\$ I wouldn't put it global. What if the list_stuff is triggered again while it is still running? Bad idea! \$\endgroup\$ – ANeves thinks SE is evil Aug 13 '12 at 17:42
  • \$\begingroup\$ @ANeves well, javascript is not threaded, so, I fail to see the problem. \$\endgroup\$ – user10745 Aug 13 '12 at 18:15
  • \$\begingroup\$ I don't know if execution is interrupted, or if execution of the second call waits for the first to finish, or if they just run concurrently. Either way: if you just want to override it, why not simply setting it to null in the end? \$\endgroup\$ – ANeves thinks SE is evil Aug 14 '12 at 16:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.