From: Mateusz Waronwy <mmzwarowny@dundee.ac.uk>
Date: Thu, 12 Nov 2020 15:13:36 +0000 (+0100)
Subject: JAL-3690 - fixed state inconsistency when restarting the calcworker
X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=e756b6615edab2c9c33f5402dc337065a6799065

JAL-3690 - fixed state inconsistency when restarting the calcworker
---

diff --git a/src/jalview/workers/AlignCalcManager2.java b/src/jalview/workers/AlignCalcManager2.java
index f7d263a..7a19321 100644
--- a/src/jalview/workers/AlignCalcManager2.java
+++ b/src/jalview/workers/AlignCalcManager2.java
@@ -28,12 +28,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
 {
   private abstract class WorkerManager
   {
-    static final int IDLE = 0;
-    static final int QUEUED = 1;
-    static final int RUNNING = 2;
-    static final int CANCELLING = 3;
-
-    protected volatile int state = IDLE;
     protected volatile boolean enabled = true;
             
     protected final AlignCalcWorkerI worker;
@@ -58,41 +52,21 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       this.enabled = enabled;
     }
     
-    synchronized protected void setState(int state) 
-    {
-      this.state = state;
-    }
-    
-    int getState()
-    {
-      return state;
-    }
-    
-    void restart()
+    synchronized void restart()
     {
       if (!isEnabled())
       {
         return;
       }
-      if (state == IDLE)
-      {
-        submit();
-      }
-      else if (state == QUEUED)
-      {
-        // job already queued, do nothing
-      }
-      else if (state == RUNNING)
+      if (isWorking())
       {
         cancel();
-        submit();
-      }
-      else if (state == CANCELLING)
-      {
-        submit();
       }
+      submit();
     }
     
+    abstract boolean isWorking();
+    
     protected abstract void submit();
     
     abstract void cancel();  
@@ -107,6 +81,12 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     {
       super(worker);
     }
+    
+    @Override
+    boolean isWorking()
+    {
+      return task != null && !task.isDone();
+    }
 
     @Override
     protected void submit()
@@ -118,9 +98,7 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       }
       Cache.log.debug(format("Worker %s queued",
               worker.getClass().getName()));
-      setState(QUEUED);
       task = executor.submit(() -> {
-        setState(RUNNING);
         try
         {
           Cache.log.debug(format("Worker %s started",
@@ -141,8 +119,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
         }
         finally
         {
-          // fixme: should not set to idle if another task is already queued for execution
-          setState(IDLE);
         }
       });
     }
@@ -150,18 +126,13 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     @Override
     synchronized void cancel()
     {
-      if (task == null || state == IDLE || state == CANCELLING)
+      if (!isWorking())
       {
         return;
       }
       Cache.log.debug(format("Cancelling worker %s",
               worker.getClass().getName()));
-      setState(CANCELLING);
       task.cancel(true);
-      if (task.isCancelled())
-      {
-        setState(IDLE);
-      }
     }
   }
   
@@ -177,6 +148,12 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       this.worker = worker;
     }
     
+    @Override
+    boolean isWorking()
+    {
+      return task != null && !task.isDone();
+    }
+    
     protected void submit()
     {
       if (task != null && !(task.isDone() || task.isCancelled()))
@@ -186,7 +163,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       }
       Cache.log.debug(format("Worker %s queued",
               worker.getClass().getName()));
-      setState(QUEUED);
       final var runnable = new Runnable()
       {
         private boolean started = false;
@@ -202,7 +178,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
             {
               Cache.log.debug(format("Worker %s started",
                       worker.getClass().getName()));
-              setState(RUNNING);
               worker.startUp();
               started = true;
             }
@@ -213,7 +188,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
                 Cache.log.debug(format("Worker %s finished",
                         worker.getClass().getName()));
                 completed = true;
-                setState(IDLE);
               }
             }
           } catch (Throwable th)
@@ -221,7 +195,6 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
             Cache.log.debug(format("Worker %s failed",
                     worker.getClass().getName()), th);
             completed = true;
-            setState(IDLE);
           }
           if (completed)
           {
@@ -242,18 +215,13 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     
     synchronized protected void cancel()
     {
-      if (task == null || state == IDLE || state == CANCELLING)
+      if (!isWorking())
       {
         return;
       }
       Cache.log.debug(format("Cancelling worker %s",
               worker.getClass().getName()));
-      setState(CANCELLING);
       task.cancel(false);
-      if (task.isCancelled())
-      {
-        setState(IDLE);
-      }
       executor.submit(() -> {
         worker.cancel();
       });
@@ -374,7 +342,7 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     }
     else
     {
-      return registered.get(worker).getState() == WorkerManager.RUNNING;
+      return registered.get(worker).isWorking();
     }
   }
 
@@ -386,7 +354,7 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       for (var entry : registered.entrySet())
       {
         if (entry.getKey().involves(annot) &&
-                entry.getValue().getState() == WorkerManager.RUNNING)
+                entry.getValue().isWorking())
         {
           return true;
         }
@@ -402,7 +370,7 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
     {
       for (var manager : registered.values())
       {
-        if (manager.getState() == WorkerManager.RUNNING)
+        if (manager.isWorking())
         {
           return true;
         }