From a003c4914103e908e046292e9e828fc551058d33 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Wed, 15 Jun 2016 15:07:39 +0200 Subject: [PATCH 1/2] Revert "fix possible deadlock in serialized background #1689" This reverts commit 99a5170ce231a89d85fb99ce5909a00694eda1b5. This commit broke the serial execution of background tasks. See: Fixes #1775. --- .../java/org/androidannotations/api/BackgroundExecutor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java b/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java index f3296428bc..063774cf59 100644 --- a/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -133,7 +133,7 @@ public static synchronized void execute(Task task) { task.executionAsked = true; future = directExecute(task, task.remainingDelay); } - if ((task.id != null || task.serial != null) && !task.managed.get()) { + if (task.id != null || task.serial != null) { /* keep task */ task.future = future; TASKS.add(task); From 99852635a330a50f4edc7bc43542ddf080fff88d Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Wed, 15 Jun 2016 15:07:52 +0200 Subject: [PATCH 2/2] Support synchronous executors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BackgroundExecutor assumed that its executor was asynchronous, so that in execute(Task), a task could not be removed before it has been added to the TASKS list. However, this assumption was wrong with a synchronous executor: directExecute(…) would execute the task synchronously, leading to call Task#postExecute(), that removes the task from TASKS. Immediately after the execution, directExecute(…) would add the task to TASKS, which let the BackgroundExecutor in an incorrect state. Therefore, add the task to the TASKS list before the execution, so that it will also work with synchronous executors. Fixes #1689. --- .../api/BackgroundExecutor.java | 10 +++--- .../test/ThreadActivityTest.java | 31 ++++++++++++++++--- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java b/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java index 063774cf59..1b1b25b787 100644 --- a/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java +++ b/AndroidAnnotations/androidannotations-core/androidannotations-api/src/main/java/org/androidannotations/api/BackgroundExecutor.java @@ -128,16 +128,14 @@ private static Future directExecute(Runnable runnable, long delay) { * executor) */ 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); } + if (task.serial == null || !hasSerialRunning(task.serial)) { + task.executionAsked = true; + task.future = directExecute(task, task.remainingDelay); + } } /** diff --git a/AndroidAnnotations/androidannotations-core/androidannotations-test/src/test/java/org/androidannotations/test/ThreadActivityTest.java b/AndroidAnnotations/androidannotations-core/androidannotations-test/src/test/java/org/androidannotations/test/ThreadActivityTest.java index c86d16e941..71487e431e 100644 --- a/AndroidAnnotations/androidannotations-core/androidannotations-test/src/test/java/org/androidannotations/test/ThreadActivityTest.java +++ b/AndroidAnnotations/androidannotations-core/androidannotations-test/src/test/java/org/androidannotations/test/ThreadActivityTest.java @@ -163,6 +163,31 @@ public void parallelBackgroundTasks() { } } + @Test + public void serializedBackgroundTasksAsynchronous() throws InterruptedException { + /* set an executor with 4 threads */ + BackgroundExecutor.setExecutor(Executors.newFixedThreadPool(4)); + + testSerializedBackgroundTasks(); + } + + @Test + public void serializedBackgroundTasksSynchronous() throws InterruptedException { + /* set a synchronous executor */ + BackgroundExecutor.setExecutor(new Executor() { + @Override + public void execute(Runnable command) { + try { + command.run(); + } catch (RuntimeException ignored) { + // ignored + } + } + }); + + testSerializedBackgroundTasks(); + } + /** * Verify that serialized background tasks are correctly serialized. * @@ -173,14 +198,10 @@ public void parallelBackgroundTasks() { * Once all tasks have completed execution, verify that the items in the * list are ordered. */ - @Test - public void serializedBackgroundTasks() { + private void testSerializedBackgroundTasks() { /* number of items to add to the list */ final int NB_ADD = 10; - /* set an executor with 4 threads */ - BackgroundExecutor.setExecutor(Executors.newFixedThreadPool(4)); - /* * the calls are serialized, but not necessarily on the same thread, so * we need to synchronize to avoid cache effects