SONAR-7168 fileSystem must be reset during a restart

deletion of temp directory is required for uninstalling plugins but also to cleanly start web server (eg. deleting unsuccessfully processed analysis reports)

5 files changed, 166 insertions, 68 deletions

diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/FileSystem.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/FileSystem.java
new file mode 100644
index 00000000000..4efe53bf489
--- /dev/null
+++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/FileSystem.java
@@ -0,0 +1,29 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.process.monitor;
+
+import java.io.File;
+import java.io.IOException;
+
+public interface FileSystem {
+  void reset() throws IOException;
+
+  File getTempDir();
+}
diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java
index 2f9490b1844..019d6388cd9 100644
--- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java
+++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/JavaProcessLauncher.java
@@ -39,16 +39,20 @@ public class JavaProcessLauncher {
   private final File tempDir;
   private final AllProcessesCommands allProcessesCommands;
 
-  public JavaProcessLauncher(Timeouts timeouts, File tempDir, AllProcessesCommands allProcessesCommands) {
+  public JavaProcessLauncher(Timeouts timeouts, File tempDir) {
     this.timeouts = timeouts;
     this.tempDir = tempDir;
-    this.allProcessesCommands = allProcessesCommands;
+    this.allProcessesCommands = new AllProcessesCommands(tempDir);
   }
 
   public void close() {
     allProcessesCommands.close();
   }
 
+  public ProcessCommands getProcessCommand(int processNumber, boolean clean) {
+    return allProcessesCommands.getProcessCommand(processNumber, clean);
+  }
+
   ProcessRef launch(JavaCommand command) {
     Process process = null;
     try {
diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java
index 8c3e459106f..481495b7d14 100644
--- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java
+++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java
@@ -20,6 +20,7 @@
 package org.sonar.process.monitor;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -28,7 +29,6 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.sonar.process.AllProcessesCommands;
 import org.sonar.process.Lifecycle;
 import org.sonar.process.Lifecycle.State;
 import org.sonar.process.ProcessCommands;
@@ -43,32 +43,34 @@ public class Monitor {
 
   private static int restartorInstanceCounter = 0;
 
-  private final AllProcessesCommands allProcessesCommands;
-  private final JavaProcessLauncher launcher;
-
+  private final FileSystem fileSystem;
   private final SystemExit systemExit;
+  private final boolean watchForHardStop;
   private final Thread shutdownHook = new Thread(new MonitorShutdownHook(), "Monitor Shutdown Hook");
 
   private final List<WatcherThread> watcherThreads = new CopyOnWriteArrayList<>();
   private final Lifecycle lifecycle = new Lifecycle();
+
   private final TerminatorThread terminator = new TerminatorThread();
   private final RestartRequestWatcherThread restartWatcher = new RestartRequestWatcherThread();
   @CheckForNull
   private List<JavaCommand> javaCommands;
   @CheckForNull
+  private JavaProcessLauncher launcher;
+  @CheckForNull
   private RestartorThread restartor;
   @CheckForNull
   HardStopWatcherThread hardStopWatcher;
   static int nextProcessId = 1;
 
-  Monitor(File tempDir, SystemExit exit) {
-    this.allProcessesCommands = new AllProcessesCommands(tempDir);
-    this.launcher = new JavaProcessLauncher(TIMEOUTS, tempDir, allProcessesCommands);
+  Monitor(FileSystem fileSystem, SystemExit exit, boolean watchForHardStop) {
+    this.fileSystem = fileSystem;
     this.systemExit = exit;
+    this.watchForHardStop = watchForHardStop;
   }
 
-  public static Monitor create(File tempDir) {
-    return new Monitor(tempDir, new SystemExit());
+  public static Monitor create(FileSystem fileSystem, boolean watchForHardStop) {
+    return new Monitor(fileSystem, new SystemExit(), watchForHardStop);
   }
 
   /**
@@ -88,6 +90,8 @@ public class Monitor {
 
     // intercepts CTRL-C
     Runtime.getRuntime().addShutdownHook(shutdownHook);
+
+    // start watching for restart requested by child process
     this.restartWatcher.start();
 
     this.javaCommands = commands;
@@ -97,12 +101,42 @@ public class Monitor {
   private void startProcesses() {
     // do no start any child process if not in state INIT or RESTARTING (a stop could be in progress too)
     if (lifecycle.tryToMoveTo(State.STARTING)) {
+      resetFileSystem();
+
       startAndMonitorProcesses();
       stopIfAnyProcessDidNotStart();
+      watchForHardStop();
+    }
+  }
+
+  private void watchForHardStop() {
+    if (!watchForHardStop) {
+      return;
+    }
+
+    if (this.hardStopWatcher != null) {
+      this.hardStopWatcher.stopWatching();
+      awaitTermination(this.hardStopWatcher);
+    }
+    ProcessCommands processCommand = this.launcher.getProcessCommand(CURRENT_PROCESS_NUMBER, true);
+    this.hardStopWatcher = new HardStopWatcherThread(processCommand);
+    this.hardStopWatcher.start();
+  }
+
+  private void resetFileSystem() {
+    // since JavaLauncher depends on temp directory, which is reset below, we need to close it first
+    closeJavaLauncher();
+    try {
+      fileSystem.reset();
+    } catch (IOException e) {
+      // failed to reset FileSystem
+      throw new RuntimeException("Failed to reset file system", e);
     }
   }
 
   private void startAndMonitorProcesses() {
+    File tempDir = fileSystem.getTempDir();
+    this.launcher = new JavaProcessLauncher(TIMEOUTS, tempDir);
     for (JavaCommand command : javaCommands) {
       try {
         ProcessRef processRef = launcher.launch(command);
@@ -115,10 +149,11 @@ public class Monitor {
     }
   }
 
-  public void watchForHardStop() {
-    ProcessCommands processCommand = this.allProcessesCommands.getProcessCommand(CURRENT_PROCESS_NUMBER, true);
-    this.hardStopWatcher = new HardStopWatcherThread(processCommand);
-    this.hardStopWatcher.start();
+  private void closeJavaLauncher() {
+    if (this.launcher != null) {
+      this.launcher.close();
+      this.launcher = null;
+    }
   }
 
   private void monitor(ProcessRef processRef) {
@@ -213,7 +248,7 @@ public class Monitor {
         Runtime.getRuntime().removeShutdownHook(shutdownHook);
       }
       // cleanly close JavaLauncher
-      launcher.close();
+      closeJavaLauncher();
     }
   }
 
@@ -227,6 +262,7 @@ public class Monitor {
   }
 
   private void stopAsync(State stoppingState) {
+    assert stoppingState == State.STOPPING || stoppingState == State.HARD_STOPPING;
     if (lifecycle.tryToMoveTo(stoppingState)) {
       terminator.start();
     }
@@ -310,6 +346,7 @@ public class Monitor {
 
   public class HardStopWatcherThread extends Thread {
     private final ProcessCommands processCommands;
+    private boolean watch = true;
 
     public HardStopWatcherThread(ProcessCommands processCommands) {
       super("Hard stop watcher");
@@ -318,7 +355,7 @@ public class Monitor {
 
     @Override
     public void run() {
-      while (lifecycle.getState() != Lifecycle.State.STOPPED) {
+      while (watch && lifecycle.getState() != Lifecycle.State.STOPPED) {
         if (processCommands.askedForStop()) {
           trace("Stopping process");
           Monitor.this.stop();
@@ -332,6 +369,9 @@ public class Monitor {
       }
     }
 
+    public void stopWatching() {
+      this.watch = false;
+    }
   }
 
   private void stopProcesses() {
diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java
index 20053415c15..af32f3f759b 100644
--- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java
+++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/JavaProcessLauncherTest.java
@@ -23,7 +23,6 @@ import java.io.File;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
-import org.sonar.process.AllProcessesCommands;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
@@ -37,7 +36,7 @@ public class JavaProcessLauncherTest {
   public void fail_to_launch() throws Exception {
     File tempDir = temp.newFolder();
     JavaCommand command = new JavaCommand("test");
-    JavaProcessLauncher launcher = new JavaProcessLauncher(new Timeouts(), tempDir, new AllProcessesCommands(tempDir));
+    JavaProcessLauncher launcher = new JavaProcessLauncher(new Timeouts(), tempDir);
     try {
       // command is not correct (missing options), java.lang.ProcessBuilder#start()
       // throws an exception
diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java
index 713ace625dc..ea25a399cfe 100644
--- a/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java
+++ b/server/sonar-process-monitor/src/test/java/org/sonar/process/monitor/MonitorTest.java
@@ -45,16 +45,23 @@ import org.sonar.process.NetworkUtils;
 import org.sonar.process.ProcessCommands;
 import org.sonar.process.SystemExit;
 
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 import static org.sonar.process.monitor.MonitorTest.HttpProcessClientAssert.assertThat;
 
 public class MonitorTest {
 
-  static File testJar;
-  Monitor monitor;
-  SystemExit exit = mock(SystemExit.class);
+  private static File testJar;
+
+  private FileSystem fileSystem = mock(FileSystem.class);
+  private SystemExit exit = mock(SystemExit.class);
+
+  private Monitor underTest;
 
   /**
    * Find the JAR file containing the test apps. Classes can't be moved in sonar-process-monitor because
@@ -105,8 +112,8 @@ public class MonitorTest {
   @After
   public void tearDown() {
     try {
-      if (monitor != null) {
-        monitor.stop();
+      if (underTest != null) {
+        underTest.stop();
       }
     } catch (Throwable ignored) {
     }
@@ -114,9 +121,9 @@ public class MonitorTest {
 
   @Test
   public void fail_to_start_if_no_commands() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     try {
-      monitor.start(Collections.<JavaCommand>emptyList());
+      underTest.start(Collections.<JavaCommand>emptyList());
       fail();
     } catch (IllegalArgumentException e) {
       assertThat(e).hasMessage("At least one command is required");
@@ -125,51 +132,52 @@ public class MonitorTest {
 
   @Test
   public void fail_to_start_multiple_times() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
-    monitor.start(Arrays.asList(newStandardProcessCommand()));
+    underTest = newDefaultMonitor(tempDir);
+    underTest.start(singletonList(newStandardProcessCommand()));
     boolean failed = false;
     try {
-      monitor.start(Arrays.asList(newStandardProcessCommand()));
+      underTest.start(singletonList(newStandardProcessCommand()));
     } catch (IllegalStateException e) {
       failed = e.getMessage().equals("Can not start multiple times");
     }
-    monitor.stop();
+    underTest.stop();
     assertThat(failed).isTrue();
   }
 
   @Test
   public void start_then_stop_gracefully() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     HttpProcessClient client = new HttpProcessClient(tempDir, "test");
     // blocks until started
-    monitor.start(Arrays.asList(client.newCommand()));
+    underTest.start(singletonList(client.newCommand()));
 
     assertThat(client).isReady()
       .wasStartedBefore(System.currentTimeMillis());
 
     // blocks until stopped
-    monitor.stop();
+    underTest.stop();
     assertThat(client)
       .isNotReady()
       .wasGracefullyTerminated();
-    assertThat(monitor.getState()).isEqualTo(State.STOPPED);
+    assertThat(underTest.getState()).isEqualTo(State.STOPPED);
+    verify(fileSystem).reset();
   }
 
   @Test
   public void start_then_stop_sequence_of_commands() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1");
     HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2");
-    monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
+    underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
 
     // start p2 when p1 is fully started (ready)
     assertThat(p1)
       .isReady()
       .wasStartedBefore(p2);
     assertThat(p2)
-        .isReady();
+      .isReady();
 
-    monitor.stop();
+    underTest.stop();
 
     // stop in inverse order
     assertThat(p1)
@@ -178,68 +186,73 @@ public class MonitorTest {
     assertThat(p2)
       .isNotReady()
       .wasGracefullyTerminatedBefore(p1);
+    verify(fileSystem).reset();
   }
 
   @Test
   public void stop_all_processes_if_monitor_shutdowns() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1");
     HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2");
-    monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
+    underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
     assertThat(p1).isReady();
     assertThat(p2).isReady();
 
     // emulate CTRL-C
-    monitor.getShutdownHook().run();
-    monitor.getShutdownHook().join();
+    underTest.getShutdownHook().run();
+    underTest.getShutdownHook().join();
 
     assertThat(p1).wasGracefullyTerminated();
     assertThat(p2).wasGracefullyTerminated();
+
+    verify(fileSystem).reset();
   }
 
   @Test
   public void restart_all_processes_if_one_asks_for_restart() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1");
     HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2");
-    monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
+    underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
 
     assertThat(p1).isReady();
     assertThat(p2).isReady();
 
     p2.restart();
 
-    assertThat(monitor.waitForOneRestart()).isTrue();
+    assertThat(underTest.waitForOneRestart()).isTrue();
 
     assertThat(p1)
-        .wasStarted(2)
-        .wasGracefullyTerminated(1);
+      .wasStarted(2)
+      .wasGracefullyTerminated(1);
     assertThat(p2)
-        .wasStarted(2)
-        .wasGracefullyTerminated(1);
+      .wasStarted(2)
+      .wasGracefullyTerminated(1);
 
-    monitor.stop();
+    underTest.stop();
 
     assertThat(p1)
-        .wasStarted(2)
-        .wasGracefullyTerminated(2);
+      .wasStarted(2)
+      .wasGracefullyTerminated(2);
     assertThat(p2)
-        .wasStarted(2)
-        .wasGracefullyTerminated(2);
+      .wasStarted(2)
+      .wasGracefullyTerminated(2);
+
+    verify(fileSystem, times(2)).reset();
   }
 
   @Test
   public void stop_all_processes_if_one_shutdowns() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1");
     HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2");
-    monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
+    underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
     assertThat(p1.isReady()).isTrue();
     assertThat(p2.isReady()).isTrue();
 
     // kill p1 -> waiting for detection by monitor than termination of p2
     p1.kill();
-    monitor.awaitTermination();
+    underTest.awaitTermination();
 
     assertThat(p1)
       .isNotReady()
@@ -247,15 +260,17 @@ public class MonitorTest {
     assertThat(p2)
       .isNotReady()
       .wasGracefullyTerminated();
+
+    verify(fileSystem).reset();
   }
 
   @Test
   public void stop_all_processes_if_one_fails_to_start() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1");
     HttpProcessClient p2 = new HttpProcessClient(tempDir, "p2", -1);
     try {
-      monitor.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
+      underTest.start(Arrays.asList(p1.newCommand(), p2.newCommand()));
       fail();
     } catch (Exception expected) {
       assertThat(p1)
@@ -283,13 +298,13 @@ public class MonitorTest {
 
   @Test
   public void fail_to_start_if_bad_class_name() throws Exception {
-    monitor = newDefaultMonitor(tempDir);
+    underTest = newDefaultMonitor(tempDir);
     JavaCommand command = new JavaCommand("test")
       .addClasspath(testJar.getAbsolutePath())
       .setClassName("org.sonar.process.test.Unknown");
 
     try {
-      monitor.start(Arrays.asList(command));
+      underTest.start(singletonList(command));
       fail();
     } catch (Exception e) {
       // expected
@@ -299,17 +314,28 @@ public class MonitorTest {
 
   @Test
   public void watchForHardStop_adds_a_hardStopWatcher_thread_and_starts_it() throws Exception {
-    Monitor monitor = newDefaultMonitor(tempDir);
-    assertThat(monitor.hardStopWatcher).isNull();
+    underTest = newDefaultMonitor(tempDir, true);
+    assertThat(underTest.hardStopWatcher).isNull();
 
-    monitor.watchForHardStop();
+    HttpProcessClient p1 = new HttpProcessClient(tempDir, "p1");
+    underTest.start(singletonList(p1.newCommand()));
 
-    assertThat(monitor.hardStopWatcher).isNotNull();
-    assertThat(monitor.hardStopWatcher.isAlive()).isTrue();
+    assertThat(underTest.hardStopWatcher).isNotNull();
+    assertThat(underTest.hardStopWatcher.isAlive()).isTrue();
+
+    p1.kill();
+    underTest.awaitTermination();
+
+    assertThat(underTest.hardStopWatcher.isAlive()).isFalse();
   }
 
   private Monitor newDefaultMonitor(File tempDir) throws IOException {
-    return new Monitor(tempDir, exit);
+    return newDefaultMonitor(tempDir, false);
+  }
+
+  private Monitor newDefaultMonitor(File tempDir, boolean watchForHardStop) throws IOException {
+    when(fileSystem.getTempDir()).thenReturn(tempDir);
+    return new Monitor(fileSystem, exit, watchForHardStop);
   }
 
   /**
@@ -369,7 +395,7 @@ public class MonitorTest {
     public void restart() {
       try {
         HttpRequest httpRequest = HttpRequest.post("http://localhost:" + httpPort + "/" + "restart")
-            .readTimeout(5000).connectTimeout(5000);
+          .readTimeout(5000).connectTimeout(5000);
         if (!httpRequest.ok() || !"ok".equals(httpRequest.body())) {
           throw new IllegalStateException("Wrong response calling restart");
         }