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 2018/03/18 13:46:09 UTC
[maven-surefire] 01/01: [SUREFIRE-1503] Forked JVM immediately
crashed on Unix/Linux due to new shutdown mechanism does not turn to the
old shutdown mechanism
This is an automated email from the ASF dual-hosted git repository.
tibordigana pushed a commit to branch SUREFIRE-1503
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit ff4fd1f2bb5d38efad3bfd6223a3f521ca489b2c
Author: Tibor17 <ti...@apache.org>
AuthorDate: Sun Mar 18 14:39:15 2018 +0100
[SUREFIRE-1503] Forked JVM immediately crashed on Unix/Linux due to new shutdown mechanism does not turn to the old shutdown mechanism
---
Jenkinsfile | 4 +-
.../surefire/util/internal/DumpFileUtils.java | 2 +-
.../apache/maven/surefire/booter/ForkedBooter.java | 6 +-
.../apache/maven/surefire/booter/PpidChecker.java | 118 ++++++++++++++-------
.../apache/maven/surefire/booter/ProcessInfo.java | 19 ++--
.../maven/surefire/booter/PpidCheckerTest.java | 19 +++-
6 files changed, 110 insertions(+), 58 deletions(-)
diff --git a/Jenkinsfile b/Jenkinsfile
index c33159c..585cb7f 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -27,8 +27,8 @@ properties(
)
final def oses = ['linux', 'windows']
-final def mavens = ['3.2.x', '3.3.x', '3.5.x'] // env.BRANCH_NAME == 'master' ? ['3.2.x', '3.3.x', '3.5.x'] : ['3.2.x', '3.5.x']
-final def jdks = [7, 8, 9, 10] // env.BRANCH_NAME == 'master' ? [7, 8, 9, 10] : [7, 10]
+final def mavens = env.BRANCH_NAME == 'master' ? ['3.2.x', '3.3.x', '3.5.x'] : ['3.5.x']
+final def jdks = env.BRANCH_NAME == 'master' ? [7, 8, 9, 10] : [7, 10]
final def options = ['-e', '-V', '-B', '-nsu', '-P', 'run-its']
final def goals = ['clean', 'install', 'jacoco:report']
diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/util/internal/DumpFileUtils.java b/surefire-api/src/main/java/org/apache/maven/surefire/util/internal/DumpFileUtils.java
index 302358c..c83ae1f 100644
--- a/surefire-api/src/main/java/org/apache/maven/surefire/util/internal/DumpFileUtils.java
+++ b/surefire-api/src/main/java/org/apache/maven/surefire/util/internal/DumpFileUtils.java
@@ -117,7 +117,7 @@ public final class DumpFileUtils
private static Writer createWriter( File dumpFile ) throws IOException
{
return new OutputStreamWriter( new FileOutputStream( dumpFile, true ), UTF_8 )
- .append( "# Created on " )
+ .append( "# Created at " )
.append( new SimpleDateFormat( "yyyy-MM-dd'T'HH:mm:ss.SSS" ).format( new Date() ) )
.append( StringUtils.NL );
}
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 92253d1..8a21404 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
@@ -181,14 +181,14 @@ public final class ForkedBooter
}
}
- private PingScheduler listenToShutdownCommands( Long pluginPid )
+ private PingScheduler listenToShutdownCommands( Long ppid )
{
commandReader.addShutdownListener( createExitHandler() );
AtomicBoolean pingDone = new AtomicBoolean( true );
commandReader.addNoopListener( createPingHandler( pingDone ) );
PingScheduler pingMechanisms = new PingScheduler( createPingScheduler(),
- pluginPid == null ? null : new PpidChecker( pluginPid ) );
+ ppid == null ? null : new PpidChecker( ppid ) );
if ( pingMechanisms.pluginProcessChecker != null )
{
Runnable checkerJob = processCheckerJob( pingMechanisms );
@@ -220,7 +220,7 @@ public final class ForkedBooter
catch ( Exception e )
{
DumpErrorSingleton.getSingleton()
- .dumpText( "System.exit() or native command error interrupted process checker." );
+ .dumpException( e, "System.exit() or native command error interrupted process checker." );
}
}
};
diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PpidChecker.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PpidChecker.java
index 462eacc..c163c61 100644
--- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PpidChecker.java
+++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PpidChecker.java
@@ -35,11 +35,14 @@ import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.regex.Pattern.compile;
import static org.apache.commons.io.IOUtils.closeQuietly;
+import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.commons.lang3.SystemUtils.IS_OS_UNIX;
import static org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS;
import static org.apache.maven.surefire.booter.ProcessInfo.ERR_PROCESS_INFO;
import static org.apache.maven.surefire.booter.ProcessInfo.INVALID_PROCESS_INFO;
+import static org.apache.maven.surefire.booter.ProcessInfo.unixProcessInfo;
+import static org.apache.maven.surefire.booter.ProcessInfo.windowsProcessInfo;
/**
* Recognizes PID of Plugin process and determines lifetime.
@@ -63,30 +66,30 @@ final class PpidChecker
*/
static final Pattern UNIX_CMD_OUT_PATTERN = compile( "^(((\\d+)-)?(\\d{1,2}):)?(\\d{1,2}):(\\d{1,2})$" );
- private final long pluginPid;
+ private final long ppid;
- private volatile ProcessInfo pluginProcessInfo;
+ private volatile ProcessInfo parentProcessInfo;
private volatile boolean stopped;
- PpidChecker( long pluginPid )
+ PpidChecker( long ppid )
{
- this.pluginPid = pluginPid;
+ this.ppid = ppid;
//todo WARN logger (after new logger is finished) that (IS_OS_UNIX && canExecuteUnixPs()) is false
}
boolean canUse()
{
- return pluginProcessInfo == null
- ? IS_OS_WINDOWS || IS_OS_UNIX && canExecuteUnixPs()
- : pluginProcessInfo.isValid() && !pluginProcessInfo.isError();
+ final ProcessInfo ppi = parentProcessInfo;
+ return ppi == null ? IS_OS_WINDOWS || IS_OS_UNIX && canExecuteUnixPs() : ppi.canUse();
}
/**
* This method can be called only after {@link #canUse()} has returned {@code true}.
*
* @return {@code true} if parent process is alive; {@code false} otherwise
- * @throws IllegalStateException if {@link #canUse()} returns {@code false}
- * or the object has been {@link #destroyActiveCommands() destroyed}
+ * @throws IllegalStateException if {@link #canUse()} returns {@code false}, error to read process
+ * or this object has been {@link #destroyActiveCommands() destroyed}
+ * @throws NullPointerException if extracted e-time is null
*/
@SuppressWarnings( "unchecked" )
boolean isProcessAlive()
@@ -96,35 +99,65 @@ final class PpidChecker
throw new IllegalStateException( "irrelevant to call isProcessAlive()" );
}
- if ( IS_OS_WINDOWS )
+ final ProcessInfo previousInfo = parentProcessInfo;
+ try
{
- ProcessInfo previousPluginProcessInfo = pluginProcessInfo;
- pluginProcessInfo = windows();
- if ( isStopped() || pluginProcessInfo.isError() )
+ if ( IS_OS_WINDOWS )
{
- throw new IllegalStateException( "error to read process" );
+ parentProcessInfo = windows();
+
+ if ( isStopped() )
+ {
+ throw new IllegalStateException( "error [STOPPED] to read process " + ppid );
+ }
+
+ if ( parentProcessInfo.isError() )
+ {
+ throw new IllegalStateException( "error to read process " + ppid );
+ }
+
+ if ( !parentProcessInfo.canUse() )
+ {
+ throw new IllegalStateException( "Cannot use PPID " + ppid + " process information. "
+ + "Going to use NOOP events." );
+ }
+
+ // let's compare creation time, should be same unless killed or PID is reused by OS into another process
+ return previousInfo == null || parentProcessInfo.isTimeEqualTo( previousInfo );
+ }
+ else if ( IS_OS_UNIX )
+ {
+ parentProcessInfo = unix();
+
+ if ( isStopped() )
+ {
+ throw new IllegalStateException( "error [STOPPED] to read process " + ppid );
+ }
+
+ if ( parentProcessInfo.isError() )
+ {
+ throw new IllegalStateException( "error to read process " + ppid );
+ }
+
+ if ( !parentProcessInfo.canUse() )
+ {
+ throw new IllegalStateException( "Cannot use PPID " + ppid + " process information. "
+ + "Going to use NOOP events." );
+ }
+
+ // let's compare elapsed time, should be greater or equal if parent process is the same and still alive
+ return previousInfo == null || !parentProcessInfo.isTimeBefore( previousInfo );
}
- // let's compare creation time, should be same unless killed or PID is reused by OS into another process
- return pluginProcessInfo.isValid()
- && ( previousPluginProcessInfo == null
- || pluginProcessInfo.isTimeEqualTo( previousPluginProcessInfo ) );
+
+ throw new IllegalStateException( "unknown platform or you did not call canUse() before isProcessAlive()" );
}
- else if ( IS_OS_UNIX )
+ finally
{
- ProcessInfo previousPluginProcessInfo = pluginProcessInfo;
- pluginProcessInfo = unix();
- if ( isStopped() || pluginProcessInfo.isError() )
+ if ( parentProcessInfo == null )
{
- throw new IllegalStateException( "error to read process" );
+ parentProcessInfo = INVALID_PROCESS_INFO;
}
- // let's compare elapsed time, should be greater or equal if parent process is the same and still alive
- return pluginProcessInfo.isValid()
- && ( previousPluginProcessInfo == null
- || pluginProcessInfo.isTimeEqualTo( previousPluginProcessInfo )
- || pluginProcessInfo.isTimeAfter( previousPluginProcessInfo ) );
}
-
- throw new IllegalStateException();
}
// https://www.freebsd.org/cgi/man.cgi?ps(1)
@@ -139,9 +172,9 @@ final class PpidChecker
ProcessInfoConsumer reader = new ProcessInfoConsumer( Charset.defaultCharset().name() )
{
@Override
- ProcessInfo consumeLine( String line, ProcessInfo previousProcessInfo )
+ ProcessInfo consumeLine( String line, ProcessInfo previousOutputLine )
{
- if ( !previousProcessInfo.isValid() )
+ if ( previousOutputLine.isInvalid() )
{
Matcher matcher = UNIX_CMD_OUT_PATTERN.matcher( line );
if ( matcher.matches() )
@@ -150,14 +183,14 @@ final class PpidChecker
+ fromHours( matcher )
+ fromMinutes( matcher )
+ fromSeconds( matcher );
- return ProcessInfo.unixProcessInfo( pluginPid, pidUptime );
+ previousOutputLine = unixProcessInfo( ppid, pidUptime );
}
}
- return previousProcessInfo;
+ return previousOutputLine;
}
};
- return reader.execute( "/bin/sh", "-c", unixPathToPS() + " -o etime= -p " + pluginPid );
+ return reader.execute( "/bin/sh", "-c", unixPathToPS() + " -o etime= -p " + ppid );
}
ProcessInfo windows()
@@ -167,9 +200,9 @@ final class PpidChecker
private boolean hasHeader;
@Override
- ProcessInfo consumeLine( String line, ProcessInfo previousProcessInfo )
+ ProcessInfo consumeLine( String line, ProcessInfo previousOutputLine )
{
- if ( !previousProcessInfo.isValid() )
+ if ( previousOutputLine.isInvalid() )
{
StringTokenizer args = new StringTokenizer( line );
if ( args.countTokens() == 1 )
@@ -177,7 +210,10 @@ final class PpidChecker
if ( hasHeader )
{
String startTimestamp = args.nextToken();
- return ProcessInfo.windowsProcessInfo( pluginPid, startTimestamp );
+ previousOutputLine =
+ isBlank( startTimestamp )
+ ? INVALID_PROCESS_INFO
+ : windowsProcessInfo( ppid, startTimestamp.trim() );
}
else
{
@@ -185,10 +221,10 @@ final class PpidChecker
}
}
}
- return previousProcessInfo;
+ return previousOutputLine;
}
};
- String pid = String.valueOf( pluginPid );
+ String pid = String.valueOf( ppid );
String wmicPath = hasWmicStandardSystemPath() ? SYSTEM_PATH_TO_WMIC : "";
return reader.execute( "CMD", "/A", "/X", "/C",
wmicPath + "wmic process where (ProcessId=" + pid + ") get " + WMIC_CREATION_DATE
@@ -285,7 +321,7 @@ final class PpidChecker
this.charset = charset;
}
- abstract ProcessInfo consumeLine( String line, ProcessInfo previousProcessInfo );
+ abstract ProcessInfo consumeLine( String line, ProcessInfo previousOutputLine );
ProcessInfo execute( String... command )
{
diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ProcessInfo.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ProcessInfo.java
index 212e221..f076d0e 100644
--- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ProcessInfo.java
+++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ProcessInfo.java
@@ -25,9 +25,9 @@ import static org.apache.maven.surefire.util.internal.ObjectUtils.requireNonNull
* Immutable object which encapsulates PID and elapsed time (Unix) or start time (Windows).
* <br>
* Methods
- * ({@link #getPID()}, {@link #getTime()}, {@link #isTimeAfter(ProcessInfo)}, {@link #isTimeEqualTo(ProcessInfo)})
+ * ({@link #getPID()}, {@link #getTime()}, {@link #isTimeBefore(ProcessInfo)}, {@link #isTimeEqualTo(ProcessInfo)})
* throw {@link IllegalStateException}
- * if {@link #isValid()} returns {@code false} or {@link #isError()} returns {@code true}.
+ * if {@link #canUse()} returns {@code false} or {@link #isError()} returns {@code true}.
*
* @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
* @since 2.20.1
@@ -61,9 +61,14 @@ final class ProcessInfo
this.time = time;
}
- boolean isValid()
+ boolean canUse()
{
- return this != INVALID_PROCESS_INFO;
+ return !isInvalid() && !isError();
+ }
+
+ boolean isInvalid()
+ {
+ return this == INVALID_PROCESS_INFO;
}
boolean isError()
@@ -92,16 +97,16 @@ final class ProcessInfo
}
@SuppressWarnings( "unchecked" )
- boolean isTimeAfter( ProcessInfo that )
+ boolean isTimeBefore( ProcessInfo that )
{
checkValid();
that.checkValid();
- return this.time.compareTo( that.time ) > 0;
+ return this.time.compareTo( that.time ) < 0;
}
private void checkValid()
{
- if ( !isValid() || isError() )
+ if ( !canUse() )
{
throw new IllegalStateException( "invalid process info" );
}
diff --git a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PpidCheckerTest.java b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PpidCheckerTest.java
index 84f0837..fac18e2 100644
--- a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PpidCheckerTest.java
+++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PpidCheckerTest.java
@@ -19,7 +19,9 @@ package org.apache.maven.surefire.booter;
* under the License.
*/
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
import java.io.File;
import java.lang.management.ManagementFactory;
@@ -31,6 +33,7 @@ import static org.fest.assertions.Assertions.assertThat;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.junit.Assert.fail;
import static org.junit.Assume.assumeThat;
import static org.junit.Assume.assumeTrue;
import static org.powermock.reflect.Whitebox.invokeMethod;
@@ -43,6 +46,9 @@ import static org.powermock.reflect.Whitebox.invokeMethod;
*/
public class PpidCheckerTest
{
+ @Rule
+ public final ExpectedException exceptions = ExpectedException.none();
+
@Test
public void shouldHavePpidAsWindows()
{
@@ -102,14 +108,19 @@ public class PpidCheckerTest
@Test
public void shouldNotFindSuchPID()
{
- PpidChecker checker = new PpidChecker( 1000000L );
+ long ppid = 1000000L;
+
+ PpidChecker checker = new PpidChecker( ppid );
+
assertThat( checker.canUse() )
.isTrue();
- boolean isAlive = checker.isProcessAlive();
+ exceptions.expect( IllegalStateException.class );
+ exceptions.expectMessage( "Cannot use PPID " + ppid + " process information. Going to use NOOP events." );
- assertThat( isAlive )
- .isFalse();
+ checker.isProcessAlive();
+
+ fail( "this test should throw exception" );
}
@Test
--
To stop receiving notification emails like this one, please contact
tibordigana@apache.org.