Opened 10 years ago

Closed 9 years ago

#10827 closed defect (worksforme)

[patch][ccla] Chart2D with events causes considerable memory leak on render()

Reported by: Jared Jurkiewicz Owned by: Eugene Lazutkin
Priority: high Milestone: 1.6
Component: Charting Version: 1.4.0
Keywords: Cc: mnog@…, tiansh@…
Blocked By: Blocking:

Description (last modified by Adam Peller)

Reported by Masato Noguchi (IBM): When some events are listened by connectToPlot() or actions are added, dojox.charting.Chart2D causes considerable memory leaks on render(). Which is caused by calls of connect() to Shape in dojox.charting.plot2d.Base._connectEvents, which are never disconnected.

The following code actually eliminates the leaks:

dojo.extend(dojox.charting.plot2d.Base, {
		 resetEvents: function(){
		 		 dojo.forEach(this._connects, dojo.disconnect);
		 		 delete this._connects;
		 		 this.plotEvent({type: "onplotreset", plot: this});
		 },
		 _connectEvents: function(shape, o){
		 		 if(!this._connects){
		 		 		 this._connects = [];
		 		 }
		 		 this._connects.push(shape.connect("onmouseover", this, function(e){
		 		 		 o.type  = "onmouseover";
		 		 		 o.event = e;
		 		 		 this.plotEvent(o);
		 		 }));
		 		 this._connects.push(shape.connect("onmouseout", this, function(e){
		 		 		 o.type  = "onmouseout";
		 		 		 o.event = e;
		 		 		 this.plotEvent(o);
		 		 }));
		 		 this._connects.push(shape.connect("onclick", this, function(e){
		 		 		 o.type  = "onclick";
		 		 		 o.event = e;
		 		 		 this.plotEvent(o);
		 		 }));
		 }
});

Attachments (4)

chart_leak.html (4.0 KB) - added by ESBANTE 9 years ago.
Memory leak example
test_chart2d_leak_test.html (8.0 KB) - added by shengt 9 years ago.
charting_memory_leak.patch (7.4 KB) - added by Adam Peller 9 years ago.
updated patch from xiangxz
linechart_leak.html (3.0 KB) - added by aryalrabin 9 years ago.
line chart leak

Download all attachments as: .zip

Change History (24)

comment:1 Changed 10 years ago by Adam Peller

Description: modified (diff)
Summary: Charting: Chart2D with events causes considerable memory leak on render()[patch][ccla] Chart2D with events causes considerable memory leak on render()

comment:2 Changed 10 years ago by Les

This is something I don't understand about Dojo. If I destroy a widget or remove a DOM element, all event event handlers bound to that widget or DOM element should be unbound. I shouldn't have to worry about it.

See the jQuery doc for the remove() method. They unbound event handlers, but dojo.orphan() is not doing that.

http://api.jquery.com/remove/

comment:3 Changed 10 years ago by Eugene Lazutkin

Milestone: tbd1.4.2

comment:4 Changed 10 years ago by bill

Milestone: 1.4.21.5

1.4.2 has already been released. Suggest targeting for 1.5 although we could have a 1.4.3.

comment:5 in reply to:  2 Changed 9 years ago by Eugene Lazutkin

Status: newassigned

Replying to Les:

Next time just open a separate ticket, if you see a general problem. I opened a ticket against the core: #10941.

comment:6 Changed 9 years ago by Eugene Lazutkin

Resolution: fixed
Status: assignedclosed

(In [21714]) Guarding against potential memory leaks, thx Masato Noguchi (IBM) and jaredj!, !strict, fixes #10827.

comment:7 Changed 9 years ago by Adam Peller

Cc: mnog@… tiansh@… added
Resolution: fixed
Status: closedreopened

Sheng Tian (IBM) reports:

dojox.charting.action2d.Magnify, dojox.charting.action2d.MoveSlice?,dojox.charting.action2d.Shake, dojox.charting.DataChart?, dojox.charting.plot2d.Pie, all have dojo.connect without disconnect.

comment:8 Changed 9 years ago by Adam Peller

new patch provided

comment:9 Changed 9 years ago by ESBANTE

Seems like charting.action2d.Tooltip needs to be fixed too.

Changed 9 years ago by ESBANTE

Attachment: chart_leak.html added

Memory leak example

comment:10 Changed 9 years ago by ESBANTE

hi, I was getting memory leaks on IE 8 so i applied the patch to my dojo lib to see if it will fix the problem. although the memory leak persists. I am not sure if the problem is the patch or something on the approach I am using on the chart (see attached file. chart_leak.html)

Changed 9 years ago by shengt

Attachment: test_chart2d_leak_test.html added

comment:11 Changed 9 years ago by shengt

Still can see some memory leak on IE8 after applying Tooltip, Magnify, Highlight actions. See attached test file.

Changed 9 years ago by Adam Peller

Attachment: charting_memory_leak.patch added

updated patch from xiangxz

comment:12 Changed 9 years ago by Eugene Lazutkin

(In [22375]) charting: eliminating potential memory leaks in programmatic events, thx vmware!, !strict, refs #11356, refs #11374, refs #10827.

comment:13 in reply to:  11 Changed 9 years ago by Eugene Lazutkin

Replying to shengt:

Still can see some memory leak on IE8 after applying Tooltip, Magnify, Highlight actions. See attached test file.

No wonder --- your test file creates and attaches actions yet never destroys them. All external objects (actions, legends, data stores, and so on) should be destroyed separately --- the same way they were created separately.

comment:14 Changed 9 years ago by Eugene Lazutkin

I spent quality time with the updated leak test and with current code I see a sustainable memory level. I tired Chrome, IE6, and IE8.

comment:15 Changed 9 years ago by Eugene Lazutkin

Resolution: fixed
Status: reopenedclosed

Robot missed: (In [22379]) charting: memory-leak preventive changes, !strict, fixes #10827.

comment:16 Changed 9 years ago by Eugene Lazutkin

BTW, it is safe to leave JavaScript objects connected using dojo.connect(). But if a JavaScript object connected to a DOM node, it may cause leaks, and such links should be disconnected with dojo.disconnect(). That's why actions do not have to disconnect when they connected to a local animation object.

Changed 9 years ago by aryalrabin

Attachment: linechart_leak.html added

line chart leak

comment:17 Changed 9 years ago by aryalrabin

Resolution: fixed
Status: closedreopened

I am still seeing memory leaks on IE 8, FF, Chrome. I have similar as chart_leak.html. I have attached file linechart_leak.html.

comment:18 in reply to:  17 Changed 9 years ago by aryalrabin

I am using dojo 1.5

comment:19 Changed 9 years ago by Eugene Lazutkin

Milestone: 1.51.6

Bumping from non-existant 1.5.1 to 1.6.

comment:20 Changed 9 years ago by Eugene Lazutkin

Resolution: worksforme
Status: reopenedclosed

I ran linechart_leak.html using the trunk for one hour on Chrome watching the memory size --- it was clearly bounded from above being on average between 40-80M.

Note: See TracTickets for help on using tickets.