Memory leak involving jQuery Ajax requests

2019-01-25 14:05发布

问题:

I have a webpage that's leaking memory in both IE8 and Firefox; the memory usage displayed in the Windows Process Explorer just keeps growing over time.

The following page requests the "unplanned.json" url, which is a static file that never changes (though I do set my Cache-control HTTP header to no-cache to make sure that the Ajax request always goes through). When it gets the results, it clears out an HTML table, loops over the json array it got back from the server, and dynamically adds a row to an HTML table for each entry in the array. Then it waits 2 seconds and repeats this process.

Here's the entire webpage:

<html> <head>
    <title>Test Page</title>
    <script type="text/javascript"
     src="http://ajax.googleapis.com/ajax/libs/jquery/1.3/jquery.min.js"></script>
</head> <body>
<script type="text/javascript">
    function kickoff() {
        $.getJSON("unplanned.json", resetTable);
    }
    function resetTable(rows) {
        $("#content tbody").empty();
        for(var i=0; i<rows.length; i++) {
            $("<tr>"
                + "<td>" + rows[i].mpe_name + "</td>"
                + "<td>" + rows[i].bin + "</td>"
                + "<td>" + rows[i].request_time + "</td>"
                + "<td>" + rows[i].filtered_delta + "</td>"
                + "<td>" + rows[i].failed_delta + "</td>"
            + "</tr>").appendTo("#content tbody");
        }
        setTimeout(kickoff, 2000);
    }
    $(kickoff);
</script>
<table id="content" border="1" style="width:100% ; text-align:center">
<thead><tr>
    <th>MPE</th> <th>Bin</th> <th>When</th> <th>Filtered</th> <th>Failed</th>
</tr></thead>
<tbody></tbody>
</table>
</body> </html>

