Repository: maven-surefire Updated Branches: refs/heads/master 257f59e63 -> 9c8a75906
[SUREFIRE-524] Forked Process not terminated if maven process aborted. Provide means to clean up. Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/f91c17a1 Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/f91c17a1 Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/f91c17a1 Branch: refs/heads/master Commit: f91c17a1c37bdbce9663098ad65f420f44468039 Parents: e87f880 Author: Tibor17 <[email protected]> Authored: Tue Sep 15 23:38:05 2015 +0200 Committer: Tibor17 <[email protected]> Committed: Tue Sep 15 23:38:05 2015 +0200 ---------------------------------------------------------------------- .../surefire/booterclient/ForkStarter.java | 127 ++++++++++++++++--- .../lazytestprovider/TestLessInputStream.java | 6 +- pom.xml | 2 +- .../surefire/booter/MasterProcessCommand.java | 2 +- .../maven/surefire/booter/ForkedBooter.java | 125 ++++++++++++++---- 5 files changed, 213 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/f91c17a1/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java index 5eafb4f..39680b1 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java @@ -62,7 +62,10 @@ import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.LinkedBlockingQueue; @@ -70,6 +73,7 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.apache.maven.shared.utils.cli.CommandLineUtils.executeCommandLine; import static org.apache.maven.shared.utils.cli.ShutdownHookUtils.addShutDownHook; import static org.apache.maven.shared.utils.cli.ShutdownHookUtils.removeShutdownHook; @@ -103,7 +107,13 @@ import static java.lang.StrictMath.min; */ public class ForkStarter { - private final ThreadFactory threadFactory = newDaemonThreadFactory(); + private static final long PING_IN_SECONDS = 10; + + private static final ThreadFactory DAEMON_THREAD_FACTORY = newDaemonThreadFactory(); + + private static final ThreadFactory SHUTDOWN_HOOK_THREAD_FACTORY = newDaemonThreadFactory(); + + private final ScheduledExecutorService pingThreadScheduler = createPingScheduler(); /** * Closes an InputStream @@ -183,6 +193,7 @@ public class ForkStarter { defaultReporterFactory.mergeFromOtherFactories( defaultReporterFactories ); defaultReporterFactory.close(); + pingThreadScheduler.shutdownNow(); } } @@ -196,12 +207,17 @@ public class ForkStarter new ForkClient( forkedReporterFactory, startupReportConfiguration.getTestVmSystemProperties() ); TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder(); TestLessInputStream stream = builder.build(); + Thread shutdown = createImmediateShutdownHookThread( builder ); + ScheduledFuture<?> ping = triggerPingTimerForShutdown( builder ); try { + addShutDownHook( shutdown ); return fork( null, props, forkClient, effectiveSystemProperties, stream, false ); } finally { + removeShutdownHook( shutdown ); + ping.cancel( true ); builder.removeStream( stream ); } } @@ -232,23 +248,28 @@ public class ForkStarter { ThreadPoolExecutor executorService = new ThreadPoolExecutor( forkCount, forkCount, 60, TimeUnit.SECONDS, new ArrayBlockingQueue<Runnable>( forkCount ) ); - executorService.setThreadFactory( threadFactory ); - try + executorService.setThreadFactory( DAEMON_THREAD_FACTORY ); + + final Queue<String> tests = new ConcurrentLinkedQueue<String>(); + + for ( Class<?> clazz : getSuitesIterator() ) { - final Queue<String> tests = new ConcurrentLinkedQueue<String>(); + tests.add( clazz.getName() ); + } - for ( Class<?> clazz : getSuitesIterator() ) - { - tests.add( clazz.getName() ); - } + final Queue<TestProvidingInputStream> testStreams = new ConcurrentLinkedQueue<TestProvidingInputStream>(); - final Queue<TestProvidingInputStream> testStreams = new ConcurrentLinkedQueue<TestProvidingInputStream>(); + for ( int forkNum = 0, total = min( forkCount, tests.size() ); forkNum < total; forkNum++ ) + { + testStreams.add( new TestProvidingInputStream( tests ) ); + } - for ( int forkNum = 0, total = min( forkCount, tests.size() ); forkNum < total; forkNum++ ) - { - testStreams.add( new TestProvidingInputStream( tests ) ); - } + ScheduledFuture<?> ping = triggerPingTimerForShutdown( testStreams ); + Thread shutdown = createShutdownHookThread( testStreams ); + try + { + addShutDownHook( shutdown ); int failFastCount = providerConfiguration.getSkipAfterFailureCount(); final AtomicInteger notifyStreamsToSkipTestsJustNow = new AtomicInteger( failFastCount ); Collection<Future<RunResult>> results = new ArrayList<Future<RunResult>>( forkCount ); @@ -286,6 +307,8 @@ public class ForkStarter } finally { + removeShutdownHook( shutdown ); + ping.cancel( true ); closeExecutor( executorService ); } } @@ -305,12 +328,15 @@ public class ForkStarter ArrayList<Future<RunResult>> results = new ArrayList<Future<RunResult>>( 500 ); ThreadPoolExecutor executorService = new ThreadPoolExecutor( forkCount, forkCount, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>() ); - executorService.setThreadFactory( threadFactory ); + executorService.setThreadFactory( DAEMON_THREAD_FACTORY ); + final TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder(); + ScheduledFuture<?> ping = triggerPingTimerForShutdown( builder ); + Thread shutdown = createCachableShutdownHookThread( builder ); try { + addShutDownHook( shutdown ); int failFastCount = providerConfiguration.getSkipAfterFailureCount(); final AtomicInteger notifyStreamsToSkipTestsJustNow = new AtomicInteger( failFastCount ); - final TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder(); for ( final Object testSet : getSuitesIterator() ) { Callable<RunResult> pf = new Callable<RunResult>() @@ -352,6 +378,8 @@ public class ForkStarter } finally { + removeShutdownHook( shutdown ); + ping.cancel( true ); closeExecutor( executorService ); } } @@ -508,7 +536,6 @@ public class ForkStarter { throw new SurefireBooterForkException( "Error occurred in starting fork, check output in log" ); } - } catch ( CommandLineTimeOutException e ) { @@ -546,7 +573,6 @@ public class ForkStarter "The forked VM terminated without properly saying goodbye. VM crash or System.exit called?" + "\nCommand was " + cli.toString() ); } - } forkClient.close( runResult.isTimeout() ); } @@ -575,4 +601,71 @@ public class ForkStarter throw new SurefireBooterForkException( "Unable to create classloader to find test suites", e ); } } + + private static Thread createImmediateShutdownHookThread( final TestLessInputStreamBuilder builder ) + { + return SHUTDOWN_HOOK_THREAD_FACTORY.newThread( new Runnable() + { + public void run() + { + builder.getImmediateCommands().shutdown(); + } + } ); + } + + private static Thread createCachableShutdownHookThread( final TestLessInputStreamBuilder builder ) + { + return SHUTDOWN_HOOK_THREAD_FACTORY.newThread( new Runnable() + { + public void run() + { + builder.getCachableCommands().shutdown(); + } + } ); + } + + private static Thread createShutdownHookThread( final Iterable<TestProvidingInputStream> streams ) + { + return SHUTDOWN_HOOK_THREAD_FACTORY.newThread( new Runnable() + { + public void run() + { + for ( TestProvidingInputStream stream : streams ) + { + stream.shutdown(); + } + } + } ); + } + + private static ScheduledExecutorService createPingScheduler() + { + ThreadFactory threadFactory = newDaemonThreadFactory( "ping-thread-" + PING_IN_SECONDS + "sec" ); + return Executors.newScheduledThreadPool( 1, threadFactory ); + } + + private ScheduledFuture<?> triggerPingTimerForShutdown( final TestLessInputStreamBuilder builder ) + { + return pingThreadScheduler.scheduleAtFixedRate( new Runnable() + { + public void run() + { + builder.getImmediateCommands().noop(); + } + }, 0, PING_IN_SECONDS, SECONDS ); + } + + private ScheduledFuture<?> triggerPingTimerForShutdown( final Iterable<TestProvidingInputStream> streams ) + { + return pingThreadScheduler.scheduleAtFixedRate( new Runnable() + { + public void run() + { + for ( TestProvidingInputStream stream : streams ) + { + stream.noop(); + } + } + }, 0, PING_IN_SECONDS, SECONDS ); + } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/f91c17a1/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java index b608620..b15af89 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java @@ -163,11 +163,13 @@ public final class TestLessInputStream } /** - * todo + * Builds {@link TestLessInputStream streams}, registers cachable commands + * and provides accessible API to dispatch immediate commands to all atomically + * alive streams. */ public static final class TestLessInputStreamBuilder { - ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); + private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); private final Queue<TestLessInputStream> aliveStreams = new ConcurrentLinkedQueue<TestLessInputStream>(); private final ImmediateCommands immediateCommands = new ImmediateCommands(); private final CachableCommands cachableCommands = new CachableCommands(); http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/f91c17a1/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 4b7d23a..a6a14c2 100644 --- a/pom.xml +++ b/pom.xml @@ -206,7 +206,7 @@ <dependency> <groupId>org.apache.maven.shared</groupId> <artifactId>maven-shared-utils</artifactId> - <version>0.8</version> + <version>0.9-SNAPSHOT</version> </dependency> <dependency> <groupId>org.apache.maven.shared</groupId> http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/f91c17a1/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java ---------------------------------------------------------------------- diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java index 3512b5a..2129bf0 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java @@ -42,7 +42,7 @@ public enum MasterProcessCommand TEST_SET_FINISHED( 1, Void.class ), SKIP_SINCE_NEXT_TEST( 2, Void.class ), SHUTDOWN( 3, Void.class ), - /** To tell a forked process that the master process is still alive. Repeated after 30 seconds. */ + /** To tell a forked process that the master process is still alive. Repeated after 10 seconds. */ NOOP( 4, Void.class ); private final int id; http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/f91c17a1/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java index 7567cd5..c7746e9 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java @@ -25,8 +25,10 @@ import java.io.InputStream; import java.io.PrintStream; import java.lang.reflect.InvocationTargetException; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.maven.surefire.providerapi.ProviderParameters; import org.apache.maven.surefire.providerapi.SurefireProvider; @@ -35,10 +37,16 @@ import org.apache.maven.surefire.report.ReporterFactory; import org.apache.maven.surefire.report.StackTraceWriter; import org.apache.maven.surefire.suite.RunResult; import org.apache.maven.surefire.testset.TestSetFailedException; -import org.apache.maven.surefire.util.ReflectionUtils; -import org.apache.maven.surefire.util.internal.DaemonThreadFactory; +import static org.apache.maven.surefire.booter.MasterProcessCommand.SHUTDOWN; +import static org.apache.maven.surefire.booter.MasterProcessCommand.NOOP; +import static org.apache.maven.surefire.booter.ForkingRunListener.BOOTERCODE_BYE; +import static org.apache.maven.surefire.booter.ForkingRunListener.BOOTERCODE_ERROR; +import static org.apache.maven.surefire.booter.ForkingRunListener.encode; +import static org.apache.maven.surefire.util.ReflectionUtils.instantiateOneArg; +import static org.apache.maven.surefire.util.internal.DaemonThreadFactory.newDaemonThreadFactory; import static org.apache.maven.surefire.util.internal.StringUtils.encodeStringForForkCommunication; +import static java.util.concurrent.TimeUnit.SECONDS; /** * The part of the booter that is unique to a forked vm. @@ -53,17 +61,19 @@ import static org.apache.maven.surefire.util.internal.StringUtils.encodeStringFo public final class ForkedBooter { private static final long SYSTEM_EXIT_TIMEOUT_IN_SECONDS = 30; + private static final long PING_TIMEOUT_IN_SECONDS = 20; + + private static final ScheduledExecutorService JVM_TERMINATOR = createJvmTerminator(); /** * This method is invoked when Surefire is forked - this method parses and organizes the arguments passed to it and * then calls the Surefire class' run method. <p/> The system exit code will be 1 if an exception is thrown. * * @param args Commandline arguments - * @throws Throwable Upon throwables */ - public static void main( String[] args ) - throws Throwable + public static void main( String... args ) { + ScheduledFuture<?> pingScheduler = listenToShutdownCommands(); final PrintStream originalOut = System.out; try { @@ -114,23 +124,21 @@ public final class ForkedBooter LegacyPojoStackTraceWriter stackTraceWriter = new LegacyPojoStackTraceWriter( "test subystem", "no method", t.getTargetException() ); StringBuilder stringBuilder = new StringBuilder(); - ForkingRunListener.encode( stringBuilder, stackTraceWriter, false ); - encodeAndWriteToOutput( ( (char) ForkingRunListener.BOOTERCODE_ERROR ) - + ",0," + stringBuilder.toString() + "\n" , originalOut ); + encode( stringBuilder, stackTraceWriter, false ); + encodeAndWriteToOutput( ( (char) BOOTERCODE_ERROR ) + ",0," + stringBuilder + "\n" , originalOut ); } catch ( Throwable t ) { StackTraceWriter stackTraceWriter = new LegacyPojoStackTraceWriter( "test subystem", "no method", t ); StringBuilder stringBuilder = new StringBuilder(); - ForkingRunListener.encode( stringBuilder, stackTraceWriter, false ); - encodeAndWriteToOutput( ( (char) ForkingRunListener.BOOTERCODE_ERROR ) - + ",0," + stringBuilder.toString() + "\n", originalOut ); + encode( stringBuilder, stackTraceWriter, false ); + encodeAndWriteToOutput( ( (char) BOOTERCODE_ERROR ) + ",0," + stringBuilder + "\n", originalOut ); } // Say bye. - encodeAndWriteToOutput( ( (char) ForkingRunListener.BOOTERCODE_BYE ) + ",0,BYE!\n", originalOut ); + encodeAndWriteToOutput( ( (char) BOOTERCODE_BYE ) + ",0,BYE!\n", originalOut ); originalOut.flush(); // noinspection CallToSystemExit - exit( 0 ); + exit( 0, false ); } catch ( Throwable t ) { @@ -138,8 +146,58 @@ public final class ForkedBooter // noinspection UseOfSystemOutOrSystemErr t.printStackTrace( System.err ); // noinspection ProhibitedExceptionThrown,CallToSystemExit - exit( 1 ); + exit( 1, false ); } + finally + { + pingScheduler.cancel( true ); + } + } + + private static ScheduledFuture<?> listenToShutdownCommands() + { + MasterProcessReader reader = MasterProcessReader.getReader(); + reader.addListener( SHUTDOWN, createExitHandler() ); + AtomicBoolean pingDone = new AtomicBoolean( true ); + reader.addListener( NOOP, createPingHandler( pingDone ) ); + return JVM_TERMINATOR.scheduleAtFixedRate( createPingJob( pingDone ), 0, PING_TIMEOUT_IN_SECONDS, SECONDS ); + } + + private static MasterProcessListener createPingHandler( final AtomicBoolean pingDone ) + { + return new MasterProcessListener() + { + public void update( Command command ) + { + pingDone.set( true ); + } + }; + } + + private static MasterProcessListener createExitHandler() + { + return new MasterProcessListener() + { + public void update( Command command ) + { + exit( 1, true ); + } + }; + } + + private static Runnable createPingJob( final AtomicBoolean pingDone ) + { + return new Runnable() + { + public void run() + { + boolean hasPing = pingDone.getAndSet( false ); + if ( !hasPing ) + { + exit( 1, true ); + } + } + }; } private static void encodeAndWriteToOutput( String string, PrintStream out ) @@ -148,10 +206,19 @@ public final class ForkedBooter out.write( encodeBytes, 0, encodeBytes.length ); } - private static void exit( final int returnCode ) + private static void exit( int returnCode, boolean immediate ) { - launchLastDitchDaemonShutdownThread( returnCode ); - System.exit( returnCode ); + MasterProcessReader.getReader().stop(); + + if ( immediate ) + { + Runtime.getRuntime().halt( returnCode ); + } + else + { + launchLastDitchDaemonShutdownThread( returnCode ); + System.exit( returnCode ); + } } private static RunResult runSuitesInProcess( Object testSet, StartupConfiguration startupConfiguration, @@ -172,22 +239,25 @@ public final class ForkedBooter return SurefireReflector.createForkingReporterFactoryInCurrentClassLoader( trimStackTrace, originalSystemOut ); } - @SuppressWarnings( "checkstyle:emptyblock" ) - private static void launchLastDitchDaemonShutdownThread( final int returnCode ) + private static ScheduledExecutorService createJvmTerminator() { - ThreadFactory threadFactory = - DaemonThreadFactory.newDaemonThreadFactory( "last-ditch-daemon-shutdown-thread-" + ThreadFactory threadFactory = newDaemonThreadFactory( "last-ditch-daemon-shutdown-thread-" + SYSTEM_EXIT_TIMEOUT_IN_SECONDS + "sec" ); - Executors.newScheduledThreadPool( 1, threadFactory ) - .schedule( new Runnable() + return Executors.newScheduledThreadPool( 1, threadFactory ); + } + + @SuppressWarnings( "checkstyle:emptyblock" ) + private static void launchLastDitchDaemonShutdownThread( final int returnCode ) + { + JVM_TERMINATOR.schedule( new Runnable() { public void run() { Runtime.getRuntime().halt( returnCode ); } - }, SYSTEM_EXIT_TIMEOUT_IN_SECONDS, TimeUnit.SECONDS ); + }, SYSTEM_EXIT_TIMEOUT_IN_SECONDS, SECONDS ); } private static RunResult invokeProviderInSameClassLoader( Object testSet, Object factory, @@ -233,8 +303,7 @@ public final class ForkedBooter bpf.setDirectoryScannerParameters( providerConfiguration.getDirScannerParams() ); bpf.setMainCliOptions( providerConfiguration.getMainCliOptions() ); bpf.setSkipAfterFailureCount( providerConfiguration.getSkipAfterFailureCount() ); - return (SurefireProvider) ReflectionUtils.instantiateOneArg( classLoader, - startupConfiguration1.getActualClassName(), - ProviderParameters.class, bpf ); + String providerClass = startupConfiguration1.getActualClassName(); + return (SurefireProvider) instantiateOneArg( classLoader, providerClass, ProviderParameters.class, bpf ); } }
