You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ti...@apache.org on 2022/02/09 08:54:20 UTC

[maven-surefire] branch master updated: [SUREFIRE-1945] crashed tests - unit tests with large logging output does not produce surefire report

This is an automated email from the ASF dual-hosted git repository.

tibordigana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git


The following commit(s) were added to refs/heads/master by this push:
     new 8221223  [SUREFIRE-1945] crashed tests - unit tests with large logging output does not produce surefire report
8221223 is described below

commit 82212233fe18859bae13d66feecd58123e9df794
Author: tibor.digana <ti...@apache.org>
AuthorDate: Tue Feb 8 21:41:12 2022 +0100

    [SUREFIRE-1945] crashed tests - unit tests with large logging output does not produce surefire report
---
 .../surefire/booterclient/output/ForkClient.java   |  2 +-
 .../output/ThreadedStreamConsumer.java             | 31 +++++++++++++---------
 .../surefire/extensions/EventConsumerThread.java   |  7 ++++-
 .../apache/maven/surefire/booter/ForkedBooter.java | 14 ++++------
 .../maven/surefire/booter/ForkedBooterTest.java    |  6 ++---
 .../extensions/util/LineConsumerThread.java        | 10 ++++++-
 6 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java
index 1011fbf..9a2065b 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java
@@ -310,7 +310,7 @@ public class ForkClient
         public void handle( StackTraceWriter stackTrace )
         {
             getOrCreateConsoleLogger()
-                .error( "System Exit has timed out in the forked process " + forkNumber );
+                .error( "The surefire booter JVM" + forkNumber + " was interrupted and exits." );
         }
     }
 
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java
index 742aad2..f1f6722 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java
@@ -26,6 +26,7 @@ import javax.annotation.Nonnull;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.concurrent.ConcurrentLinkedDeque;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.AbstractQueuedSynchronizer;
@@ -50,8 +51,9 @@ public final class ThreadedStreamConsumer
 
     private final QueueSynchronizer<Event> synchronizer = new QueueSynchronizer<>( QUEUE_MAX_ITEMS, END_ITEM );
     private final AtomicBoolean stop = new AtomicBoolean();
-    private final AtomicBoolean isAlive = new AtomicBoolean( true );
+    private final CountDownLatch threadJoiner = new CountDownLatch( 1 );
     private final Pumper pumper;
+    private volatile boolean isAlive = true;
 
     final class Pumper
         implements Runnable
@@ -100,7 +102,8 @@ public final class ThreadedStreamConsumer
                 }
             }
 
-            isAlive.set( false );
+            threadJoiner.countDown();
+            isAlive = false;
         }
 
         boolean hasErrors()
@@ -124,28 +127,32 @@ public final class ThreadedStreamConsumer
     @Override
     public void handleEvent( @Nonnull Event event )
     {
-        if ( stop.get() )
-        {
-            return;
-        }
         // Do NOT call Thread.isAlive() - slow.
         // It makes worse performance from 790 millis to 1250 millis for 5 million messages.
-        else if ( !isAlive.get() )
+        if ( !stop.get() && isAlive )
         {
-            synchronizer.clearQueue();
-            return;
+            synchronizer.pushNext( event );
         }
-
-        synchronizer.pushNext( event );
     }
 
     @Override
     public void close()
         throws IOException
     {
-        if ( stop.compareAndSet( false, true ) )
+        if ( stop.compareAndSet( false, true ) && isAlive )
         {
             synchronizer.markStopped();
+
+            try
+            {
+                threadJoiner.await();
+            }
+            catch ( InterruptedException e )
+            {
+                Thread.currentThread().interrupt();
+            }
+
+            synchronizer.clearQueue();
         }
 
         if ( pumper.hasErrors() )
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java
index a2b1c02..79d5c85 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java
@@ -30,6 +30,7 @@ import org.apache.maven.surefire.stream.EventDecoder;
 import javax.annotation.Nonnull;
 import java.io.EOFException;
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.nio.channels.ClosedChannelException;
 import java.nio.channels.ReadableByteChannel;
 
@@ -83,7 +84,11 @@ public class EventConsumerThread extends CloseableDaemonThread
         }
         catch ( IOException e )
         {
-            if ( !( e.getCause() instanceof InterruptedException ) )
+            if ( e instanceof InterruptedIOException || e.getCause() instanceof InterruptedException )
+            {
+                Thread.currentThread().interrupt();
+            }
+            else
             {
                 arguments.dumpStreamException( e );
             }
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 fe64a95..6712ef1 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
@@ -55,10 +55,8 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import static java.lang.Math.max;
 import static java.lang.Thread.currentThread;
 import static java.util.ServiceLoader.load;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.maven.surefire.api.cli.CommandLineOption.LOGGING_LEVEL_DEBUG;
 import static org.apache.maven.surefire.api.util.ReflectionUtils.instantiateOneArg;
@@ -83,7 +81,6 @@ public final class ForkedBooter
 {
     private static final long DEFAULT_SYSTEM_EXIT_TIMEOUT_IN_SECONDS = 30L;
     private static final long PING_TIMEOUT_IN_SECONDS = 30L;
-    private static final long ONE_SECOND_IN_MILLIS = 1_000L;
     private static final String LAST_DITCH_SHUTDOWN_THREAD = "surefire-forkedjvm-last-ditch-daemon-shutdown-thread-";
     private static final String PING_THREAD = "surefire-forkedjvm-ping-";
     private static final String PROCESS_CHECKER_THREAD = "surefire-process-checker";
@@ -442,9 +439,8 @@ public final class ForkedBooter
         );
         eventChannel.bye();
         launchLastDitchDaemonShutdownThread( 0 );
-        long timeoutMillis = max( systemExitTimeoutInSeconds * ONE_SECOND_IN_MILLIS, ONE_SECOND_IN_MILLIS );
-        boolean timeoutElapsed = !acquireOnePermit( exitBarrier, timeoutMillis );
-        if ( timeoutElapsed && !eventChannel.checkError() )
+        boolean byeAckReceived = acquireOnePermit( exitBarrier );
+        if ( !byeAckReceived && !eventChannel.checkError() )
         {
             eventChannel.sendExitError( null, false );
         }
@@ -616,16 +612,16 @@ public final class ForkedBooter
         return pluginProcessChecker != null && pluginProcessChecker.canUse();
     }
 
