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 <tibo...@lycos.com> Authored: Sat May 13 18:41:30 2017 +0200 Committer: Tibor17 <tibo...@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:tibordig...@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