Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition for Pubsub unsubscribe function #11

Open
marusshi opened this issue May 7, 2018 · 1 comment
Open

Race condition for Pubsub unsubscribe function #11

marusshi opened this issue May 7, 2018 · 1 comment
Labels

Comments

@marusshi
Copy link
Collaborator

marusshi commented May 7, 2018

Currently what happens is that in Pubsub.prototype.unsubscribe, the handler for the topic is first deleted before the mqtt client unsubscribes from the topic. During this time however (when the topic handler no longer exists but the mqtt client has not yet completed unsubscribing), a publisher can publish to the topic which will invoke a non-existent handler.
Error received:

   Uncaught TypeError: self.handlers[topic] is not a function
      at MqttClient.<anonymous> (C:\Users\Benji\ThingsJS\ThingsJS\lib\core\Pubsub.js:39:23)

Handler should probably be removed after mqtt client successfully unsubscribes first.

@marusshi marusshi added the bug label May 7, 2018
@jungkumseok
Copy link
Collaborator

I'd argue that the handler should still be removed before the actual client.unsubscribe call, as the user of the Pubsub object probably intends to unsubscribe immediately. In other words, the user does not wish to listen to the given topic as soon as she calls pubsub.unsubscribe, so if messages are received during this process, it should probably be ignored.

i.e.

  1. User calls pubsub.unsubscribe
  2. Handler is deleted
  3. Our object calls client.unsubscribe
  4. Mqtt message ACKed

So if a message is received after 1, but before 4, they should not be handled.

That said, to handle the error you provided, I think we can put a check and print a warning if there is no handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants