Repository: maven-surefire Updated Branches: refs/heads/master 5cb02c375 -> 99438ac9d
SUREFIRE-1101 Surefire does not shutdown thread-pools programatically after test run has finished. Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/99438ac9 Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/99438ac9 Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/99438ac9 Branch: refs/heads/master Commit: 99438ac9dd995176b64a872dc628fd66f74f90ed Parents: 5cb02c3 Author: tibordigana <tibo...@lycos.com> Authored: Tue Oct 7 07:49:24 2014 +0200 Committer: tibordigana <tibo...@lycos.com> Committed: Tue Oct 7 22:15:49 2014 +0200 ---------------------------------------------------------------------- .../pc/AbstractThreadPoolStrategy.java | 34 +++-- .../surefire/junitcore/pc/Destroyable.java | 38 ++++++ .../surefire/junitcore/pc/InvokerStrategy.java | 15 +-- .../pc/NonSharedThreadPoolStrategy.java | 13 +- .../surefire/junitcore/pc/ParallelComputer.java | 89 ++++++------- .../junitcore/pc/ParallelComputerBuilder.java | 24 +++- .../maven/surefire/junitcore/pc/Scheduler.java | 129 ++++++++++++------- .../junitcore/pc/SchedulingStrategy.java | 26 +++- .../junitcore/pc/SharedThreadPoolStrategy.java | 17 ++- .../junitcore/pc/SingleThreadScheduler.java | 40 ++++-- .../junitcore/pc/ThreadResourcesBalancer.java | 6 + .../pc/ParallelComputerBuilderTest.java | 93 ++++++++++++- .../junitcore/pc/ParallelComputerUtilTest.java | 32 ++++- 13 files changed, 406 insertions(+), 150 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/AbstractThreadPoolStrategy.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/AbstractThreadPoolStrategy.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/AbstractThreadPoolStrategy.java index 56621d5..852e24e 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/AbstractThreadPoolStrategy.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/AbstractThreadPoolStrategy.java @@ -23,7 +23,7 @@ import java.util.Collection; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.TimeUnit; /** * Abstract parallel scheduling strategy in private package. @@ -43,7 +43,7 @@ abstract class AbstractThreadPoolStrategy private final Collection<Future<?>> futureResults; - private final AtomicBoolean canSchedule = new AtomicBoolean( true ); + private volatile boolean isDestroyed; AbstractThreadPoolStrategy( ExecutorService threadPool ) { @@ -66,11 +66,6 @@ abstract class AbstractThreadPoolStrategy return futureResults; } - protected final void disable() - { - canSchedule.set( false ); - } - @Override public void schedule( Runnable task ) { @@ -87,7 +82,7 @@ abstract class AbstractThreadPoolStrategy @Override protected boolean stop() { - boolean wasRunning = canSchedule.getAndSet( false ); + boolean wasRunning = disable(); if ( threadPool.isShutdown() ) { wasRunning = false; @@ -102,7 +97,7 @@ abstract class AbstractThreadPoolStrategy @Override protected boolean stopNow() { - boolean wasRunning = canSchedule.getAndSet( false ); + boolean wasRunning = disable(); if ( threadPool.isShutdown() ) { wasRunning = false; @@ -114,6 +109,9 @@ abstract class AbstractThreadPoolStrategy return wasRunning; } + /** + * @see Scheduler.ShutdownHandler + */ @Override protected void setDefaultShutdownHandler( Scheduler.ShutdownHandler handler ) { @@ -125,9 +123,21 @@ abstract class AbstractThreadPoolStrategy } } - @Override - public final boolean canSchedule() + public boolean destroy() { - return canSchedule.get(); + try + { + if ( !isDestroyed )//just an optimization + { + disable(); + threadPool.shutdown(); + this.isDestroyed |= threadPool.awaitTermination( Long.MAX_VALUE, TimeUnit.NANOSECONDS ); + } + return isDestroyed; + } + catch ( InterruptedException e ) + { + return false; + } } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Destroyable.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Destroyable.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Destroyable.java new file mode 100644 index 0000000..284ce5a --- /dev/null +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Destroyable.java @@ -0,0 +1,38 @@ +package org.apache.maven.surefire.junitcore.pc; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Destroys the embedded thread-pool. + * + * @author <a href="mailto:tibor.dig...@gmail.com">Tibor Digana (tibor17)</a> + * @see ParallelComputerBuilder + * @since 2.18 + */ +public interface Destroyable +{ + /** + * Calling {@link java.util.concurrent.ThreadPoolExecutor#shutdown()} + * and {@link java.util.concurrent.ThreadPoolExecutor#awaitTermination(long, java.util.concurrent.TimeUnit)}. + * + * @return {@code true} if not interrupted in current thread + */ + boolean destroy(); +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/InvokerStrategy.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/InvokerStrategy.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/InvokerStrategy.java index 4dd7f10..06c328d 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/InvokerStrategy.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/InvokerStrategy.java @@ -21,7 +21,6 @@ package org.apache.maven.surefire.junitcore.pc; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.atomic.AtomicBoolean; /** * The sequentially executing strategy in private package. @@ -33,7 +32,6 @@ import java.util.concurrent.atomic.AtomicBoolean; final class InvokerStrategy extends SchedulingStrategy { - private final AtomicBoolean canSchedule = new AtomicBoolean( true ); private final Queue<Thread> activeThreads = new ConcurrentLinkedQueue<Thread>(); @@ -58,13 +56,13 @@ final class InvokerStrategy @Override protected boolean stop() { - return canSchedule.getAndSet( false ); + return disable(); } @Override protected boolean stopNow() { - final boolean stopped = stop(); + final boolean stopped = disable(); for ( Thread activeThread; ( activeThread = activeThreads.poll() ) != null; ) { activeThread.interrupt(); @@ -79,14 +77,13 @@ final class InvokerStrategy } @Override - public boolean canSchedule() + public boolean finished() + throws InterruptedException { - return canSchedule.get(); + return disable(); } - @Override - public boolean finished() - throws InterruptedException + public boolean destroy() { return stop(); } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/NonSharedThreadPoolStrategy.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/NonSharedThreadPoolStrategy.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/NonSharedThreadPoolStrategy.java index df80ad9..9fa1e6b 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/NonSharedThreadPoolStrategy.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/NonSharedThreadPoolStrategy.java @@ -47,16 +47,9 @@ final class NonSharedThreadPoolStrategy public boolean finished() throws InterruptedException { - boolean wasRunning = canSchedule(); + boolean wasRunning = disable(); getThreadPool().shutdown(); - try - { - getThreadPool().awaitTermination( Long.MAX_VALUE, TimeUnit.NANOSECONDS ); - return wasRunning; - } - finally - { - disable(); - } + getThreadPool().awaitTermination( Long.MAX_VALUE, TimeUnit.NANOSECONDS ); + return wasRunning; } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputer.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputer.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputer.java index bf28c70..483e5d9 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputer.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputer.java @@ -59,45 +59,8 @@ public abstract class ParallelComputer this.timeoutForcedNanos = secondsToNanos( timeoutForcedInSeconds ); } - private static long secondsToNanos( double seconds ) - { - double nanos = seconds > 0 ? seconds * 1E9 : 0; - return Double.isInfinite( nanos ) || nanos >= Long.MAX_VALUE ? 0 : (long) nanos; - } - - private static long minTimeout( long timeout1, long timeout2 ) - { - if ( timeout1 == 0 ) - { - return timeout2; - } - else if ( timeout2 == 0 ) - { - return timeout1; - } - else - { - return Math.min( timeout1, timeout2 ); - } - } - - private static void printShutdownHook( Collection<String> executedTests, - Future<Collection<Description>> testsBeforeShutdown ) - throws ExecutionException, InterruptedException - { - if ( testsBeforeShutdown != null ) - { - for ( final Description executedTest : testsBeforeShutdown.get() ) - { - if ( executedTest != null && executedTest.getDisplayName() != null ) - { - executedTests.add( executedTest.getDisplayName() ); - } - } - } - } - - public abstract Collection<Description> shutdown( boolean shutdownNow ); + protected abstract Collection<Description> describeStopped( boolean shutdownNow ); + abstract boolean shutdownThreadPoolsAwaitingKilled(); protected final void beforeRunQuietly() { @@ -109,6 +72,7 @@ public abstract class ParallelComputer { shutdownStatus.tryFinish(); forcedShutdownStatus.tryFinish(); + boolean notInterrupted = true; if ( shutdownScheduler != null ) { shutdownScheduler.shutdownNow(); @@ -123,10 +87,11 @@ public abstract class ParallelComputer } catch ( InterruptedException e ) { - return false; + notInterrupted = false; } } - return true; + notInterrupted &= shutdownThreadPoolsAwaitingKilled(); + return notInterrupted; } public String describeElapsedTimeout() @@ -203,7 +168,7 @@ public abstract class ParallelComputer throws Exception { boolean stampedStatusWithTimeout = ParallelComputer.this.shutdownStatus.tryTimeout(); - return stampedStatusWithTimeout ? ParallelComputer.this.shutdown( false ) : null; + return stampedStatusWithTimeout ? ParallelComputer.this.describeStopped( false ) : null; } }; } @@ -216,7 +181,7 @@ public abstract class ParallelComputer throws Exception { boolean stampedStatusWithTimeout = ParallelComputer.this.forcedShutdownStatus.tryTimeout(); - return stampedStatusWithTimeout ? ParallelComputer.this.shutdown( true ) : null; + return stampedStatusWithTimeout ? ParallelComputer.this.describeStopped( true ) : null; } }; } @@ -235,4 +200,42 @@ public abstract class ParallelComputer { return timeoutForcedNanos > 0; } + + private static long secondsToNanos( double seconds ) + { + double nanos = seconds > 0 ? seconds * 1E9 : 0; + return Double.isInfinite( nanos ) || nanos >= Long.MAX_VALUE ? 0 : (long) nanos; + } + + private static long minTimeout( long timeout1, long timeout2 ) + { + if ( timeout1 == 0 ) + { + return timeout2; + } + else if ( timeout2 == 0 ) + { + return timeout1; + } + else + { + return Math.min( timeout1, timeout2 ); + } + } + + private static void printShutdownHook( Collection<String> executedTests, + Future<Collection<Description>> testsBeforeShutdown ) + throws ExecutionException, InterruptedException + { + if ( testsBeforeShutdown != null ) + { + for ( final Description executedTest : testsBeforeShutdown.get() ) + { + if ( executedTest != null && executedTest.getDisplayName() != null ) + { + executedTests.add( executedTest.getDisplayName() ); + } + } + } + } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilder.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilder.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilder.java index ca7cc60..9353349 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilder.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilder.java @@ -64,10 +64,10 @@ import static org.apache.maven.surefire.junitcore.pc.Type.*; * {@link ParallelComputerBuilder#useOnePool(int)} must be greater than the number of concurrent suites and classes * altogether. * <p/> - * The Computer can be shutdown in a separate thread. Pending tests will be interrupted if the argument is + * The Computer can be stopped in a separate thread. Pending tests will be interrupted if the argument is * <tt>true</tt>. * <pre> - * computer.shutdown(true); + * computer.describeStopped(true); * </pre> * * @author Tibor Digana (tibor17) @@ -254,18 +254,30 @@ public final class ParallelComputerBuilder } @Override - public Collection<Description> shutdown( boolean shutdownNow ) + protected Collection<Description> describeStopped( boolean shutdownNow ) { - Collection<Description> startedTests = notThreadSafeTests.shutdown( shutdownNow ); - final Scheduler m = this.master; + Collection<Description> startedTests = notThreadSafeTests.describeStopped( shutdownNow ); + final Scheduler m = master; if ( m != null ) { - startedTests.addAll( m.shutdown( shutdownNow ) ); + startedTests.addAll( m.describeStopped( shutdownNow ) ); } return startedTests; } @Override + boolean shutdownThreadPoolsAwaitingKilled() + { + boolean notInterrupted = notThreadSafeTests.shutdownThreadPoolsAwaitingKilled(); + final Scheduler m = master; + if ( m != null ) + { + notInterrupted &= m.shutdownThreadPoolsAwaitingKilled(); + } + return notInterrupted; + } + + @Override public Runner getSuite( RunnerBuilder builder, Class<?>[] cls ) throws InitializationError { http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Scheduler.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Scheduler.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Scheduler.java index a16f1d2..8feeb02 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Scheduler.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/Scheduler.java @@ -22,6 +22,7 @@ package org.apache.maven.surefire.junitcore.pc; import org.junit.runner.Description; import org.junit.runners.model.RunnerScheduler; +import java.util.Arrays; import java.util.Collection; import java.util.Set; import java.util.concurrent.ConcurrentLinkedQueue; @@ -45,6 +46,9 @@ import java.util.concurrent.ThreadPoolExecutor; public class Scheduler implements RunnerScheduler { + private static final Collection<Description> UNUSED_DESCRIPTIONS = + Arrays.asList( null, Description.TEST_MECHANISM, Description.EMPTY ); + private final Balancer balancer; private final SchedulingStrategy strategy; @@ -202,51 +206,93 @@ public class Scheduler * Attempts to stop all actively executing tasks and immediately returns a collection * of descriptions of those tasks which have started prior to this call. * <p/> - * This scheduler and other registered schedulers will shutdown, see {@link #register(Scheduler)}. + * This scheduler and other registered schedulers will stop, see {@link #register(Scheduler)}. * If <tt>shutdownNow</tt> is set, waiting methods will be interrupted via {@link Thread#interrupt}. * - * @param shutdownNow if <tt>true</tt> interrupts waiting methods + * @param stopNow if <tt>true</tt> interrupts waiting test methods * @return collection of descriptions started before shutting down */ - public Collection<Description> shutdown( boolean shutdownNow ) + protected Collection<Description> describeStopped( boolean stopNow ) { - shutdown = true; - Collection<Description> activeChildren = new ConcurrentLinkedQueue<Description>(); + Collection<Description> executedTests = new ConcurrentLinkedQueue<Description>(); + stop( executedTests, false, stopNow ); + return executedTests; + } - if ( started && description != null ) + /** + * Stop/Shutdown/Interrupt scheduler and its children (if any). + * + * @param executedTests Started tests which have finished normally or abruptly till called this method. + * @param tryCancelFutures Useful to set to {@code false} if a timeout is specified in plugin config. + * When the runner of + * {@link ParallelComputer#getSuite(org.junit.runners.model.RunnerBuilder, Class[])} + * is finished in + * {@link org.junit.runners.Suite#run(org.junit.runner.notification.RunNotifier)} + * all the thread-pools created by {@link ParallelComputerBuilder.PC} are already dead. + * See the unit test <em>ParallelComputerBuilder#timeoutAndForcedShutdown()</em>. + * @param stopNow Interrupting tests by {@link java.util.concurrent.ExecutorService#shutdownNow()} or + * {@link java.util.concurrent.Future#cancel(boolean) Future#cancel(true)} or + * {@link Thread#interrupt()}. + */ + private void stop( Collection<Description> executedTests, boolean tryCancelFutures, boolean stopNow ) + { + shutdown = true; + try { - activeChildren.add( description ); - } + if ( executedTests != null && started && !UNUSED_DESCRIPTIONS.contains( description ) ) + { + executedTests.add( description ); + } - for ( Controller slave : slaves ) + for ( Controller slave : slaves ) + { + slave.stop( executedTests, tryCancelFutures, stopNow ); + } + } + finally { try { - activeChildren.addAll( slave.shutdown( shutdownNow ) ); + balancer.releaseAllPermits(); } - catch ( Throwable t ) + finally { - logQuietly( t ); + if ( stopNow ) + { + strategy.stopNow(); + } + else if ( tryCancelFutures ) + { + strategy.stop(); + } + else + { + strategy.disable(); + } } } + } - try - { - balancer.releaseAllPermits(); - } - finally + protected boolean shutdownThreadPoolsAwaitingKilled() + { + if ( masterController == null ) { - if ( shutdownNow ) + stop( null, true, false ); + boolean isNotInterrupted = true; + if ( strategy != null ) { - strategy.stopNow(); + isNotInterrupted = strategy.destroy(); } - else + for ( Controller slave : slaves ) { - strategy.stop(); + isNotInterrupted &= slave.destroy(); } + return isNotInterrupted; + } + else + { + throw new UnsupportedOperationException( "cannot call this method if this is not a master scheduler" ); } - - return activeChildren; } protected void beforeExecute() @@ -277,7 +323,7 @@ public class Scheduler } catch ( RejectedExecutionException e ) { - shutdown( false ); + stop( null, true, false ); } catch ( Throwable t ) { @@ -297,13 +343,6 @@ public class Scheduler { logQuietly( e ); } - finally - { - for ( Controller slave : slaves ) - { - slave.awaitFinishedQuietly(); - } - } } private Runnable wrapTask( final Runnable task ) @@ -357,21 +396,17 @@ public class Scheduler return Scheduler.this.canSchedule(); } - void awaitFinishedQuietly() + void stop( Collection<Description> executedTests, boolean tryCancelFutures, boolean shutdownNow ) { - try - { - slave.finished(); - } - catch ( Throwable t ) - { - slave.logQuietly( t ); - } + slave.stop( executedTests, tryCancelFutures, shutdownNow ); } - Collection<Description> shutdown( boolean shutdownNow ) + /** + * @see org.apache.maven.surefire.junitcore.pc.Destroyable#destroy() + */ + boolean destroy() { - return slave.shutdown( shutdownNow ); + return slave.strategy.destroy(); } @Override @@ -387,6 +422,14 @@ public class Scheduler } } + /** + * There is a way to shutdown the hierarchy of schedulers. You can do it in master scheduler via + * {@link #shutdownThreadPoolsAwaitingKilled()} which kills the current master and children recursively. + * If alternatively a shared {@link java.util.concurrent.ExecutorService} used by the master and children + * schedulers is shutdown from outside, then the {@link ShutdownHandler} is a hook calling current + * {@link #describeStopped(boolean)}. The method {@link #describeStopped(boolean)} is again shutting down children + * schedulers recursively as well. + */ public class ShutdownHandler implements RejectedExecutionHandler { @@ -406,7 +449,7 @@ public class Scheduler { if ( executor.isShutdown() ) { - shutdown( false ); + Scheduler.this.stop( null, true, false ); } final RejectedExecutionHandler poolHandler = this.poolHandler; if ( poolHandler != null ) http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SchedulingStrategy.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SchedulingStrategy.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SchedulingStrategy.java index f419cb7..1ce744d 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SchedulingStrategy.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SchedulingStrategy.java @@ -22,6 +22,7 @@ package org.apache.maven.surefire.junitcore.pc; import org.junit.runners.model.RunnerScheduler; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; /** * Specifies the strategy of scheduling whether sequential, or parallel. @@ -36,8 +37,11 @@ import java.util.concurrent.RejectedExecutionException; * @since 2.16 */ public abstract class SchedulingStrategy + implements Destroyable { + private final AtomicBoolean canSchedule = new AtomicBoolean( true ); + /** * Schedules tasks if {@link #canSchedule()}. * @@ -92,6 +96,16 @@ public abstract class SchedulingStrategy return stop(); } + /** + * Persistently disables this strategy. Atomically ignores {@link Balancer} to acquire a new permit.<p/> + * The method {@link #canSchedule()} atomically returns {@code false}. + * @return {@code true} if {@link #canSchedule()} has return {@code true} on the beginning of this method call. + */ + protected boolean disable() + { + return canSchedule.getAndSet( false ); + } + protected void setDefaultShutdownHandler( Scheduler.ShutdownHandler handler ) { } @@ -103,7 +117,15 @@ public abstract class SchedulingStrategy protected abstract boolean hasSharedThreadPool(); /** - * @return <tt>true</tt> unless stopped or finished. + * @return <tt>true</tt> unless stopped, finished or disabled. */ - protected abstract boolean canSchedule(); + protected boolean canSchedule() + { + return canSchedule.get(); + } + + protected void logQuietly( Throwable t ) + { + t.printStackTrace( System.out ); + } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SharedThreadPoolStrategy.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SharedThreadPoolStrategy.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SharedThreadPoolStrategy.java index 88907e6..cfe6faa 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SharedThreadPoolStrategy.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SharedThreadPoolStrategy.java @@ -50,7 +50,7 @@ final class SharedThreadPoolStrategy public boolean finished() throws InterruptedException { - boolean wasRunningAll = canSchedule(); + boolean wasRunningAll = disable(); for ( Future<?> futureResult : getFutureResults() ) { try @@ -60,19 +60,23 @@ final class SharedThreadPoolStrategy catch ( InterruptedException e ) { // after called external ExecutorService#shutdownNow() - // or ExecutorService#shutdown() wasRunningAll = false; } catch ( ExecutionException e ) { - // test throws exception + // JUnit core throws exception. + if ( e.getCause() != null ) + { + logQuietly( e.getCause() ); + } } catch ( CancellationException e ) { - // cannot happen because not calling Future#cancel() + /** + * Cancelled by {@link Future#cancel(boolean)} in {@link stop()} and {@link stopNow()}. + */ } } - disable(); return wasRunningAll; } @@ -90,12 +94,11 @@ final class SharedThreadPoolStrategy private boolean stop( boolean interrupt ) { - final boolean wasRunning = canSchedule(); + final boolean wasRunning = disable(); for ( Future<?> futureResult : getFutureResults() ) { futureResult.cancel( interrupt ); } - disable(); return wasRunning; } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SingleThreadScheduler.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SingleThreadScheduler.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SingleThreadScheduler.java index 79b3197..42a5c59 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SingleThreadScheduler.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/SingleThreadScheduler.java @@ -22,7 +22,9 @@ package org.apache.maven.surefire.junitcore.pc; import org.junit.runner.Description; import org.junit.runners.model.RunnerScheduler; +import java.util.Arrays; import java.util.Collection; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; @@ -39,32 +41,46 @@ import java.util.concurrent.TimeUnit; */ final class SingleThreadScheduler { + private static final Collection<Description> UNUSED_DESCRIPTIONS = + Arrays.asList( null, Description.TEST_MECHANISM, Description.EMPTY ); + private final ExecutorService pool = newPool(); private final Scheduler master = new Scheduler( null, SchedulingStrategies.createParallelSharedStrategy( pool ) ); + private static ExecutorService newPool() + { + final ThreadFactory factory = new ThreadFactory() + { + public Thread newThread( Runnable r ) + { + return new Thread( r, "maven-surefire-plugin@NotThreadSafe" ); + } + }; + return new ThreadPoolExecutor( 1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>(), factory ); + } + RunnerScheduler newRunnerScheduler() { return new Scheduler( null, master, SchedulingStrategies.createParallelSharedStrategy( pool ) ); } /** - * @see Scheduler#shutdown(boolean) + * @see Scheduler#describeStopped(boolean) */ - Collection<Description> shutdown( boolean shutdownNow ) + Collection<Description> describeStopped( boolean shutdownNow ) { - return master.shutdown( shutdownNow ); + Collection<Description> activeChildren = + new ConcurrentLinkedQueue<Description>( master.describeStopped( shutdownNow ) ); + activeChildren.removeAll( UNUSED_DESCRIPTIONS ); + return activeChildren; } - private static ExecutorService newPool() + /** + * @see Scheduler#shutdownThreadPoolsAwaitingKilled() + */ + boolean shutdownThreadPoolsAwaitingKilled() { - final ThreadFactory factory = new ThreadFactory() - { - public Thread newThread( Runnable r ) - { - return new Thread( r, "maven-surefire-plugin@NotThreadSafe" ); - } - }; - return new ThreadPoolExecutor(1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<Runnable>(), factory); + return master.shutdownThreadPoolsAwaitingKilled(); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ThreadResourcesBalancer.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ThreadResourcesBalancer.java b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ThreadResourcesBalancer.java index 322d443..455874c 100644 --- a/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ThreadResourcesBalancer.java +++ b/surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ThreadResourcesBalancer.java @@ -49,9 +49,15 @@ final class ThreadResourcesBalancer * @param numPermits number of permits to acquire when maintaining concurrency on tests. * Must be >0 and < {@link Integer#MAX_VALUE}. * @param fair <tt>true</tt> guarantees the waiting schedulers to wake up in order they acquired a permit + * @throws IllegalArgumentException if <tt>numPermits</tt> is not positive number */ ThreadResourcesBalancer( int numPermits, boolean fair ) { + if ( numPermits <= 0 ) + { + throw new IllegalArgumentException( + String.format( "numPermits=%d should be positive number", numPermits ) ); + } balancer = new Semaphore( numPermits, fair ); this.numPermits = numPermits; } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilderTest.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilderTest.java b/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilderTest.java index cad3062..ce6ef44 100644 --- a/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilderTest.java +++ b/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilderTest.java @@ -20,11 +20,13 @@ package org.apache.maven.surefire.junitcore.pc; */ import net.jcip.annotations.NotThreadSafe; +import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.Description; import org.junit.runner.JUnitCore; import org.junit.runner.Result; import org.junit.runner.RunWith; @@ -33,8 +35,12 @@ import org.junit.runners.Suite; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.Date; import java.util.Iterator; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.TimeUnit; import static org.hamcrest.core.AnyOf.anyOf; import static org.hamcrest.core.Is.is; @@ -48,6 +54,8 @@ import static org.junit.Assert.*; */ public class ParallelComputerBuilderTest { + private static final Object class1Lock = new Object(); + private static volatile boolean beforeShutdown; private static volatile Runnable shutdownTask; @@ -506,6 +514,37 @@ public class ParallelComputerBuilderTest assertThat( computer.poolCapacity, is( 10 ) ); } + @Test + public void beforeAfterThreadChanges() + throws InterruptedException + { + Collection<Thread> expectedThreads = jvmThreads(); + ParallelComputerBuilder parallelComputerBuilder = new ParallelComputerBuilder(); + parallelComputerBuilder.parallelMethods( 3 ); + ParallelComputer computer = parallelComputerBuilder.buildComputer(); + Result result = new JUnitCore().run( computer, TestWithBeforeAfter.class ); + System.out.println( new Date() + " finished test run" ); + assertTrue( result.wasSuccessful() ); + assertThat( jvmThreads(), is( expectedThreads ) ); + } + + private static Collection<Thread> jvmThreads() + { + Thread[] t = new Thread[1000]; + Thread.enumerate( t ); + ArrayList<Thread> appThreads = new ArrayList<Thread>( t.length ); + Collections.addAll( appThreads, t ); + appThreads.removeAll( Collections.singleton( null ) ); + Collections.sort( appThreads, new Comparator<Thread>() + { + public int compare( Thread t1, Thread t2 ) + { + return (int) Math.signum( t1.getId() - t2.getId() ); + } + } ); + return appThreads; + } + private static class ShutdownTest { Result run( final boolean useInterrupt ) @@ -522,7 +561,7 @@ public class ParallelComputerBuilderTest { public void run() { - Collection<org.junit.runner.Description> startedTests = computer.shutdown( useInterrupt ); + Collection<Description> startedTests = computer.describeStopped( useInterrupt ); assertThat( startedTests.size(), is( not( 0 ) ) ); } }; @@ -540,10 +579,10 @@ public class ParallelComputerBuilderTest public void test1() throws InterruptedException { - synchronized ( Class1.class ) + synchronized ( class1Lock ) { ++concurrentMethods; - Class1.class.wait( 500 ); + class1Lock.wait( 500 ); maxConcurrentMethods = Math.max( maxConcurrentMethods, concurrentMethods-- ); } } @@ -764,4 +803,52 @@ public class ParallelComputerBuilderTest assertThat( Thread.currentThread().getName(), is( not( "maven-surefire-plugin@NotThreadSafe" ) ) ); } } + + public static class TestWithBeforeAfter + { + @BeforeClass + public static void beforeClass() + throws InterruptedException + { + System.out.println( new Date() + " BEG: beforeClass" ); + TimeUnit.SECONDS.sleep( 1 ); + System.out.println( new Date() + " END: beforeClass" ); + } + + @Before + public void before() + throws InterruptedException + { + System.out.println( new Date() + " BEG: before" ); + TimeUnit.SECONDS.sleep( 1 ); + System.out.println( new Date() + " END: before" ); + } + + @Test + public void test() + throws InterruptedException + { + System.out.println( new Date() + " BEG: test" ); + TimeUnit.SECONDS.sleep( 1 ); + System.out.println( new Date() + " END: test" ); + } + + @After + public void after() + throws InterruptedException + { + System.out.println( new Date() + " BEG: after" ); + TimeUnit.SECONDS.sleep( 1 ); + System.out.println( new Date() + " END: after" ); + } + + @AfterClass + public static void afterClass() + throws InterruptedException + { + System.out.println( new Date() + " BEG: afterClass" ); + TimeUnit.SECONDS.sleep( 1 ); + System.out.println( new Date() + " END: afterClass" ); + } + } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/99438ac9/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerUtilTest.java ---------------------------------------------------------------------- diff --git a/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerUtilTest.java b/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerUtilTest.java index e094e11..3b92750 100644 --- a/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerUtilTest.java +++ b/surefire-providers/surefire-junit47/src/test/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerUtilTest.java @@ -995,7 +995,7 @@ public final class ParallelComputerUtilTest long timeSpent = runtime.stop(); long deltaTime = 500L; - assertEquals( 2500L, timeSpent, deltaTime ); + assertEquals( 5000L, timeSpent, deltaTime ); String description = pc.describeElapsedTimeout(); assertTrue( description.contains( "The test run has finished abruptly after timeout of 2.5 seconds.") ); assertTrue( description.contains( "These tests were executed in prior to the shutdown operation:\n" @@ -1029,7 +1029,9 @@ public final class ParallelComputerUtilTest public void timeoutAndForcedShutdown() throws TestSetFailedException, ExecutionException, InterruptedException { - // The JUnitCore returns after 2.5s and the test-methods in TestClass are interrupted after 3.5s. + // The JUnitCore returns after 3.5s and the test-methods in TestClass are timed out after 2.5s. + // No new test methods are scheduled for execution after 2.5s. + // Interruption of test methods after 3.5s. Properties properties = new Properties(); properties.setProperty( PARALLEL_KEY, "methods" ); properties.setProperty( THREADCOUNTMETHODS_KEY, "2" ); @@ -1042,13 +1044,37 @@ public final class ParallelComputerUtilTest long timeSpent = runtime.stop(); long deltaTime = 500L; - assertEquals( 2500L, timeSpent, deltaTime ); + assertEquals( 3500L, timeSpent, deltaTime ); String description = pc.describeElapsedTimeout(); assertTrue( description.contains( "The test run has finished abruptly after timeout of 2.5 seconds.") ); assertTrue( description.contains( "These tests were executed in prior to the shutdown operation:\n" + TestClass.class.getName() ) ); } + @Test + public void forcedTimeoutAndShutdown() + throws TestSetFailedException, ExecutionException, InterruptedException + { + // The JUnitCore returns after 3.5s and the test-methods in TestClass are interrupted after 3.5s. + Properties properties = new Properties(); + properties.setProperty( PARALLEL_KEY, "methods" ); + properties.setProperty( THREADCOUNTMETHODS_KEY, "2" ); + properties.setProperty( PARALLEL_TIMEOUTFORCED_KEY, Double.toString( 3.5d ) ); + properties.setProperty( PARALLEL_TIMEOUT_KEY, Double.toString( 4.0d ) ); + JUnitCoreParameters params = new JUnitCoreParameters( properties ); + ParallelComputerBuilder pcBuilder = new ParallelComputerBuilder( params ); + ParallelComputer pc = pcBuilder.buildComputer(); + new JUnitCore().run( pc, TestClass.class ); + long timeSpent = runtime.stop(); + long deltaTime = 500L; + + assertEquals( 3500L, timeSpent, deltaTime ); + String description = pc.describeElapsedTimeout(); + assertTrue( description.contains( "The test run has finished abruptly after timeout of 3.5 seconds.") ); + assertTrue( description.contains( "These tests were executed in prior to the shutdown operation:\n" + + TestClass.class.getName() ) ); + } + public static class TestClass { @Test