894193 - Push race condition when device unregisters when server has outstanding unACKed notification

Status: RESOLVED WONTFIX

(Core :: DOM: Device Interfaces, defect)

Description

[Chris Peterson [:cpeterson]]

STR:
1. Web app registers for channel with channelID X.
2. App server sends notification to push server for channelID X.
3. Web app calls unregister(), removing device entry for channelID X.
4. *BEFORE* push server receives and processes unregister message, the push server tries to deliver channelID X notification from step 2.

RESULT:
Device is confused because it received a notification for a channel that it no longer knows about! The server will continue to redeliver the notification because the device doesn't ACK a notification for a channel it no longer recognizes.

I saw this problem when I was testing with dougt's Go push server, but Nikhil thinks it is theoretically possible with other push servers. dougt's push server kept delivering the unrecognized notification, so I added some device code to send a fake ACK to shut the server up.

Comment 1

[JR Conlin [:jrconlin,:jconlin]]

PushGo should be good on this, since an unregister cleans the associated data out of memcache. I'm adding a test to the full client test suite to verify that this is the case.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to JR Conlin [:jrconlin,:jconlin] from comment #1)
> PushGo should be good on this, since an unregister cleans the associated
> data out of memcache. I'm adding a test to the full client test suite to
> verify that this is the case.

What about the case where the unregister never reaches the push server?

Comment 3

[Chris Peterson [:cpeterson]]

nsm is referring to the fact that the current push client removes the channel from its local channel list BEFORE sending the unregister request to the server.

Comment 4

[JR Conlin [:jrconlin,:jconlin]]

In any case, the server deletes pending updates for an unregistered channel. It also relays any update for a channel associated with the UAID. It is assumed that the client will ignore any invalid channel and will ACK by responding with all channels sent (valid or not). On ACK, the server removes the version information for the associated channels. 

This is done in order to reduce the scaling issues associated with the protocol so that the greatest number of clients can be served for the least cost.

Comment 5

[Chris Peterson [:cpeterson]]

So it sounds like this is not really a bug and clients need to be sure to ACK every update received, even for unknown channels. We might want to clarify that in the SimplePush wiki docs.