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 2017/05/13 16:41:54 UTC

maven-surefire git commit: [SUREFIRE-1302] Surefire does not wait long enough for the forked VM and assumes it to be dead

Repository: maven-surefire
Updated Branches:
  refs/heads/SUREFIRE-1302 [created] b98a22bc3


[SUREFIRE-1302] Surefire does not wait long enough for the forked VM and assumes it to be dead


Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo
Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/b98a22bc
Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/b98a22bc
Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/b98a22bc

Branch: refs/heads/SUREFIRE-1302
Commit: b98a22bc3e9a9e6619635236ce0c1c3925bd3376
Parents: d6f3816
Author: Tibor17 <ti...@lycos.com>
Authored: Sat May 13 18:41:30 2017 +0200
Committer: Tibor17 <ti...@lycos.com>
Committed: Sat May 13 18:41:30 2017 +0200

----------------------------------------------------------------------
 .../surefire/booterclient/ForkStarter.java      |   2 +-
 .../maven/surefire/booter/CommandReader.java    |  13 ---
 surefire-booter/pom.xml                         |   5 +
 .../maven/surefire/booter/ForkedBooter.java     |  54 ++++++---
 .../maven/surefire/booter/ForkedBooterTest.java | 116 +++++++++++++++++++
 .../maven/surefire/booter/JUnit4SuiteTest.java  |   3 +-
 6 files changed, 165 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/b98a22bc/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 a2a5095..39113d0 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
