dataTableSettings not correctly identifying: Leaking Memory

gutzofter

I started a discussion here: http://datatables.net/forums/comments.php?DiscussionID=2306&page=1#Item_0

Here is my failing tests using your framework:
[code]
// DATA_TEMPLATE: dom_data
oTest.fnStart( "Identification Bug" );

$(document).ready( function () {
var tableHolder = $('#demo').html();
var testDataTable = $('#example').dataTable();

/* Bug for identification checks */

oTest.fnTest(
"Bug identified removing table and adding it back - will PASS with bug",
function () {
},
function () {
$('#demo').remove();
$('').appendTo('#container');

$('#demo').html(tableHolder);
testDataTable = $('#example').dataTable({ bDestory: true });
return testDataTable.dataTableSettings.length == 2; }
);

oTest.fnTest(
"Bug identified removing table and adding it back - will FAIL with bug",
function () {
},
function () {
$('#demo').remove();
$('').appendTo('#container');

$('#demo').html(tableHolder);
testDataTable = $('#example').dataTable({ bDestory: true });
return testDataTable.dataTableSettings.length == 1; }
);

oTest.fnComplete();
} );
[/code]

Failure is in both firefox and ie.

Please help!

gutzofter

What I would like to happen is that if you retry to initialize a data table with the same same id it should tell you that. but the comparison is on html elements. So it is possible to add another table with the same id into dataTableSettings. This creates memory leaks.

I noticed this as I added more unit tests to my code the tests got slower and slower.

Note this occurs using beta and stable version.

allan

Hi gutzofter,

Thanks very much for this! Showing the issue in the test harness - nice one :-)

It's a very interesting point that, and not something I had considered - having the table removed from the DOM by and action not under DataTables control. While obviously fair enough - this will remove the table itself, but not any other elements which have been injected by DataTables, and leave the event handlers in place, which isn't really what is wanted I would imagine. To some extent this is got around in your test above by removing the parent element of the table - it will still leave a few things knocking around that you don't want (the memory use as you indicate for example!).

One way around this would be to call fnDestroy() before removing the element from the DOM, which will release everything, and then allow you to take the actions you require.

I'm slightly hesitant to include a check for the ID as well as the table node, since the table has been removed in a "unexpected way". The same theory applies to using fnUpdate to update information in a row, rather than just writing the information content to the element directly - it's under DataTables' control, so the action should be performed through DataTables. That's not to say 'm adverse to the idea, but I'd like to consider it and it's consequences before including that logic :-). Any thoughts on it are very welcome!

I've also spotted that I've got a typo in the 'bDestory' property :-/. This will be corrected in the next beta (or release if that happens first), so something that will require a small code update here...

Regards,
Allan

gutzofter

My application is a single page that uses ajax to load templates intp the parent container. So in the tests if you change from to:
[code]
.remove();
[/code]
[code]
.html('new content');
[/code]
This happens by the. User selecting content them going to some other content. Them back to the original content. So if I use datatables in my other content I keep hanging onto these memory structures until a refresh.

You are right. Really needd some thought on dealing with this.

Side note: in the eighties I worked on a system that when it errored it would output on a teletype: FAT AL ERROR. I became known as the Fat Albert system. So when talking about dstroying datatables we are actually de-storying them.

Thanks for your input.

I heard that it might be possible to bind a listener to the jquery code that

gutzofter

My application is a single page that uses ajax to load templates intp the parent container. So in the tests if you change from to:
[code]
.remove();
[/code]
[code]
.html('new content');
[/code]
This happens by the. User selecting content them going to some other content. Them back to the original content. So if I use datatables in my other content I keep hanging onto these memory structures until a refresh.

You are right. Really needd some thought on dealing with this.

Side note: in the eighties I worked on a system that when it errored it would output on a teletype: FAT AL ERROR. I became known as the Fat Albert system. So when talking about dstroying datatables we are actually de-storying them.

Thanks for your input.

I heard that it might be possible to bind a listener to the jquery code that

gutzofter

@Allan - thanks for the input I'm now using the fnDestroy in my tests. It is kludgey, but will use until I come up with something better. One caveat for the fnDestroy with multiple tables. When I get to a better internet connection than my blackberry. I will update this.

gutzofter

I use two tables in my content. if I use my reference to fnDestroy like this:
[code]
if (tables.length > 0) {
tables.fnDestroy();
}
[/code]

It will reinitialize one of the tables. So I have to do this:
[code]
if (tables.length > 0) {
tables.each(function() {
$(this).dataTable().fnDestroy();
});
}
[/code]

Is this normal?

allan

Yes this is expected - DataTables' API function will only operate on the active instance. Which instance of the object is considered to be active is defined by the variable $.fn.dataTableExt.iApiIndex ( http://datatables.net/development/ ).

It's not ideal, and will probably change when it comes time to think about DataTables 2, but this is expected behaviour at the moment.

Still thinking about the best way of dealing with the memory leak you've pointed out - thinking that it will be a good check to make and deal with. The key thing will be to make it efficient :-)

Regards,
Allan

gutzofter

@allan -

oops! found an artifact in my code. The testing for the tables length > 0. This came from trying to destroy tables, the code that I use is:
[code]
removeDataTables: function() {
tables.each(function() {
$(this).dataTable().fnDestroy();
});
}
[/code]

Looking at the above code, I'm entirely satisfied with it. The each explicitly tells me what is going on: I'm iterating over a list of tables.

A side note concerning the logging. For me using alerts is just crazy. I'm replacing that with exception generation.
I use an automated testing framework, so i need exceptions instead of something that stops code execution and waits for user input.

Maybe generate an exception when trying to remove the HTML.

allan

DataTables will only generate an alert() when there is an important error that the developer needs to be aware of and be able to take action about it. The alerts() should never make it to the point where they can be visible by the end user.

Regards,
Allan

gutzofter

@allan - I'm sorry if I gave you the impression that my development cycle is:
1> make change to code
2> Go look at display of web page.

Most of my development is through TDD. The only time that I'm looking at the actual web page is when I'm finished with my functionality. So I'm writing unit tests all the time. I have my browser set up with a program called XRefresh that when I save my code it will automatically refresh my browser (my unit tests get run). I need the unit test to tell me that there is a problem. I don't want to to have to click on anything during my execution of unit tests.

allan

I think this is fair enough - I'll look at including an option to have DataTables throw an error in future, rather than the alert. Should be easy in 1.7 since _fnLog is it the single point for this kind of error.

Allan