Skip to content

Commit caf1550

Browse files
committed
make taskQueueWaitingForInit thread safe
There are two calls to startPendingTasks that can happen on two threads, which means sometimes taskQueueWaitingForInit.poll() can return null and throws a NPE when it is given to submit. To handle this we are making taskQueueWaitingForInit thread safe by calling synchronized on. Doing so handles this case but also rare cases where something is being added as we process the queue.
1 parent fa33217 commit caf1550

File tree

1 file changed

+38
-32
lines changed

1 file changed

+38
-32
lines changed

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSTaskController.java

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -49,25 +49,27 @@ void addTaskToQueue(Runnable runnable) {
4949
}
5050

5151
private void addTaskToQueue(PendingTaskRunnable task) {
52-
task.taskId = lastTaskId.incrementAndGet();
53-
54-
if (pendingTaskExecutor == null) {
55-
logger.debug("Adding a task to the pending queue with ID: " + task.taskId);
56-
// The tasks haven't been executed yet...add them to the waiting queue
57-
taskQueueWaitingForInit.add(task);
58-
} else if (!pendingTaskExecutor.isShutdown()) {
59-
logger.debug("Executor is still running, add to the executor with ID: " + task.taskId);
60-
try {
61-
// If the executor isn't done with tasks, submit the task to the executor
62-
pendingTaskExecutor.submit(task);
63-
} catch (RejectedExecutionException e) {
64-
logger.info("Executor is shutdown, running task manually with ID: " + task.taskId);
65-
// Run task manually when RejectedExecutionException occurs due to the ThreadPoolExecutor.AbortPolicy
66-
// The pendingTaskExecutor is already shutdown by the time it tries to run the task
67-
// Issue #669
68-
// https://github.com/OneSignal/OneSignal-Android-SDK/issues/669
69-
task.run();
70-
e.printStackTrace();
52+
synchronized (taskQueueWaitingForInit) {
53+
task.taskId = lastTaskId.incrementAndGet();
54+
55+
if (pendingTaskExecutor == null) {
56+
logger.debug("Adding a task to the pending queue with ID: " + task.taskId);
57+
// The tasks haven't been executed yet...add them to the waiting queue
58+
taskQueueWaitingForInit.add(task);
59+
} else if (!pendingTaskExecutor.isShutdown()) {
60+
logger.debug("Executor is still running, add to the executor with ID: " + task.taskId);
61+
try {
62+
// If the executor isn't done with tasks, submit the task to the executor
63+
pendingTaskExecutor.submit(task);
64+
} catch (RejectedExecutionException e) {
65+
logger.info("Executor is shutdown, running task manually with ID: " + task.taskId);
66+
// Run task manually when RejectedExecutionException occurs due to the ThreadPoolExecutor.AbortPolicy
67+
// The pendingTaskExecutor is already shutdown by the time it tries to run the task
68+
// Issue #669
69+
// https://github.com/OneSignal/OneSignal-Android-SDK/issues/669
70+
task.run();
71+
e.printStackTrace();
72+
}
7173
}
7274
}
7375
}
@@ -77,19 +79,21 @@ private void addTaskToQueue(PendingTaskRunnable task) {
7779
* Run available pending tasks on an Executor
7880
*/
7981
void startPendingTasks() {
80-
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "startPendingTasks with task queue quantity: " + taskQueueWaitingForInit.size());
81-
if (!taskQueueWaitingForInit.isEmpty()) {
82-
pendingTaskExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() {
83-
@Override
84-
public Thread newThread(@NonNull Runnable runnable) {
85-
Thread newThread = new Thread(runnable);
86-
newThread.setName(OS_PENDING_EXECUTOR + newThread.getId());
87-
return newThread;
82+
synchronized (taskQueueWaitingForInit) {
83+
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "startPendingTasks with task queue quantity: " + taskQueueWaitingForInit.size());
84+
if (!taskQueueWaitingForInit.isEmpty()) {
85+
pendingTaskExecutor = Executors.newSingleThreadExecutor(new ThreadFactory() {
86+
@Override
87+
public Thread newThread(@NonNull Runnable runnable) {
88+
Thread newThread = new Thread(runnable);
89+
newThread.setName(OS_PENDING_EXECUTOR + newThread.getId());
90+
return newThread;
91+
}
92+
});
93+
94+
while (!taskQueueWaitingForInit.isEmpty()) {
95+
pendingTaskExecutor.submit(taskQueueWaitingForInit.poll());
8896
}
89-
});
90-
91-
while (!taskQueueWaitingForInit.isEmpty()) {
92-
pendingTaskExecutor.submit(taskQueueWaitingForInit.poll());
9397
}
9498
}
9599
}
@@ -102,7 +106,9 @@ private void onTaskRan(long taskId) {
102106
}
103107

104108
ConcurrentLinkedQueue<Runnable> getTaskQueueWaitingForInit() {
105-
return taskQueueWaitingForInit;
109+
synchronized (taskQueueWaitingForInit) {
110+
return taskQueueWaitingForInit;
111+
}
106112
}
107113

108114
void shutdownNow() {

0 commit comments

Comments
 (0)