Unsubscribe inside publish triggers an error

[krzysieki]

Hi

Firstly thanks for this great piece of asynchronous code!

Sometimes there is a need to unsubscribe from a message in a callback. At the moment (in some conditions) it triggers an error. Code example:

var mySubscriber = function(msg, data) {
  PubSub.unsubscribe(mySubscriber);
  console.log('mySubscriber');
};
var mySubscriber2 = function(msg, data) {
  console.log('mySubscriber2');
};

PubSub.subscribe( 'MY MESSAGE', mySubscriber);
PubSub.subscribe( 'MY MESSAGE', mySubscriber2);

PubSub.publish( 'MY MESSAGE', 'hello world!' );

BR

[mroderick]

Hey, thank you for your error report.

I've created a new release that fixes this issue: https://github.com/mroderick/PubSubJS/releases/tag/v1.3.9

[Mathews2115]

@mroderick
First of all, thank you for this wonderful library.

Though I would like to dispute this fix:
What if a user has more than 2 subscribers to a message? And let's say callback number 2 decides to unsubscribe from this message. This means that callback number three will not be called during this delivery session because max length was reached.
(for example, you have several dialogs subscribing to a "close all dialogs" message. The last dialog will not get a message delivered)

var mySubscriber = function(msg, data) {
  // Unsubscribe - I only want to be fired once
  PubSub.unsubscribe(mySubscriber);
  console.log('mySubscriber');
};
var mySubscriber2 = function(msg, data) {
  console.log('mySubscriber2');
};
var mySubscriber3 = function(msg, data) {
  console.log('mySubscriber3 - I never get fired off, sad face.');
};


PubSub.subscribe( 'ONE SHOT', mySubscriber);
PubSub.subscribe( 'ONE SHOT', mySubscriber2);
PubSub.subscribe( 'ONE SHOT', mySubscriber3);

PubSub.publish( 'ONE SHOT', 'hello world!' );

A suggestion for a fix would be to add a 'deleteRequested' flag that gets asserted if unsubscribe is called during delivery. If not currently delivering, the subscriber is immediately deleted.

When iterating through the subscriber list, only call callSubscriber if the deleted flag is not asserted for the subscriber.

Then, when you are finished iterating through the list - call a cleanup function that will delete all subscribers that have the deleted flag asserted.

Thoughts?

[aron]

@Mathews2115 have you run into this issue recently? The internal code has changed a fair bit since this issue was closed and the above example works as expected on the latest version. If you can reproduce it, it's worth updating the example and opening a new issue for this.

[Mathews2115]

@aron You're completely right. I'm dumb. I thought I had a later version and didn't verify before commenting, therefore committing every sin I complain about. Thank you for verifying this.
Cheers.