If it helps, here's an example of the json I'm sending back (it's this exact array wuith thousands of entries instead of just one):

[
    {
        mpe_name: "DBOSS-995",
        request_time: "09/18/2009 11:51:06",
        bin: 4,
        filtered_delta: 1,
        failed_delta: 1
    }
]

EDIT: I've accepted Toran's extremely helpful answer, but I feel I should post some additional code, since his removefromdom jQuery plugin has some limitations:

  • It only removes individual elements. So you can't give it a query like `$("#content tbody tr")` and expect it to remove all of the elements you've specified.
  • Any element that you remove with it must have an `id` attribute. So if I want to remove my `tbody`, then I must assign an `id` to my `tbody` tag or else it will give an error.
  • It removes the element itself and all of its descendants, so if you simply want to empty that element then you'll have to re-create it afterwards (or modify the plugin to empty instead of remove).

So here's my page above modified to use Toran's plugin. For the sake of simplicity I didn't apply any of the general performance advice offered by Peter. Here's the page which now no longer memory leaks:

<html>
<head>
    <title>Test Page</title>
    <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.3/jquery.min.js"></script>
</head>
<body>
<script type="text/javascript">
<!--
    $.fn.removefromdom = function(s) {
        if (!this) return;

        var el = document.getElementById(this.attr("id"));

        if (!el) return;

        var bin = document.getElementById("IELeakGarbageBin");

        //before deleting el, recursively delete all of its children.
        while (el.childNodes.length > 0) {
            if (!bin) {
                bin = document.createElement("DIV");
                bin.id = "IELeakGarbageBin";
                document.body.appendChild(bin);
            }

            bin.appendChild(el.childNodes[el.childNodes.length - 1]);
            bin.innerHTML = "";
        }

        el.parentNode.removeChild(el);

        if (!bin) {
            bin = document.createElement("DIV");
            bin.id = "IELeakGarbageBin";
            document.body.appendChild(bin);
        }

        bin.appendChild(el);
        bin.innerHTML = "";
    };

    var resets = 0;
    function kickoff() {
        $.getJSON("unplanned.json", resetTable);
    }
    function resetTable(rows) {
        $("#content tbody").removefromdom();
        $("#content").append('<tbody id="id_field_required"></tbody>');
        for(var i=0; i<rows.length; i++) {
            $("#content tbody").append("<tr><td>" + rows[i].mpe_name + "</td>"
                + "<td>" + rows[i].bin + "</td>"
                + "<td>" + rows[i].request_time + "</td>"
                + "<td>" + rows[i].filtered_delta + "</td>"
                + "<td>" + rows[i].failed_delta + "</td></tr>");
        }
        resets++;
        $("#message").html("Content set this many times: " + resets);
        setTimeout(kickoff, 2000);
    }
    $(kickoff);
// -->
</script>
<div id="message" style="color:red"></div>
<table id="content" border="1" style="width:100% ; text-align:center">
<thead><tr>
    <th>MPE</th>
    <th>Bin</th>
    <th>When</th>
    <th>Filtered</th>
    <th>Failed</th>
</tr></thead>
<tbody id="id_field_required"></tbody>
</table>
</body>
</html>

FURTHER EDIT: I'll leave my question unchanged, though it's worth noting that this memory leak has nothing to do with Ajax. In fact, the following code would memory leak just the same and be just as easily solved with Toran's removefromdom jQuery plugin:

function resetTable() {
    $("#content tbody").empty();
    for(var i=0; i<1000; i++) {
        $("#content tbody").append("<tr><td>" + "DBOSS-095" + "</td>"
            + "<td>" + 4 + "</td>"
            + "<td>" + "09/18/2009 11:51:06" + "</td>"
            + "<td>" + 1 + "</td>"
            + "<td>" + 1 + "</td></tr>");
    }
    setTimeout(resetTable, 2000);
}
$(resetTable);

回答1:

I'm not sure why firefox isn't happy w/ this but I can say from experience that in IE6/7/8 you must set innerHTML = ""; on the object you want removed from the DOM. (if you created this DOM element dynamically that is)

$("#content tbody").empty(); might not be releasing these dynamically generated DOM elements.

Instead try something like the below (this is a jQuery plugin I wrote to solve the issue).

jQuery.fn.removefromdom = function(s) {
    if (!this) return;

    var bin = $("#IELeakGarbageBin");

    if (!bin.get(0)) {
        bin = $("<div id='IELeakGarbageBin'></div>");
        $("body").append(bin);
    }

    $(this).children().each(
            function() {
                bin.append(this);
                document.getElementById("IELeakGarbageBin").innerHTML = "";
            }
    );

    this.remove();

    bin.append(this);
    document.getElementById("IELeakGarbageBin").innerHTML = "";
};

You would call this like so: $("#content").removefromdom();

The only issue here is that you need to re-create your table each time you want to build it.

Also, if this does solve your issue in IE you can read more about this in a blog post that I wrote earlier this year when I came across the same problem.

Edit I updated the plugin above to be 95% JavaScript free now so it's using more jQuery than the previous version. You will still notice that I have to use innerHTML because the jQuery function html(""); doesn't act the same for IE6/7/8



回答2:

I'm not sure about the leak, but your resetTable() function is very inefficient. Try fixing those problems first and see where you end up.

  • Don't append to the DOM in a loop. If you must do DOM manipulation, then append to a document fragment, and then move that fragment to the DOM.
  • But innerHTML is faster than DOM manipulation anyway, so use that if you can.
  • Store jQuery sets into local variables - no need to re-run the selector every time.
  • Also store repeated references in a local variable.
  • When iterating over a collection of any sort, store the length in a local variable, too.

New code:

<html> <head>
    <title>Test Page</title>
    <script type="text/javascript"
     src="http://ajax.googleapis.com/ajax/libs/jquery/1.3/jquery.min.js"></script>
</head> <body>
<script type="text/javascript">
$(function()
{
    var $tbody = $("#content tbody");

    function kickoff() {
        $.getJSON("test.php", resetTable);
    }

    function resetTable(rows)
    {
        var html = ''
          , i = 0
          , l = rows.length
          , row;
        for ( ; i < l; i++ )
        {
            row = rows[i];
            html += "<tr>"
                + "<td>" + row.mpe_name + "</td>"
                + "<td>" + row.bin + "</td>"
                + "<td>" + row.request_time + "</td>"
                + "<td>" + row.filtered_delta + "</td>"
                + "<td>" + row.failed_delta + "</td>"
            + "</tr>";
        }
        $tbody.html( html );
        setTimeout(kickoff, 2000);
    }

    kickoff();
});
</script>
<table id="content" border="1" style="width:100% ; text-align:center">
<thead>
    <th>MPE</th> <th>Bin</th> <th>When</th> <th>Filtered</th> <th>Failed</th>
</thead>
<tbody></tbody>
</table>
</body> </html>

References:

  • jQuery Peformance Rules
  • Speed Up Your Javascript


回答3:

Please correct me if I'm wrong here but doesn't SetTimeout(fn) prevents the release of the caller's memory space? So that all variables/memory allocated during the resetTable(rows) method would remain allocated until the loop finished?

If that's the case, pushing string construction and appendTo logic to a different method may be a little better since those objects would get freed up after each calling and only the memory of the returned value (in this case either the string markup or nothing if the new method did the appendTo()) would remain in memory.

In essence:

Initial Call to kickoff

-> Calls resetTable()

-> -> SetTimeout Calls kickoff again

-> -> -> Calls resetTable() again

-> -> -> -> Continue until Infinite

If the code never truly resolves, the tree would continue to grow.

An alternative way of freeing up some memory based on this would be like the below code:

function resetTable(rows) {
    appendRows(rows);
    setTimeout(kickoff, 2000);
}
function appendRows(rows)
{
    var rowMarkup = '';
    var length = rows.length
    var row;

    for (i = 0; i < length; i++)
    {
        row = rows[i];
        rowMarkup += "<tr>"
                + "<td>" + row.mpe_name + "</td>"
                + "<td>" + row.bin + "</td>"
                + "<td>" + row.request_time + "</td>"
                + "<td>" + row.filtered_delta + "</td>"
                + "<td>" + row.failed_delta + "</td>"
                + "</tr>";      
    }

    $("#content tbody").html(rowMarkup);
}

This will append the markup to your tbody and then finish that part of the stack. I am pretty sure that each iteration of "rows" will still remain in memory; however, the markup string and such should free up eventually.

Again...it's been a while since I've looked at SetTimeout at this low level so I could be completely wrong here. In anycase, this won't remove the leak and only possibly decrease the rate of growth. It depends on how the garbage collector of the JavaScript engines in use deal with SetTimeout loops like you have here.