-    private static boolean acquireOnePermit( Semaphore barrier, long timeoutMillis )
+    private static boolean acquireOnePermit( Semaphore barrier )
     {
         try
         {
-            return barrier.tryAcquire( timeoutMillis, MILLISECONDS );
+            return barrier.tryAcquire( Integer.MAX_VALUE, SECONDS );
         }
         catch ( InterruptedException e )
         {
             // cancel schedulers, stop the command reader and exit 0
-            return true;
+            return false;
         }
     }
 
diff --git a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ForkedBooterTest.java b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ForkedBooterTest.java
index da3516e..b5a1851 100644
--- a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ForkedBooterTest.java
+++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ForkedBooterTest.java
@@ -150,7 +150,7 @@ public class ForkedBooterTest
     public void testBarrier1() throws Exception
     {
         Semaphore semaphore = new Semaphore( 2 );
-        boolean acquiredOnePermit = invokeMethod( ForkedBooter.class, "acquireOnePermit", semaphore, 30_000L );
+        boolean acquiredOnePermit = invokeMethod( ForkedBooter.class, "acquireOnePermit", semaphore );
 
         assertThat( acquiredOnePermit ).isTrue();
         assertThat( semaphore.availablePermits() ).isEqualTo( 1 );
@@ -163,9 +163,9 @@ public class ForkedBooterTest
         Thread.currentThread().interrupt();
         try
         {
-            boolean acquiredOnePermit = invokeMethod( ForkedBooter.class, "acquireOnePermit", semaphore, 30_000L );
+            boolean acquiredOnePermit = invokeMethod( ForkedBooter.class, "acquireOnePermit", semaphore );
 
-            assertThat( acquiredOnePermit ).isTrue();
+            assertThat( acquiredOnePermit ).isFalse();
             assertThat( semaphore.availablePermits() ).isEqualTo( 0 );
         }
         finally
diff --git a/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/util/LineConsumerThread.java b/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/util/LineConsumerThread.java
index 4cb1e88..41558f0 100644
--- a/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/util/LineConsumerThread.java
+++ b/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/util/LineConsumerThread.java
@@ -24,6 +24,7 @@ import org.apache.maven.surefire.extensions.EventHandler;
 
 import javax.annotation.Nonnull;
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.charset.Charset;
 import java.util.Scanner;
@@ -81,7 +82,14 @@ public final class LineConsumerThread extends CloseableDaemonThread
                 }
             }
         }
-        catch ( IllegalStateException | IOException e )
+        catch ( IOException e )
+        {
+            if ( e instanceof InterruptedIOException || e.getCause() instanceof InterruptedException )
+            {
+                Thread.currentThread().interrupt();
+            }
+        }
+        catch ( IllegalStateException e )
         {
             // not needed
         }