From e7c4a306ac47843029d183d8e3f5ca7730184cf2 Mon Sep 17 00:00:00 2001 From: Sébastien Lesaint Date: Wed, 13 Jan 2016 11:24:01 +0100 Subject: 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) --- .../java/org/sonar/process/monitor/FileSystem.java | 29 +++++ .../sonar/process/monitor/JavaProcessLauncher.java | 8 +- .../java/org/sonar/process/monitor/Monitor.java | 70 +++++++++--- .../process/monitor/JavaProcessLauncherTest.java | 3 +- .../org/sonar/process/monitor/MonitorTest.java | 124 +++++++++++++-------- 5 files changed, 166 insertions(+), 68 deletions(-) create mode 100644 server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/FileSystem.java (limited to 'server/sonar-process-monitor/src') 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 watcherThreads = new CopyOnWriteArrayList<>(); private final Lifecycle lifecycle = new Lifecycle(); + private final TerminatorThread terminator = new TerminatorThread(); private final RestartRequestWatcherThread restartWatcher = new RestartRequestWatcherThread(); @CheckForNull private List 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.emptyList()); + underTest.start(Collections.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"); } -- cgit v1.2.3