do not call stop finalization code without checking lifeCycle state

this avoid having multiple thread call the stope finalization concurrently and killing each other, which is unpredictable and very hard to reproduce behavior

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 234239e5bec55ee56fad75e46ec40b8c99e7d422..508c73b5e0b4d73bf84986b7003be111201fb673 100644 (file)
--- 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
@@ -104,7 +104,7 @@ public class ManagedProcessHandler {
         hardStop();
       } else {
         // enforce stop and clean-up even if process has been quickly stopped
-        stopForcibly();
+        finalizeStop();
       }
     } else {
       // already stopping or stopped
@@ -123,7 +123,7 @@ public class ManagedProcessHandler {
         LOG.info("{} failed to stop in a quick fashion. Killing it.", processId.getKey());
       }
       // enforce stop and clean-up even if process has been quickly stopped
-      stopForcibly();
+      finalizeStop();
     } else {
       // already stopping or stopped
       waitForDown();
@@ -178,7 +178,7 @@ public class ManagedProcessHandler {
     return new InterruptedException(errorMessage);
   }
 
-  public void stopForcibly() {
+  private void finalizeStop() {
     interrupt(eventWatcher);
     interrupt(stopWatcher);
     if (process != null) {
@@ -253,7 +253,15 @@ public class ManagedProcessHandler {
         Thread.currentThread().interrupt();
         // stop watching process
       }
-      stopForcibly();
+      // since process is already stopped, this will only finalize the stop sequence
+      // call hardStop() rather than finalizeStop() directly because hardStop() checks lifeCycle state and this
+      // avoid running to concurrent stop finalization pieces of code
+      try {
+        hardStop();
+      } catch (InterruptedException e) {
+        LOG.debug("Interrupted while stopping [{}] after process ended", processId.getKey(), e);
+        Thread.currentThread().interrupt();
+      }
     }
   }
 

diff --git a/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessHandlerTest.java b/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessHandlerTest.java
index 467c6bfbc55b9e5bc8ca0c024198599d84d6818b..5b18ee1d7cb049471f15d462177790656840ad86 100644 (file)
--- a/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessHandlerTest.java
+++ b/server/sonar-main/src/test/java/org/sonar/application/process/ManagedProcessHandlerTest.java
@@ -173,24 +173,6 @@ public class ManagedProcessHandlerTest {
     }
   }
 
-  @Test
-  public void stopForcibly_stops_the_process_without_graceful_request_for_stop() {
-    ManagedProcessHandler underTest = newHanderBuilder(A_PROCESS_ID).build();
-
-    try (TestManagedProcess testProcess = new TestManagedProcess()) {
-      underTest.start(() -> testProcess);
-
-      underTest.stopForcibly();
-      assertThat(underTest.getState()).isEqualTo(ManagedProcessLifecycle.State.STOPPED);
-      assertThat(testProcess.askedForHardStop).isFalse();
-      assertThat(testProcess.destroyedForcibly).isTrue();
-
-      // second execution of stopForcibly does nothing. It's still stopped.
-      underTest.stopForcibly();
-      assertThat(underTest.getState()).isEqualTo(ManagedProcessLifecycle.State.STOPPED);
-    }
-  }
-
   @Test
   public void process_stops_after_graceful_request_for_stop() throws Exception {
     ProcessLifecycleListener listener = mock(ProcessLifecycleListener.class);