Ticket #5382 (new defect)

Opened 12 months ago

Last modified 4 months ago

Memory leak on new() and destroy() widget

Reported by: guest Owned by: bill
Priority: high Milestone: future
Component: Dijit Version: 1.0
Severity: normal Keywords:
Cc:

Description (last modified by bill) (diff)

Just create a widget,and then destroy it.This will causes memory leak. I use procexp.exehttp://www.microsoft.com/technet/sysinternals/utilities/processexplorer.mspx to monitor the memory usage,and it's increasing. My OS is Windows XP and this memory leak occur in IE6 and IE7 both.

graph from sIeve

Attachments

test.html (0.9 kB) - added by guest 12 months ago.
test page.
Memo.js (348 bytes) - added by guest 12 months ago.
the widget written for test.
memory usage graph.rar (43.9 kB) - added by guest 11 months ago.
memory usage graph(by procexp.exe)
memory.png (4.8 kB) - added by bill 6 months ago.
graph from sIeve
iehashbug.html (446 bytes) - added by haysmark 4 months ago.
Test demonstrating IE hash bug, which causes IE to fail to release the memory even on delete and CollectGarbage?().
iebug_graph.JPG (22.6 kB) - added by haysmark 4 months ago.
Result of running IE hash bug test in sieve.

Change History

Changed 12 months ago by guest

test page.

Changed 12 months ago by guest

the widget written for test.

Changed 12 months ago by bill

  • owner set to bill
  • milestone set to 1.1

Note: related to #4641 but this is failing on a trivial widget, so easier test case then using Menu. I'll try to address for 1.1.

Changed 12 months ago by peller

while it's entirely possible we have a memory leak here, I'm not sure increasing process size proves it. There's GC running and caches, so it's not always obvious. Does this growth continue linearly?

Changed 11 months ago by guest

memory usage graph(by procexp.exe)

Changed 11 months ago by guest

yeah,the memory usage is ebb and flow,but it's increasing obviously.I upload two memory usage graphs.One is near 5 minutes after IE browser running,and the other is near 10 minutes.The memory is 20M first,and it's 24M 5 minutes later,and 30M 10 minutes later.

Changed 9 months ago by bill

  • milestone changed from 1.1 to 1.2

Changed 8 months ago by doughays

  • priority changed from normal to high

can this be moved back into 1.1?

Changed 8 months ago by peller

I think it's out of 1.1 unless there's a patch to look at. Perhaps we can consider for 1.1.1 also with a patch.

Changed 6 months ago by bill

graph from sIeve

Changed 6 months ago by bill

  • description modified (diff)

Yup, I see the memory increasing, although sIeve says there are no DOM leaks... I guess it's a different sort of problem.

Changed 4 months ago by bill

  • milestone changed from 1.2 to future

Changed 4 months ago by haysmark

Test demonstrating IE hash bug, which causes IE to fail to release the memory even on delete and CollectGarbage?().

Changed 4 months ago by haysmark

Result of running IE hash bug test in sieve.

Changed 4 months ago by haysmark

It's not a Dojo bug; it's an IE bug with deleting from hashes.

I've attached a very simple test with zero Dojo demonstrating this bug. Basically, delete from a hash does not clear the memory in IE. Even calling CollectGarbage?() after the delete does not clean it up! This bug affects many key Dojo and Dijit components, like dojo.connect and dijit.registry (dijit/manager.js), since they keep a hash for things like widget id.

For example, dijit/manager.js has some bugged code:

remove: function(/*String*/ id){
	delete this._hash[id];
},

Just creating and destroying dijit._Widget 500 times will increase the memory footprint by .5MB! The graph:

Result of running IE hash bug test in sieve.

Fortunately I've found the workaround:

  1. Make a new hash.
  2. Mixin the old hash's contents.
  3. Delete the old hash.
  4. Replace the reference to the old hash with the new hash.
  5. CollectGarbage?()

For instance, in dijit/manager.js:

remove: function(/*String*/ id){
	delete this._hash[id];
	// IE seems to leave remnants of the reference anyway in memory
	// need to delete the entire hash to clean up
	var newhash={};
	dojo.mixin(newhash,this._hash);
	delete this._hash;
	this._hash=newhash;
	if(dojo.isIE){
		// invoke the GC for best results
		CollectGarbage();
	}
},

This code creates a drastic improvement in memory footprint for plain _Widgets! Of course more complicated widgets, like Menu, will still eat up a lot of memory. My guess is that dojo.connects are also failing to be cleared from memory, whether they are hashing/being hashed, or are just using plain arrays. Basically, wherever we delete/splice/whathaveyou something from a hash/array, we have to paste in this workaround to keep memory down.

Changed 4 months ago by bill

Interesting (and depressing), particularly that this occurs on IE7 too.

I think recreating a hash every time an element is added would be unacceptable because of the CPU cost (and also btw I'm surprised it doesn't leak more memory than the current code). I suppose we could have some kind of Hash class that recreated the hash every 100 inserts or so... it's a pretty radical change though.

In any case it's good to know what's going on.

Note: See TracTickets for help on using tickets.