Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#6883 closed defect (fixed)

Grid dojox.grid._View.destroy() leaks nodes/memory in IE

Reported by: guest Owned by: Bryan Forbes
Priority: high Milestone: 1.2
Component: DojoX Grid Version:
Keywords: grid dom node leak Cc: anthony.fryer@…
Blocked By: Blocking:

Description

Running a simple create/destroy test of the dojox.grid.DataGrid? in sieve shows dom node leakage. I tracked down one source of leakage to the destroy() method in _View.js. I'm not sure if this.headerNode is being properly clobbered.

I looked modified _View.destroy() to use the _Widget way of destroying a domNode. A retest in sieve showed a reduction in the number of dom nodes leaked per create/destroy test.

My modified dojox.grid._View.destroy method is shown below...

destroy: function(){
	//dojox.grid.util.removeNode(this.headerNode);
	dojo._destroyElement(this.headerNode);
	delete this.headerNode;
	this.inherited(arguments);
}

Attachments (2)

test_grid_programmatic.html (2.1 KB) - added by guest 11 years ago.
modified test_programmatic for node leak sieve testing of grid
leak_free_grid.txt (3.1 KB) - added by guest 11 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 11 years ago by guest

More investigation revealed there's a few places where dom nodes are leaked in the DataGrid?. I did manage to eventually track them all down and fix it. I can now create and destroy a DataGrid? at will in Sieve and have zero dom node leakage. I have attached the output of svn diff that shows the required changes. Some of these diffs may not be required, but these changes will result in zero dom node leaks.

Anthony Fryer (anthony.fryer@…)

comment:2 Changed 11 years ago by Bryan Forbes

Thanks for your research. I downloaded sIEve http://home.wanadoo.nl/jsrosman/sIEve-0.0.8.exe and ran the main test (test_grid.html) before your patch in it and couldn't detect any leaks. I even modified the test to destroy the grid with a button and I still couldn't get it to display any leaks. Your patch looks sound, but I'm not sure it really solves any problems without seeing the results in sIEve myself (not that I don't believe you).

Changed 11 years ago by guest

Attachment: test_grid_programmatic.html added

modified test_programmatic for node leak sieve testing of grid

comment:3 Changed 11 years ago by guest

You can only see widget node leakes if you dynamically create and destroy a widget in a web page. test_grid.html is a once off instantiation of a grid and won't demonstrate node leaks in the grid.

To demonstrate the leak, i've attached a modified test_grid_programmatic.html that provides a button that toggles the grid (ie. creates and destroys the grid). If you run this in Sieve, every time you destroy the grid, you'll notice the DOM Usage increasing. A good widget should return the Dom usage to the same point every time it is destroyed (unless it has form elements but thats an IE issue with forms).

The grid leaks dom nodes because it's not cleaning up objects that reference dom nodes and has dojo.connects without corresponding dojo.disconnects in quite a few places. The patch i provided addressed these issues. I have an updated patch that i'll upload.

Anthony Fryer

Changed 11 years ago by guest

Attachment: leak_free_grid.txt added

comment:4 Changed 11 years ago by Bryan Forbes

Resolution: fixed
Status: newclosed

(In [13900]) fixes #6883 !strict

  • Fixed memory leaks in IE.
  • Added test to run in sIEve to check for memory leaks

fixes #4983

  • Added column reordering for simple views using dojo.dnd.
  • Changed column resizing to use dojo.dnd.
  • Added test to show column reordering.

comment:5 Changed 11 years ago by bill

Milestone: 1.2

marking tickets closed in the last three months w/blank milestone to milestone 1.2.

Note: See TracTickets for help on using tickets.