skhuttan / javapns

Automatically exported from code.google.com/p/javapns
0 stars 0 forks source link

The NotificaitionThread is not thread safe. #171

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by meirda...@gmail.com on 4 Mar 2013 at 6:52

GoogleCodeExporter commented 9 years ago
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() ).

Original comment by meirda...@gmail.com on 4 Mar 2013 at 8:37

GoogleCodeExporter commented 9 years ago
This was improved in the trunk some time ago.  The PushedNotificationS object 
is an independent snapshot produced by NotificationThread within a synchronized 
block.  No PushedNotification object can be added to the thread's list while 
the PushedNotificationS list is being created.

Original comment by sype...@gmail.com on 13 Oct 2014 at 3:03