[GH-ISSUE #411] SyncPendingMessagesService uniqueness check appears unreliable #291

New issue

Open

opened 2026-02-28 01:24:00 +03:00 by kerem · 1 comment

kerem commented

2026-02-28 01:24:00 +03:00

Owner

Originally created by @alxndrsn on GitHub (Apr 7, 2016).
Original GitHub issue: https://github.com/ushahidi/SMSSync/issues/411

Apparent issues

From reading the code of SyncPendingMessagesService, it appears the isServiceWorking() method is meant to ensure that only a single instance of the class is ever executing at one time.

`mService` assignment not safe

mService is set without checking if has been set previously: github.com/ushahidi/SMSSync@a1f2224bbb/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java (L89)

This appears to mean that if multiple service instance were created, only the most recent will be assigned to SyncPendingMessagesService.mService. Even though one or many older instances could still be running, this will not be reported by SyncPendingMessagesService.isServiceWorking().

`mService` property not thread-safe

Shared property mService is accessed in a non thread-safe way:

In rare cases this could lead to a NullPointerException in isWorking().

`isWorking()` will only evaluate `true` at an unspecified time in the future

isWorking() returns true if the service's getState().state == SyncStatus.SYNC. However, this value is only assigned in SyncPendingMessagesTask.syncPending(), which is only called from SyncPendingMessagesTask.sync(), which is only called from ``SyncPendingMessagesTask.doInBackground()](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/task/SyncPendingMessagesTask.java#L92), which is run, via [this TaskAsync.execute()` call, on a different thread.

Possible effects

It seems likely that pending messages can be sent multiple times if multiple instances of SyncPendingMessagesService are run in parallel.

Originally created by @alxndrsn on GitHub (Apr 7, 2016). Original GitHub issue: https://github.com/ushahidi/SMSSync/issues/411 # Apparent issues From reading the code of `SyncPendingMessagesService`, it appears the `isServiceWorking()` method is meant to ensure that only a single instance of the class is ever executing at one time. ## `mService` assignment not safe `mService` is set without checking if has been set previously: https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L89 This appears to mean that if multiple service instance were created, only the most recent will be assigned to `SyncPendingMessagesService.mService`. Even though one or many older instances could still be running, this will not be reported by `SyncPendingMessagesService.isServiceWorking()`. ## `mService` property not thread-safe [Shared property `mService`](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L57) is accessed in a non thread-safe way: - https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L83 - https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L89 - https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L192 In rare cases this could lead to a `NullPointerException` in [`isWorking()`](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L83). ## `isWorking()` will only evaluate `true` at an unspecified time in the future `isWorking()` returns `true` if the service's `getState().state == SyncStatus.SYNC`. However, this value is only assigned in [`SyncPendingMessagesTask.syncPending()`](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/task/SyncPendingMessagesTask.java#L213), which is only called from [`SyncPendingMessagesTask.sync()`](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/task/SyncPendingMessagesTask.java#L142), which is only called from [``SyncPendingMessagesTask.doInBackground()`](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/task/SyncPendingMessagesTask.java#L92), which is run, via [this `TaskAsync.execute()` call](https://github.com/ushahidi/SMSSync/blob/a1f2224bbb4488a9772eb35a1d8648e0dc350a59/smssync/src/main/java/org/addhen/smssync/presentation/service/SyncPendingMessagesService.java#L114), _on a different thread_. # Possible effects It seems likely that pending messages can be sent multiple times if multiple instances of `SyncPendingMessagesService` are run in parallel.

kerem commented

2026-02-28 01:24:01 +03:00

Author

Owner

@alxndrsn commented on GitHub (Apr 8, 2016):

As SyncPendingMessagesService is a subclass of IntentService, thread-safety may not be an issue.

@alxndrsn commented on GitHub (Apr 8, 2016): As `SyncPendingMessagesService` is a subclass of [`IntentService`](https://developer.android.com/reference/android/app/IntentService.html), thread-safety may not be an issue.