protect ManagedProcessHandler stop finalization code with lifecycle state

since this code is interrupting other threads, this ensures concurrent threads attempting to run this code will never interrupt each other
leading to unpredictable and unreproducible behavior

3 files changed, 99 insertions, 44 deletions

diff --git a/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessHandler.java b/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessHandler.java
index 508c73b5e0b..14da55a2ef8 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessHandler.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessHandler.java
@@ -73,7 +73,8 @@ public class ManagedProcessHandler {
       this.process = commandLauncher.get();
     } catch (RuntimeException e) {
       LOG.error("Fail to launch process [{}]", processId.getKey(), e);
-      lifecycle.tryToMoveTo(ManagedProcessLifecycle.State.STOPPED);
+      lifecycle.tryToMoveTo(ManagedProcessLifecycle.State.STOPPING);
+      finalizeStop();
       throw e;
     }
     this.stdOutGobbler = new StreamGobbler(process.getInputStream(), processId.getKey());
@@ -179,6 +180,10 @@ public class ManagedProcessHandler {
   }
 
   private void finalizeStop() {
+    if (!lifecycle.tryToMoveTo(ManagedProcessLifecycle.State.FINALIZE_STOPPING)) {
+      return;
+    }
+
     interrupt(eventWatcher);
     interrupt(stopWatcher);
     if (process != null) {
diff --git a/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessLifecycle.java b/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessLifecycle.java
index ef4f6d07d0c..7583ddb1450 100644
--- a/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessLifecycle.java
+++ b/server/sonar-main/src/main/java/org/sonar/application/process/ManagedProcessLifecycle.java
@@ -30,6 +30,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.process.ProcessId;
 
+import static org.sonar.application.process.ManagedProcessLifecycle.State.FINALIZE_STOPPING;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.HARD_STOPPING;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.INIT;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.STARTED;
@@ -40,7 +41,7 @@ import static org.sonar.application.process.ManagedProcessLifecycle.State.STOPPI
 public class ManagedProcessLifecycle {
 
   public enum State {
-    INIT, STARTING, STARTED, STOPPING, HARD_STOPPING, STOPPED
+    INIT, STARTING, STARTED, STOPPING, HARD_STOPPING, FINALIZE_STOPPING, STOPPED
   }
 
   private static final Logger LOG = LoggerFactory.getLogger(ManagedProcessLifecycle.class);
@@ -51,22 +52,19 @@ public class ManagedProcessLifecycle {
   private State state;
 
   public ManagedProcessLifecycle(ProcessId processId, List<ProcessLifecycleListener> listeners) {
-    this(processId, listeners, INIT);
-  }
-
-  ManagedProcessLifecycle(ProcessId processId, List<ProcessLifecycleListener> listeners, State initialState) {
     this.processId = processId;
     this.listeners = listeners;
-    this.state = initialState;
+    this.state = INIT;
   }
 
   private static Map<State, Set<State>> buildTransitions() {
     Map<State, Set<State>> res = new EnumMap<>(State.class);
     res.put(INIT, toSet(STARTING));
-    res.put(STARTING, toSet(STARTED, STOPPING, HARD_STOPPING, STOPPED));
-    res.put(STARTED, toSet(STOPPING, HARD_STOPPING, STOPPED));
-    res.put(STOPPING, toSet(HARD_STOPPING, STOPPED));
-    res.put(HARD_STOPPING, toSet(STOPPED));
+    res.put(STARTING, toSet(STARTED, STOPPING, HARD_STOPPING));
+    res.put(STARTED, toSet(STOPPING, HARD_STOPPING));
+    res.put(STOPPING, toSet(HARD_STOPPING, FINALIZE_STOPPING));
+    res.put(HARD_STOPPING, toSet(FINALIZE_STOPPING));
+    res.put(FINALIZE_STOPPING, toSet(STOPPED));
     res.put(STOPPED, toSet());
     return Collections.unmodifiableMap(res);
   }
diff --git a/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessLifecycleTest.java b/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessLifecycleTest.java
index a2e6ff0fb9b..ade75608b69 100644
--- a/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessLifecycleTest.java
+++ b/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessLifecycleTest.java
@@ -19,30 +19,46 @@
  */
 package org.sonar.application.process;
 
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
+import java.util.function.Predicate;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.sonar.process.ProcessId;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.application.process.ManagedProcessLifecycle.State;
+import static org.sonar.application.process.ManagedProcessLifecycle.State.FINALIZE_STOPPING;
+import static org.sonar.application.process.ManagedProcessLifecycle.State.HARD_STOPPING;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.INIT;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.STARTED;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.STARTING;
-import static org.sonar.application.process.ManagedProcessLifecycle.State.HARD_STOPPING;
+import static org.sonar.application.process.ManagedProcessLifecycle.State.STOPPED;
 import static org.sonar.application.process.ManagedProcessLifecycle.State.STOPPING;
 
+@RunWith(DataProviderRunner.class)
 public class ManagedProcessLifecycleTest {
 
   @Test
   public void initial_state_is_INIT() {
-    ManagedProcessLifecycle lifecycle = new ManagedProcessLifecycle(ProcessId.ELASTICSEARCH, Collections.emptyList());
+    ManagedProcessLifecycle lifecycle = new ManagedProcessLifecycle(ProcessId.ELASTICSEARCH, emptyList());
     assertThat(lifecycle.getState()).isEqualTo(INIT);
   }
 
   @Test
+  public void listeners_are_not_notified_of_INIT_state() {
+    TestLifeCycleListener listener = new TestLifeCycleListener();
+    new ManagedProcessLifecycle(ProcessId.ELASTICSEARCH, Arrays.asList(listener));
+    assertThat(listener.states).isEmpty();
+  }
+
+  @Test
   public void try_to_move_does_not_support_jumping_states() {
     TestLifeCycleListener listener = new TestLifeCycleListener();
     ManagedProcessLifecycle lifecycle = new ManagedProcessLifecycle(ProcessId.ELASTICSEARCH, asList(listener));
@@ -59,51 +75,87 @@ public class ManagedProcessLifecycleTest {
   }
 
   @Test
-  public void no_state_can_not_move_to_itself() {
-    for (ManagedProcessLifecycle.State state : ManagedProcessLifecycle.State.values()) {
-      assertThat(newLifeCycle(state).tryToMoveTo(state)).isFalse();
-    }
+  @UseDataProvider("allStates")
+  public void no_state_can_not_move_to_itself(State state) {
+    assertThat(newLifeCycle(state).tryToMoveTo(state)).isFalse();
+  }
+
+  @Test
+  @UseDataProvider("allStates")
+  public void can_move_to_HARD_STOPPING_from_STARTING_AND_STARTED_and_STOPPING_only(State state) {
+    verifyCanMoveTo(t -> t == STARTING || t == STARTED || t == STOPPING,
+      state, HARD_STOPPING);
   }
 
   @Test
-  public void can_move_to_STOPPING_from_STARTING_STARTED_and_STOPPING_only() {
-    for (ManagedProcessLifecycle.State state : ManagedProcessLifecycle.State.values()) {
-      TestLifeCycleListener listener = new TestLifeCycleListener();
-      boolean tryToMoveTo = newLifeCycle(state, listener).tryToMoveTo(HARD_STOPPING);
-      if (state == STARTING || state == STARTED || state == STOPPING) {
-        assertThat(tryToMoveTo).as("from state " + state).isTrue();
-        assertThat(listener.states).containsOnly(HARD_STOPPING);
-      } else {
-        assertThat(tryToMoveTo).as("from state " + state).isFalse();
-        assertThat(listener.states).isEmpty();
-      }
+  @UseDataProvider("allStates")
+  public void can_move_to_STARTED_from_STARTING_only(State state) {
+    verifyCanMoveTo(t -> t == STARTING, state, STARTED);
+  }
+
+  private void verifyCanMoveTo(Predicate<State> isAllowed, State from, State to) {
+    TestLifeCycleListener listener = new TestLifeCycleListener();
+    ManagedProcessLifecycle underTest = newLifeCycle(from, listener);
+    boolean tryToMoveTo = underTest.tryToMoveTo(to);
+    if (isAllowed.test(from)) {
+      assertThat(tryToMoveTo).isTrue();
+    } else {
+      assertThat(tryToMoveTo).isFalse();
     }
   }
 
   @Test
-  public void can_move_to_STARTED_from_STARTING_only() {
-    for (ManagedProcessLifecycle.State state : ManagedProcessLifecycle.State.values()) {
-      TestLifeCycleListener listener = new TestLifeCycleListener();
-      boolean tryToMoveTo = newLifeCycle(state, listener).tryToMoveTo(STARTED);
-      if (state == STARTING) {
-        assertThat(tryToMoveTo).as("from state " + state).isTrue();
-        assertThat(listener.states).containsOnly(STARTED);
-      } else {
-        assertThat(tryToMoveTo).as("from state " + state).isFalse();
-        assertThat(listener.states).isEmpty();
-      }
+  @UseDataProvider("allStates")
+  public void cannot_move_to_any_state_from_STOPPED(State state) {
+    assertThat(newLifeCycle(STOPPED).tryToMoveTo(state)).isFalse();
+  }
+
+  private static ManagedProcessLifecycle newLifeCycle(State from, State to, TestLifeCycleListener... listeners) {
+    ManagedProcessLifecycle lifecycle = newLifeCycle(from, listeners);
+    assertThat(lifecycle.tryToMoveTo(to)).isTrue();
+    assertThat(lifecycle.getState()).isEqualTo(to);
+    Arrays.stream(listeners)
+      .forEach(t -> assertThat(t.states).endsWith(to));
+    return lifecycle;
+  }
+
+  /**
+   * Creates a Lifecycle object in the specified state emulating that the shortest path to this state has been followed
+   * to reach it.
+   */
+  private static ManagedProcessLifecycle newLifeCycle(State state, TestLifeCycleListener... listeners) {
+    switch (state) {
+      case INIT:
+        return new ManagedProcessLifecycle(ProcessId.ELASTICSEARCH, asList(listeners));
+      case STARTING:
+        return newLifeCycle(INIT, state, listeners);
+      case STARTED:
+        return newLifeCycle(STARTING, state, listeners);
+      case STOPPING:
+        return newLifeCycle(STARTED, state, listeners);
+      case HARD_STOPPING:
+        return newLifeCycle(STARTED, state, listeners);
+      case FINALIZE_STOPPING:
+        return newLifeCycle(STOPPING, state, listeners);
+      case STOPPED:
+        return newLifeCycle(FINALIZE_STOPPING, state, listeners);
+      default:
+        throw new IllegalStateException("new state is not supported:" + state);
     }
   }
 
-  private static ManagedProcessLifecycle newLifeCycle(ManagedProcessLifecycle.State state, TestLifeCycleListener... listeners) {
-    return new ManagedProcessLifecycle(ProcessId.ELASTICSEARCH, Arrays.asList(listeners), state);
+  @DataProvider
+  public static Object[][] allStates() {
+    return Arrays.stream(State.values())
+      .map(t -> new Object[] {t})
+      .toArray(Object[][]::new);
   }
 
   private static final class TestLifeCycleListener implements ProcessLifecycleListener {
-    private final List<ManagedProcessLifecycle.State> states = new ArrayList<>();
+    private final List<State> states = new ArrayList<>();
 
     @Override
-    public void onProcessState(ProcessId processId, ManagedProcessLifecycle.State state) {
+    public void onProcessState(ProcessId processId, State state) {
       this.states.add(state);
     }
   }