Opened 11 years ago

Closed 8 years ago

#7483 closed defect (wontfix)

[patch][ccla]Topics are leaking

Reported by: tableton Owned by: Eugene Lazutkin
Priority: high Milestone: 1.7
Component: Core Version: 1.1.1
Keywords: topic listeners memory leak Cc:
Blocked By: Blocking:

Description

The topic's system of the dojo has leaks by leaving dispatchers of the topics uncleaned, even when all subscribers are unsubscribed alrady.

IMHO: In the situations where topics are used as communication mechanism between dijits, it's not desired to keep the old dispatchers in the memory.

Simple test to reproduce:

dojo.addOnLoad(function(){
	var listener = {
		callme : function(){
			console.debug('Topic published');
		}
	};

	var topicname = "test_topic" ;
	var handler = dojo.subscribe(topicname, listener, 'callme');
	dojo.publish(topicname,[]);
	dojo.unsubscribe(handler);

	topicname = null;
	listener  = null;
	handler   = null;

	//dojo._topics should be empty?
	for(var i in dojo._topics){ console.debug(i); }
});

And a small fix which i use as a temporary solution:

var topic = handle[0];
if(dojo._topics[topic]){
	var ls = dojo._topics[topic]._listeners;
	var has = false;
	for(var i=0; i < ls.length;i++){
		if(ls[i]){ has = true; break }
	}

	//remove dispatcher if the list is empty!
	if(!has){ delete dojo._topics[topic]; }
}

Attachments (2)

7483_tests.diff (1.0 KB) - added by Adam Peller 10 years ago.
and tests from John
7483_patch.diff (1.6 KB) - added by Adam Peller 10 years ago.
posting patch for John Ryding (IBM, CCLA)

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 years ago by bill

Milestone: 1.21.3

as per today's meeting, punting these core bugs

comment:2 Changed 11 years ago by sjmiles

Owner: changed from anonymous to sjmiles

comment:3 Changed 11 years ago by dante

Milestone: 1.3future

comment:4 Changed 10 years ago by Eugene Lazutkin

Owner: changed from sjmiles to Eugene Lazutkin
Status: newassigned

comment:5 Changed 10 years ago by Eugene Lazutkin

Milestone: future1.4

comment:6 Changed 10 years ago by strife25

the problem is that dojo.unsubscribe removes the listener, but not the reference to the original function from dojo._topics.

So:

dojo._topics: {
    "test_topic": function(){
			console.debug('Topic published');
		}
}

still exists after dojo.unsubscribe

Changed 10 years ago by Adam Peller

Attachment: 7483_tests.diff added

and tests from John

Changed 10 years ago by Adam Peller

Attachment: 7483_patch.diff added

posting patch for John Ryding (IBM, CCLA)

comment:7 Changed 10 years ago by Adam Peller

Summary: Topics are leaking[patch][ccla]Topics are leaking

comment:8 Changed 10 years ago by Eugene Lazutkin

Similar thing is done in DojoX AOP (http://bugs.dojotoolkit.org/browser/dojox/trunk/lang/aspect.js). But technically topics aren't leaking. Even if some topics are left unused the only overhead is a dispatcher object. Even a thousand of them (assuming we have a thousand of corresponding topics) unlikely to overload any browser. The only problem would be dynamically created topics. I advocate to shy away from such design decisions.

But in principle I agree that stubs should be removed.

I am not sure that topics are the only source of memory pollution. IIRC events (as defined in connect.js) have the same problem. It would be nice to fix both.

comment:9 Changed 10 years ago by Eugene Lazutkin

Milestone: 1.41.5

bumping tickets that didn't make the 1.4 cut, but most likely to go in the next point release.

comment:10 Changed 10 years ago by Eugene Lazutkin

Milestone: 1.51.6

This functionality is better to be moved upstream to dojo._listener, so both events and topics can benefit from it. This change is the high-impact one => rescheduling to 1.6.

comment:11 Changed 9 years ago by Eugene Lazutkin

Milestone: 1.61.7

Bumping up.

comment:12 Changed 8 years ago by Eugene Lazutkin

Resolution: wontfix
Status: assignedclosed

Event/topics code was rewritten by Kris Zyp => it has its own set of bugs, old bugs cannot be directly transferred to new implementation.

Please open a new ticket with a repro case, if the problem still persists.

Note: See TracTickets for help on using tickets.