I think I have found a bug - DOM node leak (detached) when bVisible=false columns are present!
I think I have found a bug - DOM node leak (detached) when bVisible=false columns are present!
brailateo
Posts: 7Questions: 0Answers: 0
After a couple of hours of Google Chrome DevTools digging and memory snapshots it seems that if I have in aoColumnDefs one or more columns that are marked:
[code]
"bVisible": false
[/code]
on table fnDestroy, the browser is constantly leaking DOM nodes. I made a lot of tests and when I make the column visible, the number of DOM nodes is growing and coming back to the exactly same level, even after tens of opening, creating and destroying the dataTable.
I also suspect some strange code in fnDestroy function, when bRemove==true , why are you doing nBody.appendChild( oSettings.aoData[i].nTr ); (the table node shouldn't be destroyed after all?).
Also, in heap snapshot after fnDestroy I have found detached nodes for pagination and input text filter . I suspect also that keypress events are not unbind from them so I made an extra unbind:
[code]
/* Blitz all DT events */
$(oSettings.nTableWrapper).find('*').andSelf().unbind('.DT');
if ( bRemove ) {
$(oSettings.nTableWrapper).find('*').andSelf().unbind();
}
[/code]
I have found also aoData arrays that are not garbage-collected due to a reference kept in the sort function closure oSettings.aiDisplayMaster.sort( ...
I tried to replace that function with something that does not refer aoData at all ( just simply return 1 ) and the memory leak has dissapeared!
Hope this helps in order to locate the bugs because I consider DataTables the best tool for table manipulation available now on the web!
Congratulations for this marvelous software and waiting for new releases!
Best regards,
Constantin Teodorescu
[code]
"bVisible": false
[/code]
on table fnDestroy, the browser is constantly leaking DOM nodes. I made a lot of tests and when I make the column visible, the number of DOM nodes is growing and coming back to the exactly same level, even after tens of opening, creating and destroying the dataTable.
I also suspect some strange code in fnDestroy function, when bRemove==true , why are you doing nBody.appendChild( oSettings.aoData[i].nTr ); (the table node shouldn't be destroyed after all?).
Also, in heap snapshot after fnDestroy I have found detached nodes for pagination and input text filter . I suspect also that keypress events are not unbind from them so I made an extra unbind:
[code]
/* Blitz all DT events */
$(oSettings.nTableWrapper).find('*').andSelf().unbind('.DT');
if ( bRemove ) {
$(oSettings.nTableWrapper).find('*').andSelf().unbind();
}
[/code]
I have found also aoData arrays that are not garbage-collected due to a reference kept in the sort function closure oSettings.aiDisplayMaster.sort( ...
I tried to replace that function with something that does not refer aoData at all ( just simply return 1 ) and the memory leak has dissapeared!
Hope this helps in order to locate the bugs because I consider DataTables the best tool for table manipulation available now on the web!
Congratulations for this marvelous software and waiting for new releases!
Best regards,
Constantin Teodorescu
This discussion has been closed.
Replies
That's a fair point. When the remove flag is set, there isn't much point in adding the rows back into the DOM. I see you are working with 1.9.4 (or before), but this issue is also present in the latest dev code. I'll take a look at optimising this.
Regarding the leak, could you try this in 1.10 please ( http://datatables.net/download - pre-beta). I think / hope, it should be resolve there and the event unbinding is much more efficient...
Allan
thank you for your quick answer! I have downloaded the 1.10.0-dev version and I made some tests!
Yes, this version leaks DOM nodes (and memory) too!
I have prepared a small test program with just two JavaScript libraries (jquery-1.10.2 and dataTables) and just two functions, one that dynamically creates a datatable with data coming from an ajax request, and one that destroys it, and made a few tests watching DOM nodes creating and releasing in Google Chrome Dev Tools / Timeline ! The whole story and test files, screen snapshots of timeline and heap is available here: http://www.mozartrocks.ro/dt-test/
Thank you for your work on this software and looking forward to your new stable release,
Best regards, Constantin Teodorescu
here it is a small patch applied to the 1.9.4 branch, in fnDestroy function that seems to fix some DOM node leakage (not all).
Here: http://www.mozartrocks.ro/dt-test/patch-1.html you will find the patch, 2 images with browser tests and results for DOM nodes and Heap snapshot in both cases (the original source and the patched one).
Hope that this might help you fix the problem
Best regards,
Constantin Teodorescu
This is excellent - thank you very much! I will sit down early next week and investigate in detail the issue here.
Regards,
Allan
It's related to hidden columns headers that hold the "click" event for sorting that is not released.
So, you should add in fnDestroy code to release the binding from those headers that are not displayed!
[code]
/* Blitz all DT events */
$(oSettings.nTableWrapper).find('*').andSelf().unbind('.DT');
$(oSettings.oInstance).off(); // just to be sure no one is listening on "sorting" events
// Remove any event listeners from column headers, even hidden
for ( i=0; i< oSettings.aoColumns.length; i++ ) {
$(oSettings.aoColumns[i].nTh).unbind();
}
[/code]
There is one more DOM node leak that I have to point it out, it's related to something with filtering but I'm not sure at this moment, I will digg further this night!
Best regards,
Constantin Teodorescu
The columns are made visible before the events are unsubscribed: https://github.com/DataTables/DataTablesSrc/blob/master/js/api/api.core.js#L78 . However, there certainly would be a leak there if the `remove` option is set. Good spotting!
Allan
You need to unbind everything from oSettings.aanFeatures['f'] and oSettings.aanFeatures['p'] and properly remove nodes before making oSettings null!
Here it is a HORRIBLE code that I used in order to remove everything from there (I didn't had the time to re-evaluate what it is the minimum actions that need to be taken so I left all that redundant code):
[code]
function recursiveEmptyDOMs(obj) {
if ( obj===null ) return;
if ( $.isArray(obj) ) {
for (var ji=0; ji
Teo
Allan