pubnub/python — pull request #3 by FrankSomething (2013-07-30 20:25:58+00:00)
Title: Ignore callback fired from unsubscribed Async connection

When a client unsubscribes from channel 'foo', the callback from that original connection should be ignored, even if the client later re-subscribes to channel 'foo.'

Because the AsyncHTTPClient request for the initial subscription is not closed on unsubscribe() (the magic singleton behavior makes this impractical), that request can still return several seconds later. When the request returns, the current code checks to see that the Pubnub object is currently subscribed to 'foo.' If so, then the callback is fired; if not, the callback is not fired and the response is ignored.

However, it can occur that the client unsubscribes from 'foo' and then re-subscribes to 'foo' before that initial request returns. In this case, the original callback and the new callback will be fired (or, if they are the same, the callback will be fired twice: once for the original subscription, and once for the re-subscription). I.e., by the time the original subscription request returns, the Pubnub is again subscribed to channel 'foo' and fires the original callback. Additionally, when the client re-subscribed, another request was created, and that request will also return and fire its callback. There are now two recursive-loops waiting on the channel, each firing its own callback.

The proposed changes tag each call to subscribe() with a uuid and store the uuid(s) in the self.subscriptions[channel] dictionary. When a user unsubscribes from the channel, the stored uuid(s) are removed. Then, when the AsyncHTTPClient request returns, in addition to checking whether the Pubnub object is subscribed to the channel, it also checks that the uuid is present in the self.subscriptions[channel] set. If not (e.g., because the client unsubscribed from the channel), then the callback is not fired and the recursive loop stops.

This provides finer-grained control and avoids some potential race conditions, and would appear to make the "if not self.subscriptions[channel]['connected']:" tests redundant, but I left those in for someone more intimately familiar with this codebase to evaluate.

I note that this appears to be a development branch that's a few months out of sync from master; I've put up a similar PR for master.