Race condition between unsubscribing and event triggering

[dannyhertz]

Currently if you trigger an event and immediately unsubscribe from a channel there is a race condition between the two operations, often leading to the user being unsubscribed and then an event triggering on a unsubscribed channel. I think processing the unsubscribe event on the _clientEventQueue should ensure proper ordering of operations. What do you think?

[lukeredpath]

@dannyhertz it's a possible solution, but I foresee a potential issue.

Pushing event triggers onto an internal operation queue is nice because the queue can be suspended if the channel disconnects and unsuspended when the channel is re-subscribed, meaning any events that were not triggered prior to disconnect are then sent.

Pushing the unsubscribe call onto this queue could lead to an issue where the channel disconnects after the unsubscribe call, but before the operation runs (say because of a connection issue) - when the connection comes back and the channel is re-subscribed to, the previously queued unsubscribe operation will then run and you'll be unsubscribed again, unexpectedly.

It might be better to use a dispatch queue to synchronise calls to to unsubscribe and triggerEvent but this adds extra complexity.

What problems are you seeing from this race condition? Is the triggering of an event after unsubscribing causing an actual issue for you? It seems that it should just fail gracefully.

Some other possible solutions include:

• Suspending the clientEventQueue when unsubscribe is called.
• Checking isSubscribed inside the block passed to addOperationWithBlock before actually sending the event.

[dannyhertz]

Hey! Thanks for the super fast response.

Yeah, a dispatch queue might be helpful here, but I understand that comes with added complexity. The issue I am having is during my application's channel teardown. I am using a queuing system (based on arrays not threads) to help throttle my events that is flushed and processed every x seconds. When the user is leaving the system it invalidates the timer, flushes/processes any remaining items on the queue and then unsubscribes. The problem is the unsubscribe is happening right away and then the event queue finally tries to trigger the event on an unsubscribed channel. I get a pusher error and it also breaks my application logic because those last events that are triggered are often ones that tell another party to also close his/her channel. Am I missing an obvious way to get around this?

Thanks again!

[lukeredpath]

@dannyhertz how are you using libPusher currently, as a CocoaPods or a static library, or the source directly? If you are using the source or CocoaPods, then you should be able to modify the libPusher source to try some of my suggestions out.

I'd start with wrapping the call to send the event in the operation block in PTPusherChannel with a check to see if it's still subscribed.

[dannyhertz]

@lukeredpath yes, I'm currently using the pod variety. Won't wrapping the call with a check just prevent errors rather than actually give me the behavior I need (sending the final events before unsubscribing)?

[lukeredpath]

Right, I see, I didn't realise you wanted to send the events before unsubscribing. So the inherent issue here is that triggering events is asynchronous and unsubscribing is for all intents and purposes, synchronous.

I've thought about this some more and using a dispatch queue won't necessarily help here. It will ensure that event trigger operations are all added to the operation queue before the unsubscribe call is processed, but it doesn't fix the race condition because there is still the second queue layer that's causing the race.

Maybe pushing the unsubscribe onto the event queue is the answer - I think I can come up with a workaround to the problem I described.

[dannyhertz]

Yup! That's my real issue. I believe using a single queue for all event triggering and channel subscribing would solve the issue, but I'm sure that would introduce it own set of issues. Looking forward to seeing what you come up with and thanks again!

[mloughran]

@lukeredpath I'm not expert on this code, but rather than pushing the unsubscribe onto the queue is there an argument for avoiding the queue in the connected / subscribed case?

@dannyhertz I'm curious, what's your use case?