Skip to content

Commit

Permalink
Fix issue 26
Browse files Browse the repository at this point in the history
Do not cache subscribers.length when publishing topics, as subscribers might change this by subscribing or unsubscribing.
  • Loading branch information
mroderick committed Jul 23, 2013
1 parent 87fd8b6 commit 9c30d81
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
5 changes: 4 additions & 1 deletion src/pubsub.js
Expand Up @@ -70,7 +70,10 @@ https://github.com/mroderick/PubSubJS
return;
}

for ( i = 0, j = subscribers.length; i < j; i++ ){
// do not cache the length of the subscribers array, as it might change if there are unsubscribtions
// by subscribers during delivery of a topic
// see https://github.com/mroderick/PubSubJS/issues/26
for ( i = 0; i < subscribers.length; i++ ){
callSubscriber( subscribers[i].func, originalMessage, data );
}
}
Expand Down
33 changes: 24 additions & 9 deletions test/test-unsubscribe.js
@@ -1,14 +1,15 @@
/*jslint white:true*/
/*jslint white:true, stupid:true*/
/*global
PubSub,
buster,
assert,
refute,
require,
sinon
*/
(function( global ){
"use strict";

var PubSub = global.PubSub || require("../src/pubsub"),
TestHelper = global.TestHelper || require("../test/helper");

Expand All @@ -19,8 +20,8 @@
message = TestHelper.getUniqueString(),
token = PubSub.subscribe( message, func),
result = PubSub.unsubscribe( token );
assert.equals( result, token );

assert.equals( result, token );
},

"should return false when unsuccesful" : function(){
Expand All @@ -35,9 +36,9 @@

// now let's try unsubscribing the same method twice
PubSub.unsubscribe( token );
assert.equals( PubSub.unsubscribe( token ), false );
assert.equals( PubSub.unsubscribe( token ), false );
},


"with function argument should return true when succesful" : function(){
var func = function(){},
Expand All @@ -47,7 +48,7 @@
PubSub.subscribe( message, func);
result = PubSub.unsubscribe( func );

assert.equals( result, true );
assert.equals( result, true );
},

"with function argument should return false when unsuccesful" : function(){
Expand All @@ -69,9 +70,23 @@
PubSub.unsubscribe( func );

// unsubscribe again
assert.equals( PubSub.unsubscribe( func ), false );
}
assert.equals( PubSub.unsubscribe( func ), false );
},

'must not throw exception when unsubscribing as part of publishing' : function(){
refute.exception(function(){
var topic = TestHelper.getUniqueString(),
sub1 = function(){
PubSub.unsubscribe(sub1);
},
sub2 = function(){};

PubSub.subscribe( topic, sub1 );
PubSub.subscribe( topic, sub2 );

PubSub.publishSync( topic, 'hello world!' );
});
}
});

}(this));

0 comments on commit 9c30d81

Please sign in to comment.