From ca0b3a94ced3ee10f76c9465ccfa19767d62884d Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 8 Mar 2016 14:10:00 +0100 Subject: [PATCH] SONAR-7435 move awaitTermination to ProcessUtils and add UTs --- .../org/sonar/process/monitor/Monitor.java | 32 ++------ .../java/org/sonar/process/ProcessUtils.java | 20 +++++ .../org/sonar/process/ProcessUtilsTest.java | 79 +++++++++++++++++++ 3 files changed, 105 insertions(+), 26 deletions(-) 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 1aef72c479c..a4e1bfd4b7f 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 @@ -26,13 +26,13 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import javax.annotation.CheckForNull; -import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.process.DefaultProcessCommands; import org.sonar.process.Lifecycle; import org.sonar.process.Lifecycle.State; import org.sonar.process.ProcessCommands; +import org.sonar.process.ProcessUtils; import org.sonar.process.SystemExit; public class Monitor { @@ -178,7 +178,7 @@ public class Monitor { public void awaitTermination() { while (awaitChildProcessesTermination()) { trace("await termination of restartor..."); - awaitTermination(restartor); + ProcessUtils.awaitTermination(restartor); } cleanAfterTermination(); } @@ -188,7 +188,7 @@ public class Monitor { trace("finished waiting, restartRequested={}", restartRequested); if (restartRequested) { trace("awaitTermination restartor={}", restartor); - awaitTermination(restartor); + ProcessUtils.awaitTermination(restartor); } return restartRequested; } @@ -197,7 +197,7 @@ public class Monitor { trace("await termination of child processes..."); List watcherThreadsCopy = new ArrayList<>(this.watcherThreads); for (WatcherThread watcherThread : watcherThreadsCopy) { - awaitTermination(watcherThread); + ProcessUtils.awaitTermination(watcherThread); } trace("all child processes done"); return hasRestartBeenRequested(watcherThreadsCopy); @@ -221,7 +221,7 @@ public class Monitor { trace("start hard stop async..."); stopAsync(State.HARD_STOPPING); trace("await termination of terminator..."); - awaitTermination(terminator); + ProcessUtils.awaitTermination(terminator); cleanAfterTermination(); trace("exit..."); systemExit.exit(0); @@ -232,7 +232,7 @@ public class Monitor { if (lifecycle.tryToMoveTo(State.STOPPED)) { trace("await termination of restartWatcher and hardStopWatcher..."); // wait for restartWatcher and hardStopWatcher to cleanly stop - awaitTermination(restartWatcher, hardStopWatcher); + ProcessUtils.awaitTermination(restartWatcher, hardStopWatcher); trace("restartWatcher done"); // removing shutdown hook to avoid called stop() unnecessarily unless already in shutdownHook if (!systemExit.isInShutdownHook()) { @@ -444,26 +444,6 @@ public class Monitor { } } - private static void awaitTermination(Thread... threads) { - for (Thread thread : threads) { - awaitTermination(thread); - } - } - - private static void awaitTermination(@Nullable Thread t) { - if (t == null || Thread.currentThread() == t) { - return; - } - - while (t.isAlive()) { - try { - t.join(); - } catch (InterruptedException e) { - // ignore and stop blocking - } - } - } - private static void trace(String s) { LOG.trace(s); } diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java index 041c4f5ccc1..e468172dfe9 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java +++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessUtils.java @@ -71,4 +71,24 @@ public class ProcessUtils { IOUtils.closeQuietly(process.getErrorStream()); } } + + public static void awaitTermination(Thread... threads) { + for (Thread thread : threads) { + awaitTermination(thread); + } + } + + public static void awaitTermination(@Nullable Thread t) { + if (t == null || Thread.currentThread() == t) { + return; + } + + while (t.isAlive()) { + try { + t.join(); + } catch (InterruptedException e) { + // ignore, keep on waiting for t to stop + } + } + } } diff --git a/server/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java b/server/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java index 45aeb2a7079..9b6be953e6b 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/ProcessUtilsTest.java @@ -23,6 +23,7 @@ import org.junit.Test; import org.sonar.test.TestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.process.ProcessUtils.awaitTermination; public class ProcessUtilsTest { @@ -30,4 +31,82 @@ public class ProcessUtilsTest { public void private_constructor() { assertThat(TestUtils.hasOnlyPrivateConstructors(ProcessUtils.class)).isTrue(); } + + @Test + public void awaitTermination_does_not_fail_on_null_Thread_argument() { + awaitTermination((Thread) null); + } + + @Test(timeout = 100L) + public void awaitTermination_does_not_wait_on_currentThread() { + awaitTermination(Thread.currentThread()); + } + + @Test(timeout = 3000) + public void awaitTermination_ignores_interrupted_exception_of_current_thread() throws InterruptedException { + final EverRunningThread runningThread = new EverRunningThread(); + final Thread safeJoiner = new Thread() { + @Override + public void run() { + awaitTermination(runningThread); + } + }; + final Thread simpleJoiner = new Thread() { + @Override + public void run() { + try { + runningThread.join(); + } catch (InterruptedException e) { + System.err.println("runningThread interruption detected in SimpleJoiner"); + } + } + }; + runningThread.start(); + safeJoiner.start(); + simpleJoiner.start(); + + // interrupt safeJoiner _before simpleJoiner to work around some arbitrary sleep delay_ which should not stop watching + safeJoiner.interrupt(); + + // interrupting simpleJoiner which should stop + simpleJoiner.interrupt(); + + while (simpleJoiner.isAlive()) { + // wait for simpleJoiner to stop + } + + // safeJoiner must still be alive + assertThat(safeJoiner.isAlive()).isTrue() ; + + // stop runningThread + runningThread.stopIt(); + + while (runningThread.isAlive()) { + // wait for runningThread to stop + } + + // wait for safeJoiner to stop because runningThread has stopped, if it doesn't, the test will fail with a timeout + safeJoiner.join(); + } + + @Test(timeout = 100L) + public void awaitTermination_of_vararg_does_not_fail_when_there_is_a_null_or_current_thread() { + awaitTermination(null, Thread.currentThread(), null); + } + + private static class EverRunningThread extends Thread { + private volatile boolean stop = false; + + @Override + public void run() { + while (!stop) { + // infinite loop! + } + } + + public void stopIt() { + this.stop = true; + } + } + } -- 2.39.5