SUREFIRE-1101 Surefire does not shutdown thread-pools programatically af...

surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/AbstractThreadPoolStrategy.java

@@ -23,7 +23,7 @@
 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 @@ protected final Collection<Future<?>> getFutureResults()
         return futureResults;
     }
 
-    protected final void disable()
-    {
-        canSchedule.set( false );
-    }
-
     @Override
     public void schedule( Runnable task )
     {
@@ -87,7 +82,7 @@ public void schedule( Runnable task )
     @Override
     protected boolean stop()
     {
-        boolean wasRunning = canSchedule.getAndSet( false );
+        boolean wasRunning = disable();
         if ( threadPool.isShutdown() )
         {
             wasRunning = false;
@@ -102,7 +97,7 @@ protected boolean stop()
     @Override
     protected boolean stopNow()
     {
-        boolean wasRunning = canSchedule.getAndSet( false );
+        boolean wasRunning = disable();
         if ( threadPool.isShutdown() )
         {
             wasRunning = false;
@@ -114,6 +109,9 @@ protected boolean stopNow()
         return wasRunning;
     }
 
+    /**
+     * @see Scheduler.ShutdownHandler
+     */
     @Override
     protected void setDefaultShutdownHandler( Scheduler.ShutdownHandler handler )
     {
@@ -125,9 +123,21 @@ protected void setDefaultShutdownHandler( Scheduler.ShutdownHandler handler )
         }
     }
 
-    @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;
+        }
     }
 }

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.digana@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();
+}

surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/InvokerStrategy.java

@@ -21,7 +21,6 @@
 
 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 @@
 final class InvokerStrategy
     extends SchedulingStrategy
 {
-    private final AtomicBoolean canSchedule = new AtomicBoolean( true );
 
     private final Queue<Thread> activeThreads = new ConcurrentLinkedQueue<Thread>();
 
@@ -58,13 +56,13 @@ public void schedule( Runnable task )
     @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 @@ public boolean hasSharedThreadPool()
     }
 
     @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();
     }

surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/NonSharedThreadPoolStrategy.java

@@ -47,16 +47,9 @@ public boolean hasSharedThreadPool()
     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;
     }
 }

surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputer.java

@@ -59,45 +59,8 @@ public ParallelComputer( double timeoutInSeconds, double timeoutForcedInSeconds
         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 @@ protected final boolean afterRunQuietly()
     {
         shutdownStatus.tryFinish();
         forcedShutdownStatus.tryFinish();
+        boolean notInterrupted = true;
         if ( shutdownScheduler != null )
         {
             shutdownScheduler.shutdownNow();
@@ -123,10 +87,11 @@ protected final boolean afterRunQuietly()
             }
             catch ( InterruptedException e )
             {
-                return false;
+                notInterrupted = false;
             }
         }
-        return true;
+        notInterrupted &= shutdownThreadPoolsAwaitingKilled();
+        return notInterrupted;
     }
 
     public String describeElapsedTimeout()
@@ -203,7 +168,7 @@ public Collection<Description> call()
                 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 Collection<Description> call()
                 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 @@ private boolean hasTimeoutForced()
     {
         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() );
+                }
+            }
+        }
+    }
 }

surefire-providers/surefire-junit47/src/main/java/org/apache/maven/surefire/junitcore/pc/ParallelComputerBuilder.java

@@ -64,10 +64,10 @@
  * {@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,17 +254,29 @@ private PC()
         }
 
         @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