mwkirk / javapns

Test import of svn javapns repo from Google Code
3 stars 0 forks source link

The NotificaitionThread is not thread safe. #171

Open mwkirk opened 11 years ago

mwkirk commented 11 years ago

Original author: meirda...@gmail.com (March 04, 2013 06:52:39)

What steps will reproduce the problem? I notice a potential problems viewing the source code.

What is the expected output? What do you see instead? Unexplained errors using the PushQueue.

What version of the product are you using? On what operating system? 2.2

Please provide any additional information below. It seem that at some locations the Push-Queue is not thread-safe. For example: in runQueue() method (in NotificationThread class),

The following line are not thread safe. "... PayloadPerDevice message = messages.get(0); messages.remove(message); ..."

two threads my call messages.get(0) one after the other and send the same message twice. (and then then both messages will remove the 2 next messages- hence the second message wont be sent).

also the while statement may cause a NullException, since aworking thread may enter the loop that will soon be empty. while (!messages.isEmpty())

Cheers, Meir

Original issue: http://code.google.com/p/javapns/issues/detail?id=171

mwkirk commented 11 years ago

From meirda...@gmail.com on March 04, 2013 08:37:20 This issue is incorrectly report. The messages object is instanced per thread, therefore there is no race condition.

However, a similar claim (race conditions) can be found in: getPushedNotifications(), clearPushedNotifications() 1) getPushedNotifications() - first count the capacity pf number of pushNotificaitons and then adds theme to a Collection. At this point the number could change.

2) I Would be expect to read all the pushed notifications and then clear them. But, how can I be sure that I am removing the notifications that I read. (And not remove a notification that was just added right after I called getPushedNotifications() ).