SONAR-12043 make Lifecycle threadsafe

and make ProcessEntryPointTest#launch_then_request_graceful_stop stable
and add trace log when some code tests the current state of Lifecyle (#getState() replaced by isCurrentState(State))

2 files changed, 39 insertions, 24 deletions

diff --git a/server/sonar-process/src/main/java/org/sonar/process/Lifecycle.java b/server/sonar-process/src/main/java/org/sonar/process/Lifecycle.java
index 506c63e92f8..31bd4f683e1 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/Lifecycle.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/Lifecycle.java
@@ -25,6 +25,7 @@ import java.util.EnumMap;
 import java.util.EnumSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -47,7 +48,7 @@ public class Lifecycle {
 
   private static final Map<State, Set<State>> TRANSITIONS = buildTransitions();
 
-  private State state = INIT;
+  private AtomicReference<State> state = new AtomicReference<>(INIT);
 
   private static Map<State, Set<State>> buildTransitions() {
     Map<State, Set<State>> res = new EnumMap<>(State.class);
@@ -72,19 +73,26 @@ public class Lifecycle {
     return EnumSet.copyOf(Arrays.asList(states));
   }
 
-  public State getState() {
-    return state;
+  public boolean isCurrentState(State candidateState) {
+    State currentState = this.state.get();
+    boolean res = currentState == candidateState;
+    LOG.trace("isCurrentState({}): {} ({})", candidateState, res, currentState);
+    return res;
   }
 
-  public synchronized boolean tryToMoveTo(State to) {
-    boolean res = false;
-    State currentState = state;
-    if (TRANSITIONS.get(currentState).contains(to)) {
-      this.state = to;
-      res = true;
-    }
-    LOG.trace("tryToMoveTo from {} to {} => {}", currentState, to, res);
-    return res;
+  public boolean tryToMoveTo(State to) {
+    AtomicReference<State> lastFrom = new AtomicReference<>();
+    State newState = this.state.updateAndGet(from -> {
+      lastFrom.set(from);
+      if (TRANSITIONS.get(from).contains(to)) {
+        return to;
+      }
+      return from;
+    });
+
+    boolean updated = newState == to && lastFrom.get() != to;
+    LOG.trace("tryToMoveTo from {} to {} => {}", lastFrom.get(), to, updated);
+    return updated;
   }
 
   @Override
@@ -96,11 +104,11 @@ public class Lifecycle {
       return false;
     }
     Lifecycle lifecycle = (Lifecycle) o;
-    return state == lifecycle.state;
+    return state.get() == lifecycle.state.get();
   }
 
   @Override
   public int hashCode() {
-    return state.hashCode();
+    return state.get().hashCode();
   }
 }
diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java
index 1976f5699b4..5de9194cc39 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java
@@ -29,6 +29,7 @@ import org.sonar.process.sharedmemoryfile.ProcessCommands;
 
 import static java.util.Optional.of;
 import static java.util.Optional.ofNullable;
+import static org.sonar.process.Lifecycle.State.STOPPED;
 
 public class ProcessEntryPoint {
 
@@ -108,6 +109,7 @@ public class ProcessEntryPoint {
     } catch (Exception e) {
       logger.warn("Fail to start {}", getKey(), e);
     } finally {
+      logger.trace("Hard stopping to clean any resource...");
       hardStop();
     }
   }
@@ -133,6 +135,7 @@ public class ProcessEntryPoint {
         monitored.awaitStop();
       }
     } else {
+      logger.trace("Timeout waiting for process to go UP or OPERATIONAL. Hard stopping...");
       hardStop();
     }
   }
@@ -157,7 +160,7 @@ public class ProcessEntryPoint {
   }
 
   boolean isStarted() {
-    return lifecycle.getState() == Lifecycle.State.STARTED;
+    return lifecycle.isCurrentState(Lifecycle.State.STARTED);
   }
 
   void stop() {
@@ -166,7 +169,6 @@ public class ProcessEntryPoint {
         try {
           // join() does nothing if thread already finished
           stoppingThread.join();
-          lifecycle.tryToMoveTo(Lifecycle.State.STOPPED);
           commands.endWatch();
           exit.exit(0);
         } catch (InterruptedException e) {
@@ -179,7 +181,7 @@ public class ProcessEntryPoint {
   private Optional<StopperThread> stopAsync() {
     if (lifecycle.tryToMoveTo(Lifecycle.State.STOPPING)) {
       long terminationTimeoutMs = Long.parseLong(props.nonNullValue(PROPERTY_TERMINATION_TIMEOUT_MS));
-      stopperThread = new StopperThread(monitored, terminationTimeoutMs);
+      stopperThread = new StopperThread(monitored, lifecycle, terminationTimeoutMs);
       stopperThread.start();
       stopWatcher.stopWatching();
       return of(stopperThread);
@@ -197,7 +199,6 @@ public class ProcessEntryPoint {
         try {
           // join() does nothing if thread already finished
           stoppingThread.join();
-          lifecycle.tryToMoveTo(Lifecycle.State.STOPPED);
         } catch (InterruptedException e) {
           Thread.currentThread().interrupt();
         }
@@ -209,7 +210,7 @@ public class ProcessEntryPoint {
   private Optional<HardStopperThread> hardStopAsync() {
     if (lifecycle.tryToMoveTo(Lifecycle.State.HARD_STOPPING)) {
       long terminationTimeoutMs = Long.parseLong(props.nonNullValue(PROPERTY_TERMINATION_TIMEOUT_MS));
-      hardStopperThread = new HardStopperThread(monitored, terminationTimeoutMs, stopperThread);
+      hardStopperThread = new HardStopperThread(monitored, lifecycle, terminationTimeoutMs, stopperThread);
       hardStopperThread.start();
       hardStopWatcher.stopWatching();
       stopWatcher.stopWatching();
@@ -219,8 +220,8 @@ public class ProcessEntryPoint {
     return ofNullable(hardStopperThread);
   }
 
-  Lifecycle.State getState() {
-    return lifecycle.getState();
+  boolean isCurrentState(Lifecycle.State candidateState) {
+    return lifecycle.isCurrentState(candidateState);
   }
 
   Thread getShutdownHook() {
@@ -282,8 +283,13 @@ public class ProcessEntryPoint {
    */
   private static class StopperThread extends AbstractStopperThread {
 
-    private StopperThread(Monitored monitored, long terminationTimeoutMs) {
-      super("Stopper", monitored::stop, terminationTimeoutMs);
+    private StopperThread(Monitored monitored, Lifecycle lifecycle, long terminationTimeoutMs) {
+      super("Stopper", () -> {
+        if (!lifecycle.isCurrentState(STOPPED)) {
+          monitored.stop();
+          lifecycle.tryToMoveTo(STOPPED);
+        }
+      }, terminationTimeoutMs);
     }
 
   }
@@ -293,7 +299,7 @@ public class ProcessEntryPoint {
    */
   private static class HardStopperThread extends AbstractStopperThread {
 
-    private HardStopperThread(Monitored monitored, long terminationTimeoutMs, @Nullable StopperThread stopperThread) {
+    private HardStopperThread(Monitored monitored, Lifecycle lifecycle, long terminationTimeoutMs, @Nullable StopperThread stopperThread) {
       super(
         "HardStopper",
         () -> {
@@ -301,6 +307,7 @@ public class ProcessEntryPoint {
             stopperThread.stopIt();
           }
           monitored.hardStop();
+          lifecycle.tryToMoveTo(STOPPED);
         },
         terminationTimeoutMs);
     }