@@ -116,7 +116,7 @@ public class ForkStarter
 {
     private static final String EXECUTION_EXCEPTION = "ExecutionException";
 
-    private static final long PING_IN_SECONDS = 10;
+    private static final long PING_IN_SECONDS = 1L;
 
     private static final int TIMEOUT_CHECK_PERIOD_MILLIS = 100;
 

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/b98a22bc/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java
----------------------------------------------------------------------
diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java
index ed7d4fa..460123a 100644
--- a/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java
+++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java
@@ -19,8 +19,6 @@ package org.apache.maven.surefire.booter;
  * under the License.
  */
 
-import org.apache.maven.plugin.surefire.log.api.ConsoleLogger;
-import org.apache.maven.plugin.surefire.log.api.NullConsoleLogger;
 import org.apache.maven.surefire.testset.TestSetFailedException;
 
 import java.io.DataInputStream;
@@ -53,7 +51,6 @@ import static org.apache.maven.surefire.util.internal.DaemonThreadFactory.newDae
 import static org.apache.maven.surefire.util.internal.StringUtils.encodeStringForForkCommunication;
 import static org.apache.maven.surefire.util.internal.StringUtils.isBlank;
 import static org.apache.maven.surefire.util.internal.StringUtils.isNotBlank;
-import static org.apache.maven.surefire.util.internal.ObjectUtils.requireNonNull;
 
 /**
  * Reader of commands coming from plugin(master) process.
@@ -84,8 +81,6 @@ public final class CommandReader
 
     private int iteratedCount;
 
-    private volatile ConsoleLogger logger = new NullConsoleLogger();
-
     private CommandReader()
     {
     }
@@ -106,12 +101,6 @@ public final class CommandReader
         return this;
     }
 
-    public CommandReader setLogger( ConsoleLogger logger )
-    {
-        this.logger = requireNonNull( logger, "null logger" );
-        return this;
-    }
-
     public boolean awaitStarted()
         throws TestSetFailedException
     {
@@ -393,7 +382,6 @@ public final class CommandReader
                     {
                         String errorMessage = "[SUREFIRE] std/in stream corrupted: first sequence not recognized";
                         DumpErrorSingleton.getSingleton().dumpStreamText( errorMessage );
-                        logger.error( errorMessage );
                         break;
                     }
                     else
@@ -449,7 +437,6 @@ public final class CommandReader
                 {
                     String msg = "[SUREFIRE] std/in stream corrupted";
                     DumpErrorSingleton.getSingleton().dumpStreamException( e, msg );
-                    logger.error( msg, e );
                 }
             }
             finally

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/b98a22bc/surefire-booter/pom.xml
----------------------------------------------------------------------
diff --git a/surefire-booter/pom.xml b/surefire-booter/pom.xml
index b79cceb..a0dde50 100644
--- a/surefire-booter/pom.xml
+++ b/surefire-booter/pom.xml
@@ -36,6 +36,11 @@
       <groupId>org.apache.maven.surefire</groupId>
       <artifactId>surefire-api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/b98a22bc/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 8dabaef..e5c470a 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
@@ -33,6 +33,9 @@ import java.io.FileNotFoundException;
 import java.io.InputStream;
 import java.io.PrintStream;
 import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+import java.lang.management.MemoryUsage;
+import java.lang.management.RuntimeMXBean;
 import java.lang.reflect.InvocationTargetException;
 import java.security.AccessControlException;
 import java.security.AccessController;
@@ -42,7 +45,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
 
 import static java.lang.Math.max;
 import static java.lang.System.err;
@@ -73,9 +76,16 @@ import static org.apache.maven.surefire.util.internal.StringUtils.encodeStringFo
  */
 public final class ForkedBooter
 {
+    private static final long MAX_MEM_PING = 4096L * 1024L * 1024L;
+    private static final long MIN_MEM_PING = 512L * 1024L * 1024L;
+    private static final double MIN_MEM_PING_TIME = 2d * 10d * 1000d / 7d;
+    private static final double MIN_PING_TIME = 10d * 1000d;
+    private static final double MAX_PING_TIME = 60d * 1000d;
     private static final long DEFAULT_SYSTEM_EXIT_TIMEOUT_IN_SECONDS = 30;
     private static final long PING_TIMEOUT_IN_SECONDS = 20;
     private static final long ONE_SECOND_IN_MILLIS = 1000;
+    private static final RuntimeMXBean JMX_RUNTIME = ManagementFactory.getRuntimeMXBean();
+    private static final MemoryMXBean JMX_MEM = ManagementFactory.getMemoryMXBean();
 
     private static volatile ScheduledThreadPoolExecutor jvmTerminator;
     private static volatile long systemExitTimeoutInSeconds = DEFAULT_SYSTEM_EXIT_TIMEOUT_IN_SECONDS;
@@ -97,7 +107,7 @@ public final class ForkedBooter
             final String dumpFileName = args[1];
             final String surefirePropsFileName = args[2];
 
-            BooterDeserializer booterDeserializer =
+            final BooterDeserializer booterDeserializer =
                     new BooterDeserializer( createSurefirePropertiesIfFileExists( tmpDir, surefirePropsFileName ) );
             if ( args.length > 3 )
             {
@@ -205,22 +215,22 @@ public final class ForkedBooter
     private static ExecutorService listenToShutdownCommands( CommandReader reader )
     {
         reader.addShutdownListener( createExitHandler() );
-        AtomicBoolean pingDone = new AtomicBoolean( true );
-        reader.addNoopListener( createPingHandler( pingDone ) );
-        Runnable pingJob = createPingJob( pingDone );
+        AtomicLong pingUptime = new AtomicLong( JMX_RUNTIME.getUptime() );
+        reader.addNoopListener( createPingHandler( pingUptime ) );
+        Runnable pingJob = createPingJob( pingUptime );
         ScheduledExecutorService pingScheduler = createPingScheduler();
         pingScheduler.scheduleAtFixedRate( pingJob, 0, PING_TIMEOUT_IN_SECONDS, SECONDS );
         return pingScheduler;
     }
 
-    private static CommandListener createPingHandler( final AtomicBoolean pingDone )
+    private static CommandListener createPingHandler( final AtomicLong pingUptime )
     {
         return new CommandListener()
         {
             @Override
             public void update( Command command )
             {
-                pingDone.set( true );
+                pingUptime.set( JMX_RUNTIME.getUptime() );
             }
         };
     }
@@ -246,15 +256,14 @@ public final class ForkedBooter
         };
     }
 
-    private static Runnable createPingJob( final AtomicBoolean pingDone  )
+    private static Runnable createPingJob( final AtomicLong pingUptime  )
     {
         return new Runnable()
         {
             @Override
             public void run()
             {
-                boolean hasPing = pingDone.getAndSet( false );
-                if ( !hasPing )
+                if ( isMasterProcessIdle( pingUptime.get(), JMX_MEM ) )
                 {
                     kill();
                 }
@@ -402,8 +411,8 @@ public final class ForkedBooter
 
     private static SurefireProvider createProviderInCurrentClassloader( StartupConfiguration startupConfiguration,
                                                                         boolean isInsideFork,
-                                                                       ProviderConfiguration providerConfiguration,
-                                                                       Object reporterManagerFactory )
+                                                                        ProviderConfiguration providerConfiguration,
+                                                                        Object reporterManagerFactory )
     {
         BaseProviderFactory bpf = new BaseProviderFactory( (ReporterFactory) reporterManagerFactory, isInsideFork );
         bpf.setTestRequest( providerConfiguration.getTestSuiteDefinition() );
@@ -431,7 +440,7 @@ public final class ForkedBooter
 
     private static boolean isDebugging()
     {
-        for ( String argument : ManagementFactory.getRuntimeMXBean().getInputArguments() )
+        for ( String argument : JMX_RUNTIME.getInputArguments() )
         {
             if ( "-Xdebug".equals( argument ) || argument.startsWith( "-agentlib:jdwp" ) )
             {
@@ -440,4 +449,23 @@ public final class ForkedBooter
         }
         return false;
     }
+
+    static boolean isMasterProcessIdle( long previousUptimeMillis, MemoryMXBean memoryMXBean )
+    {
+        MemoryUsage heap = memoryMXBean.getHeapMemoryUsage();
+        long committedHeap = heap.getCommitted();
+        return isMasterProcessIdle( previousUptimeMillis, committedHeap );
+    }
+
+    static boolean isMasterProcessIdle( long previousUptimeMillis, long committedHeap )
+    {
+        long currentUptimeMillis = JMX_RUNTIME.getUptime();
+
+        double idleTimeMillis = currentUptimeMillis - previousUptimeMillis;
+
+        double idleTimeMillisToKillJvm =
+                ( MAX_PING_TIME - MIN_PING_TIME ) / ( MAX_MEM_PING - MIN_MEM_PING ) * committedHeap + MIN_MEM_PING_TIME;
+
+        return idleTimeMillis > max( idleTimeMillisToKillJvm, MIN_PING_TIME );
+    }
 }

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/b98a22bc/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ForkedBooterTest.java
----------------------------------------------------------------------
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
new file mode 100644
index 0000000..59230ab
--- /dev/null
+++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/ForkedBooterTest.java
@@ -0,0 +1,116 @@
+package org.apache.maven.surefire.booter;
+
+/*
+ * 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.
+ */
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+import java.lang.management.MemoryUsage;
+
+import static org.apache.maven.surefire.booter.ForkedBooter.isMasterProcessIdle;
+import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
+ * @since 2.20.1
+ */
+@RunWith( MockitoJUnitRunner.class )
+public class ForkedBooterTest
+{
+    @Mock
+    private MemoryMXBean memoryMXBean;
+
+    @Test
+    public void test1()
+    {
+        long previousUptimeMillis = ManagementFactory.getRuntimeMXBean().getUptime() - 9000L;
+        long committedHeap = 1024L * 1024L;
+
+        assertThat( isMasterProcessIdle( previousUptimeMillis, committedHeap ) ).isFalse();
+
+        when( memoryMXBean.getHeapMemoryUsage() ).thenReturn( new MemoryUsage( 0L, 0L, committedHeap, committedHeap ) );
+        assertThat( isMasterProcessIdle( previousUptimeMillis, memoryMXBean ) ).isFalse();
+    }
+
+    @Test
+    public void test2()
+    {
+        long previousUptimeMillis = ManagementFactory.getRuntimeMXBean().getUptime() - 10001L;
+        long committedHeap = 1024L * 1024L;
+
+        assertThat( isMasterProcessIdle( previousUptimeMillis, committedHeap ) ).isTrue();
+
+        when( memoryMXBean.getHeapMemoryUsage() ).thenReturn( new MemoryUsage( 0L, 0L, committedHeap, committedHeap ) );
+        assertThat( isMasterProcessIdle( previousUptimeMillis, memoryMXBean ) ).isTrue();
+    }
+
+    @Test
+    public void test3()
+    {
+        long previousUptimeMillis = ManagementFactory.getRuntimeMXBean().getUptime() - 9000L;
+        long committedHeap = 512L * 1024L * 1024L;
+
+        assertThat( isMasterProcessIdle( previousUptimeMillis, committedHeap ) ).isFalse();
+
+        when( memoryMXBean.getHeapMemoryUsage() ).thenReturn( new MemoryUsage( 0L, 0L, committedHeap, committedHeap ) );
+        assertThat( isMasterProcessIdle( previousUptimeMillis, memoryMXBean ) ).isFalse();
+    }
+
+    @Test
+    public void test4()
+    {
+        long previousUptimeMillis = ManagementFactory.getRuntimeMXBean().getUptime() - 10001L;
+        long committedHeap = 512L * 1024L * 1024L;
+
+        assertThat( isMasterProcessIdle( previousUptimeMillis, committedHeap ) ).isTrue();
+
+        when( memoryMXBean.getHeapMemoryUsage() ).thenReturn( new MemoryUsage( 0L, 0L, committedHeap, committedHeap ) );
+        assertThat( isMasterProcessIdle( previousUptimeMillis, memoryMXBean ) ).isTrue();
+    }
+
+    @Test
+    public void test5()
+    {
+        long previousUptimeMillis = ManagementFactory.getRuntimeMXBean().getUptime() - 59000L;
+        long committedHeap = 4096L * 1024L * 1024L;
+
+        assertThat( isMasterProcessIdle( previousUptimeMillis, committedHeap ) ).isFalse();
+
+        when( memoryMXBean.getHeapMemoryUsage() ).thenReturn( new MemoryUsage( 0L, 0L, committedHeap, committedHeap ) );
+        assertThat( isMasterProcessIdle( previousUptimeMillis, memoryMXBean ) ).isFalse();
+    }
+
+    @Test
+    public void test6()
+    {
+        long previousUptimeMillis = ManagementFactory.getRuntimeMXBean().getUptime() - 60001L;
+        long committedHeap = 4096L * 1024L * 1024L;
+
+        assertThat( isMasterProcessIdle( previousUptimeMillis, committedHeap ) ).isTrue();
+
+        when( memoryMXBean.getHeapMemoryUsage() ).thenReturn( new MemoryUsage( 0L, 0L, committedHeap, committedHeap ) );
+        assertThat( isMasterProcessIdle( previousUptimeMillis, memoryMXBean ) ).isTrue();
+    }
+}

http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/b98a22bc/surefire-booter/src/test/java/org/apache/maven/surefire/booter/JUnit4SuiteTest.java
----------------------------------------------------------------------
diff --git a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/JUnit4SuiteTest.java b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/JUnit4SuiteTest.java
index 2bdcf21..3ed5686 100644
--- a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/JUnit4SuiteTest.java
+++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/JUnit4SuiteTest.java
@@ -34,7 +34,8 @@ import org.junit.runners.Suite;
     ClasspathTest.class,
     CommandReaderTest.class,
     PropertiesWrapperTest.class,
-    SurefireReflectorTest.class
+    SurefireReflectorTest.class,
+                             ForkedBooterTest.class
 } )
 @RunWith( Suite.class )
 public class JUnit4SuiteTest