Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16206 closed defect (fixed)

this.own() in dijit/Destroyable does not clean up its own internal aspect

Reported by: djtore Owned by: bill
Priority: undecided Milestone: 1.8.2
Component: Dijit Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

Browser: Firefox 16.0.1 OS: Windows 7

Scenario: I have created a custom widget inheriting from _WidgetBase, _TemplatedMixin, and _WidgetsInTemplateMixin. It has a simple templatestring containing a div which I have some event listeners connected to for the events mouse enter, mouse leave and click. I use this.own(on( <node>, <event>, <callback-func>)) to ensure that the listeners are destroyed when my widget is destroyed. I use the latest version of the Dojo Firebug Extension to be able to observe, among other things, the number of on/aspects in my application. This number is reported in the "Info"-tab in the extension.

https://getfirebug.com/wiki/index.php/DojoFirebugExtension_Reference_Guide

I can see that each time an instance of my widget is created, the number of on/aspects increase, but when the widget is destroyed, this number does not decrease. Since I have a lot of widgets in my applications which use this.own() and my widgets are getting created and destroyed several times, the number of on/aspects increase drastically. I assume this increases the memory usage in my browser.

I believe I have been able to track down the bug cause, which resides in dijit/Destroyable.js. The last aspect in that file looks like this:

			aspect.after(handle, destroyMethodName, function(){
				handle._odh.remove();
			});

This aspect is only meant to be run once, but the signal returned from aspect.after() is ignored and never used to cancel the advice. A fix would be

			var afterSignal = aspect.after(handle, destroyMethodName, function(){
                                afterSignal.remove();
				handle._odh.remove();
			});

I have applied this fix in my local copy of Dojo 1.8.0, and have verified that the Dojo Firebug Extension no longer reports an increase in the number of on/aspects.

Change History (4)

comment:1 Changed 7 years ago by bill

Milestone: tbd1.9
Status: newassigned

Hmm, I'm doubtful there's any real bug here... although handle still has a lingering reference to the widget, seems like both of them should be garbage collected anyway, since there are presumably no external references to either of them.

But, I could add in that extra remove() call just to make the firebug extension happy.

comment:2 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [29861]:

Clean up lingering handle to avoid complaint from firebug dojo extension, and possible memory leak. Fixes #16206 !strict.

comment:3 Changed 7 years ago by bill

In [29937]:

Backport [29861] Destroyable handle cleanup to 1.8, fixes #16206 on 1.8/ branch !strict.

comment:4 Changed 7 years ago by bill

Milestone: 1.91.8.2
Note: See TracTickets for help on using tickets.