From: Mateusz Waronwy <mmzwarowny@dundee.ac.uk>
Date: Wed, 21 Oct 2020 13:51:06 +0000 (+0200)
Subject: JAL-3690 Fix concurrent modification exception when workers are cancelled
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=c87a31d2275d86ec5d64cc20844c6cd626037737;p=jalview.git

JAL-3690 Fix concurrent modification exception when workers are cancelled
---

diff --git a/src/jalview/workers/AlignCalcManager2.java b/src/jalview/workers/AlignCalcManager2.java
index 7d3cca6..dcaff10 100644
--- a/src/jalview/workers/AlignCalcManager2.java
+++ b/src/jalview/workers/AlignCalcManager2.java
@@ -1,6 +1,8 @@
 package jalview.workers;
 
 import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CancellationException;
@@ -9,13 +11,12 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.FutureTask;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
 import jalview.api.AlignCalcListener;
-import jalview.api.AlignCalcManagerI;
 import jalview.api.AlignCalcManagerI2;
 import jalview.api.AlignCalcWorkerI;
+import jalview.bin.Cache;
 import jalview.datamodel.AlignmentAnnotation;
 
 import static java.util.Collections.synchronizedList;
@@ -25,6 +26,8 @@ import static java.util.Collections.unmodifiableList;
 import java.util.ArrayList;
 import java.util.HashSet;
 
+import static java.lang.String.format;
+
 public class AlignCalcManager2 implements AlignCalcManagerI2
 {
   class AlignCalcTask extends FutureTask<Void>
@@ -35,6 +38,7 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     {
       super(new Callable<Void>() {
         public Void call() throws Exception {
+            Cache.log.debug(format("Worker %s started%n", worker.getClass().getName()));
             notifyStarted(worker);
             worker.run();
             return null;
@@ -57,7 +61,8 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       {
         get();
         success = true;
-      } catch (ExecutionException e)
+      }
+      catch (ExecutionException e)
       {
         exception = e.getCause();
         if (exception instanceof OutOfMemoryError) {
@@ -72,9 +77,15 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
         tasks.remove(this);
       }
       if (success)
+      {
+        Cache.log.debug(format("Worker %s finished%n", getWorker().getClass().getName()));
         notifyCompleted(worker);
-      else
+      }
+      else if (exception != null){
+        Cache.log.warn(format("Worker %s failed%n", getWorker().getClass().getName()));
+        exception.printStackTrace();
         notifyExceptional(worker, exception);
+      }
     }
   }
 
@@ -101,6 +112,7 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
   @Override
   public void registerWorker(AlignCalcWorkerI worker)
   {
+    Objects.requireNonNull(worker);
     synchronized (registered)
     {
       if (!registered.contains(worker))
@@ -139,13 +151,11 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
   {
     synchronized (registered)
     {
-      for (var it = registered.iterator(); it.hasNext();)
+      for (var worker : registered)
       {
-        var worker = it.next();
         if (worker.getClass().equals(cls))
         {
-          it.remove();
-          disabled.remove(worker);
+          removeWorker(worker);
         }
       }
     }
@@ -156,13 +166,11 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
   {
     synchronized (registered)
     {
-      for (var it = registered.iterator(); it.hasNext();)
+      for (var worker : registered)
       {
-        var worker = it.next();
         if (worker.involves(annot) && worker.isDeletable())
         {
-          it.remove();
-          disabled.remove(worker);
+          removeWorker(worker);
         }
       }
     }
@@ -171,7 +179,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
   @Override
   public void disableWorker(AlignCalcWorkerI worker)
   {
-    assert registered.contains(worker);
     disabled.add(worker);
   }
 
@@ -197,16 +204,14 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
   @Override
   public void startWorker(AlignCalcWorkerI worker)
   {
-    assert registered.contains(worker);
-    synchronized (tasks) {
-      for (var task : tasks)
-      {
-        if (task.getWorker().equals(worker))
-          task.cancel(true);
-      }
-    }
+    Objects.requireNonNull(worker);
     AlignCalcTask newTask = new AlignCalcTask(worker);
-    tasks.add(newTask);
+    synchronized (inProgress)
+    {
+      cancelWorker(worker);
+      inProgress.add(worker);
+      tasks.add(newTask);
+    }
     notifyQueued(worker);
     executor.execute(newTask);
   }
@@ -218,10 +223,11 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     {
       synchronized (tasks) 
       {
-        for (var task : tasks)
-        {
-          if (task.getWorker().equals(worker))
-            task.cancel(true);
+        Optional<AlignCalcTask> oldTask = tasks.stream()
+            .filter(task -> task.getWorker().equals(worker))
+            .findFirst();
+        if (oldTask.isPresent()) {
+          oldTask.get().cancel(true);
         }
       }
     }