From: Stephane Gamard <stephane.gamard@sonarsource.com>
Date: Fri, 29 Aug 2014 14:47:58 +0000 (+0200)
Subject: SONAR-4898 - fixed concurrent termination and exception on exit
X-Git-Tag: 4.5-RC1~30
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=9a20c5d4f9ce6183fb6f3182d5b793636f8a413e;p=sonarqube.git

SONAR-4898 - fixed concurrent termination and exception on exit
---

diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java b/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java
index ca227daa248..baa0f00791c 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java
@@ -122,6 +122,7 @@ public class Monitor extends Thread implements Terminable {
 
   @Override
   public synchronized void terminate() {
+
     LOGGER.debug("Monitoring thread is terminating");
 
     if (!monitorExecutionService.isShutdown()) {
@@ -131,9 +132,6 @@ public class Monitor extends Thread implements Terminable {
       watch.cancel(true);
     }
 
-    for (int i = processes.size() - 1; i >= 0; i--) {
-      processes.get(i).terminate();
-    }
     processes.clear();
     interruptAndWait();
   }
diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java b/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java
index c36df449b9e..c1c5d172c4a 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/MonitoredProcess.java
@@ -124,22 +124,24 @@ public abstract class MonitoredProcess implements ProcessMXBean {
 
   @Override
   public final void terminate() {
-    if (monitor != null) {
-      LOGGER.debug("Process[{}] terminating", name);
-      try {
-        doTerminate();
-      } catch (Exception e) {
-        LOGGER.error("Fail to terminate " + name, e);
-        // do not propagate exception
-      }
-      monitor.shutdownNow();
-      monitor = null;
-      if (pingTask != null) {
-        pingTask.cancel(true);
-        pingTask = null;
+    synchronized (monitor) {
+      if (monitor != null) {
+        LOGGER.debug("Process[{}] terminating", name);
+        try {
+          doTerminate();
+        } catch (Exception e) {
+          LOGGER.error("Fail to terminate " + name, e);
+          // do not propagate exception
+        }
+        monitor.shutdownNow();
+        monitor = null;
+        if (pingTask != null) {
+          pingTask.cancel(true);
+          pingTask = null;
+        }
+        LOGGER.debug("Process[{}] terminated", name);
+        terminated = true;
       }
-      LOGGER.debug("Process[{}] terminated", name);
-      terminated = true;
     }
   }
 
diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java
index c014122288c..5251ca5bd0c 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java
@@ -177,13 +177,11 @@ public class ProcessWrapper extends Thread implements Terminable {
         process.waitFor();
       }
     } catch (Exception e) {
-      LOGGER.info("ProcessThread has been interrupted. Killing process.");
+      LOGGER.info("ProcessThread has been interrupted. Killing node.");
       LOGGER.trace("Process exception", e);
     } finally {
-      waitUntilFinish(outputGobbler);
-      waitUntilFinish(errorGobbler);
-      ProcessUtils.closeStreams(process);
-      interruptAndWait();
+      ;
+      ;
     }
   }
 
@@ -195,16 +193,6 @@ public class ProcessWrapper extends Thread implements Terminable {
     return processMXBean;
   }
 
-  private void waitUntilFinish(@Nullable Thread thread) {
-    if (thread != null) {
-      try {
-        thread.join();
-      } catch (InterruptedException e) {
-        LOGGER.error("InterruptedException while waiting finish of " + thread.getName() + " in process '" + getName() + "'", e);
-      }
-    }
-  }
-
   private String buildJavaCommand() {
     String separator = System.getProperty("file.separator");
     return new File(new File(System.getProperty("java.home")),
@@ -269,37 +257,41 @@ public class ProcessWrapper extends Thread implements Terminable {
     return null;
   }
 
+  private final Integer terminationLock = new Integer(1);
   @Override
   public void terminate() {
-    if (processMXBean != null && process != null) {
-      LOGGER.info("{} stopping", getName());
-      // Send the terminate command to process in order to gracefully shutdown.
-      // Then hardly kill it if it didn't terminate in 30 seconds
-      ScheduledExecutorService killer = Executors.newScheduledThreadPool(1);
-      try {
-        Runnable killerTask = new Runnable() {
-          @Override
-          public void run() {
-            ProcessUtils.destroyQuietly(process);
-          }
-        };
-
-        ScheduledFuture killerFuture = killer.schedule(killerTask, 30, TimeUnit.SECONDS);
-        processMXBean.terminate();
-        killerFuture.cancel(true);
-        LOGGER.info("{} stopped", getName());
-
-      } catch (Exception ignored) {
-        LOGGER.trace("Could not terminate process", ignored);
-      } finally {
-        killer.shutdownNow();
+    synchronized (terminationLock) {
+      if (processMXBean != null && process != null) {
+        LOGGER.info("{} stopping", getName());
+        // Send the terminate command to process in order to gracefully shutdown.
+        // Then hardly kill it if it didn't terminate in 30 seconds
+        ScheduledExecutorService killer = Executors.newScheduledThreadPool(1);
+        try {
+          Runnable killerTask = new Runnable() {
+            @Override
+            public void run() {
+              ProcessUtils.destroyQuietly(process);
+            }
+          };
+
+          ScheduledFuture killerFuture = killer.schedule(killerTask, 30, TimeUnit.SECONDS);
+          processMXBean.terminate();
+          this.join();
+          killerFuture.cancel(true);
+          LOGGER.info("{} stopped", getName());
+
+        } catch (Exception ignored) {
+          LOGGER.trace("Could not terminate process", ignored);
+        } finally {
+          killer.shutdownNow();
+          interruptAndWait();
+        }
+      } else {
+        // process is not monitored through JMX, but killing it though
+        ProcessUtils.destroyQuietly(process);
       }
-      interruptAndWait();
-    } else {
-      // process is not monitored through JMX, but killing it though
-      ProcessUtils.destroyQuietly(process);
+      processMXBean = null;
     }
-    processMXBean = null;
   }
 
   public boolean waitForReady() throws InterruptedException {
@@ -323,8 +315,20 @@ public class ProcessWrapper extends Thread implements Terminable {
   }
 
   private void interruptAndWait() {
-    this.interrupt();
     try {
+      //after being interrupted, finalize the goblins
+      if (outputGobbler != null && outputGobbler.isAlive()) {
+        waitUntilFinish(outputGobbler);
+      }
+
+      if (errorGobbler != null && errorGobbler.isAlive()) {
+        waitUntilFinish(errorGobbler);
+      }
+      if (process != null) {
+        ProcessUtils.closeStreams(process);
+      }
+
+      //Join while the main thread terminates
       if (this.isAlive()) {
         this.join();
       }
@@ -333,6 +337,17 @@ public class ProcessWrapper extends Thread implements Terminable {
     }
   }
 
+  private void waitUntilFinish(@Nullable Thread thread) {
+    if (thread != null && thread.isAlive()) {
+      try {
+        thread.join();
+      } catch (InterruptedException e) {
+        LOGGER.error("InterruptedException while waiting finish of " + thread.getName() + " in process '" + getName() + "'", e);
+      }
+    }
+  }
+
+
   private static class StreamGobbler extends Thread {
     private final InputStream is;
     private final Logger logger;