BackgroundExecutor may cause deadlock for serial tasks #1689
Description
First of all, this issue only happens when running robolectric test because I override the Executor, but it can happen in production if tasks are executed too fast.
I have the following executor, that simple execute tasks in serial for test purpose:
public class UIThreadExecutor implements Executor{
@Override
public void execute(@NonNull Runnable command) {
try {
command.run();
}catch (Exception ignored){
}
}
}
And I have the follow test:
@Test
public void testSerial(){
BackgroundExecutor.setExecutor(new UIThreadExecutor());
for (int i = 0; i < 10; i++){
activity.testSerial();
}
await().atMost(10, TimeUnit.SECONDS).until(new Callable<Boolean>() {
@Override
public Boolean call() throws Exception {
return activity.count == 10;
}
});
}
The activity testSerial implementation is:
public static int count = 0;
@Background(serial = "test")
public void testSerial(){
count++;
System.out.println("count = " + count);
}
If you run this test, it will cause dead lock. Digging in the code I was able find out why.
Look at this code:
public static synchronized void execute(Task task) {
Future<?> future = null;
if (task.serial == null || !hasSerialRunning(task.serial)) {
task.executionAsked = true;
future = directExecute(task, task.remainingDelay);
}
if (task.id != null || task.serial != null) {
/* keep task */
task.future = future;
TASKS.add(task);
}
}
As you see the task is add after the directExecute is called. Let's take a look at the Task clean post executed code:
private void postExecute() {
if (id == null && serial == null) {
/* nothing to do */
return;
}
CURRENT_SERIAL.set(null);
synchronized (BackgroundExecutor.class) {
/* execution complete */
TASKS.remove(this);
if (serial != null) {
Task next = take(serial);
if (next != null) {
if (next.remainingDelay != 0) {
/* the delay may not have elapsed yet */
next.remainingDelay = Math.max(0L, targetTimeMillis - System.currentTimeMillis());
}
/* a task having the same serial was queued, execute it */
BackgroundExecutor.execute(next);
}
}
}
}
As you see in this code we remove the task after the execution, but my execution always finish before the Task is added to the stack. Causing a dead lock, because the task is added to to list and never removed.
One possible solution would be simple check if the task is already finished (managed) before add it to the list: if ( (task.id != null || task.serial != null) && !task.managed.get())