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 2019/12/29 00:41:33 UTC
[maven-surefire] 01/02: [SUREFIRE-1719] Race condition results in
"VM crash or System.exit called?" failure
This is an automated email from the ASF dual-hosted git repository.
tibordigana pushed a commit to branch cli
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit 85539f1d367cf524982017c228eb58e9ebe0fb0d
Author: tibordigana <ti...@apache.org>
AuthorDate: Sun Dec 29 00:05:15 2019 +0100
[SUREFIRE-1719] Race condition results in "VM crash or System.exit called?" failure
---
.../plugin/surefire/booterclient/ForkStarter.java | 4 +-
.../lazytestprovider/NotifiableTestStream.java | 10 ++-
.../lazytestprovider/TestLessInputStream.java | 74 +++++++---------------
.../TestLessInputStreamBuilderTest.java | 53 ++++++++++++++--
4 files changed, 81 insertions(+), 60 deletions(-)
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 f2734ee..0ba2f46 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
@@ -436,7 +436,8 @@ public class ForkStarter
DefaultReporterFactory forkedReporterFactory =
new DefaultReporterFactory( startupReportConfiguration, log, forkNumber );
defaultReporterFactories.add( forkedReporterFactory );
- ForkClient forkClient = new ForkClient( forkedReporterFactory, builder.getImmediateCommands(),
+ TestLessInputStream stream = builder.build();
+ ForkClient forkClient = new ForkClient( forkedReporterFactory, stream,
log, printedErrorStream, forkNumber )
{
@Override
@@ -448,7 +449,6 @@ public class ForkStarter
}
}
};
- TestLessInputStream stream = builder.build();
try
{
return fork( testSet,
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/NotifiableTestStream.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/NotifiableTestStream.java
index b181de1..add5a63 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/NotifiableTestStream.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/NotifiableTestStream.java
@@ -22,15 +22,23 @@ package org.apache.maven.plugin.surefire.booterclient.lazytestprovider;
import org.apache.maven.surefire.booter.Shutdown;
/**
- * Forked jvm notifies master process to provide a new test.
+ * Remote interface of forked JVM with command methods.
+ * <br>
+ * Implemented by {@link TestProvidingInputStream} and {@link TestLessInputStream} where the method
+ * {@link TestLessInputStream#provideNewTest()} purposefully does nothing. Some methods in
+ * {@link org.apache.maven.plugin.surefire.booterclient.lazytestprovider.TestLessInputStream.TestLessInputStreamBuilder}
+ * throw {@link UnsupportedOperationException}.
*
* @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
* @since 2.19
* @see TestProvidingInputStream
+ * @see TestLessInputStream
*/
public interface NotifiableTestStream
{
/**
+ * Forked jvm notifies master process to provide a new test.
+ * <br>
* Notifies {@link TestProvidingInputStream} in order to dispatch a new test back to the forked
* jvm (particular fork which hits this call); or do nothing in {@link TestLessInputStream}.
*/
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java
index 3014486..372ce00 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStream.java
@@ -228,11 +228,27 @@ public final class TestLessInputStream
}
}
+ /**
+ * Only {@link NotifiableTestStream#noop()} and {@link NotifiableTestStream#shutdown(Shutdown)} are supported.
+ * Another methods throw {@link UnsupportedOperationException}.
+ *
+ * @return commands which are immediately transmitted once to all alive forked JVMs, not cached. As opposite
+ * to cached commands, the immediate commands disappear and cannot be seen by any fork initiated after
+ * the command has dispatched.
+ */
public NotifiableTestStream getImmediateCommands()
{
return immediateCommands;
}
+ /**
+ * Cached commands are sent to all alive or future alive forks. These are termination commands which are not
+ * reversible and therefore only {@link NotifiableTestStream#shutdown(Shutdown)} and
+ * {@link NotifiableTestStream#skipSinceNextTest()} are supported.
+ * Another methods throw {@link UnsupportedOperationException}.
+ *
+ * @return commands which are cached for currently alive or future forks.
+ */
public NotifiableTestStream getCachableCommands()
{
return cachableCommands;
@@ -318,24 +334,13 @@ public final class TestLessInputStream
@Override
public void provideNewTest()
{
+ throw new UnsupportedOperationException();
}
@Override
public void skipSinceNextTest()
{
- Lock lock = rwLock.readLock();
- lock.lock();
- try
- {
- for ( TestLessInputStream aliveStream : TestLessInputStreamBuilder.this.aliveStreams )
- {
- aliveStream.skipSinceNextTest();
- }
- }
- finally
- {
- lock.unlock();
- }
+ throw new UnsupportedOperationException();
}
@Override
@@ -377,19 +382,7 @@ public final class TestLessInputStream
@Override
public void acknowledgeByeEventReceived()
{
- Lock lock = rwLock.readLock();
- lock.lock();
- try
- {
- for ( TestLessInputStream aliveStream : TestLessInputStreamBuilder.this.aliveStreams )
- {
- aliveStream.acknowledgeByeEventReceived();
- }
- }
- finally
- {
- lock.unlock();
- }
+ throw new UnsupportedOperationException();
}
}
@@ -402,6 +395,7 @@ public final class TestLessInputStream
@Override
public void provideNewTest()
{
+ throw new UnsupportedOperationException();
}
@Override
@@ -443,37 +437,13 @@ public final class TestLessInputStream
@Override
public void noop()
{
- Lock lock = rwLock.readLock();
- lock.lock();
- try
- {
- if ( TestLessInputStreamBuilder.this.addTailNodeIfAbsent( NOOP ) )
- {
- release();
- }
- }
- finally
- {
- lock.unlock();
- }
+ throw new UnsupportedOperationException();
}
@Override
public void acknowledgeByeEventReceived()
{
- Lock lock = rwLock.readLock();
- lock.lock();
- try
- {
- if ( TestLessInputStreamBuilder.this.addTailNodeIfAbsent( BYE_ACK ) )
- {
- release();
- }
- }
- finally
- {
- lock.unlock();
- }
+ throw new UnsupportedOperationException();
}
private void release()
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStreamBuilderTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStreamBuilderTest.java
index 5d9b5af..bbc85d4 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStreamBuilderTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/lazytestprovider/TestLessInputStreamBuilderTest.java
@@ -33,10 +33,10 @@ import java.util.NoSuchElementException;
import static org.apache.maven.plugin.surefire.booterclient.lazytestprovider.TestLessInputStream.TestLessInputStreamBuilder;
import static org.apache.maven.surefire.booter.Command.NOOP;
import static org.apache.maven.surefire.booter.Command.SKIP_SINCE_NEXT_TEST;
-import static org.apache.maven.surefire.booter.MasterProcessCommand.BYE_ACK;
import static org.apache.maven.surefire.booter.MasterProcessCommand.SHUTDOWN;
import static org.apache.maven.surefire.booter.MasterProcessCommand.decode;
import static org.apache.maven.surefire.booter.Shutdown.EXIT;
+import static org.apache.maven.surefire.booter.Shutdown.KILL;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
@@ -69,9 +69,9 @@ public class TestLessInputStreamBuilderTest
assertFalse( iterator.hasNext() );
- builder.getCachableCommands().noop();
+ builder.getCachableCommands().shutdown( KILL );
assertTrue( iterator.hasNext() );
- assertThat( iterator.next(), is( NOOP ) );
+ assertThat( iterator.next(), is( new Command( SHUTDOWN, "KILL" ) ) );
builder.removeStream( is );
}
@@ -138,14 +138,57 @@ public class TestLessInputStreamBuilderTest
{
TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
TestLessInputStream pluginIs = builder.build();
- builder.getImmediateCommands().acknowledgeByeEventReceived();
+ builder.getImmediateCommands().shutdown( KILL );
builder.getImmediateCommands().noop();
DataInputStream is = new DataInputStream( pluginIs );
Command bye = decode( is );
assertThat( bye, is( notNullValue() ) );
- assertThat( bye.getCommandType(), is( BYE_ACK ) );
+ assertThat( bye.getCommandType(), is( SHUTDOWN ) );
+ assertThat( bye.getData(), is( KILL.name() ) );
Command noop = decode( is );
assertThat( noop, is( notNullValue() ) );
assertThat( noop.getCommandType(), is( MasterProcessCommand.NOOP ) );
}
+
+ @Test( expected = UnsupportedOperationException.class )
+ public void shouldThrowUnsupportedException1()
+ {
+ TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
+ builder.getImmediateCommands().provideNewTest();
+ }
+
+ @Test( expected = UnsupportedOperationException.class )
+ public void shouldThrowUnsupportedException2()
+ {
+ TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
+ builder.getImmediateCommands().skipSinceNextTest();
+ }
+
+ @Test( expected = UnsupportedOperationException.class )
+ public void shouldThrowUnsupportedException3()
+ {
+ TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
+ builder.getImmediateCommands().acknowledgeByeEventReceived();
+ }
+
+ @Test( expected = UnsupportedOperationException.class )
+ public void shouldThrowUnsupportedException4()
+ {
+ TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
+ builder.getCachableCommands().acknowledgeByeEventReceived();
+ }
+
+ @Test( expected = UnsupportedOperationException.class )
+ public void shouldThrowUnsupportedException5()
+ {
+ TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
+ builder.getCachableCommands().provideNewTest();
+ }
+
+ @Test( expected = UnsupportedOperationException.class )
+ public void shouldThrowUnsupportedException6()
+ {
+ TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder();
+ builder.getCachableCommands().noop();
+ }
}