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/04/30 13:37:52 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 e66c64e57871ca9756fdcd9fa6fc313bf72e2e7f
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
---
.../surefire/util/internal/DumpFileUtils.java | 2 +-
.../apache/maven/surefire/booter/ForkedBooter.java | 6 +-
.../apache/maven/surefire/booter/PpidChecker.java | 93 +++++++++++++---------
.../apache/maven/surefire/booter/ProcessInfo.java | 19 +++--
.../maven/surefire/booter/PpidCheckerTest.java | 19 ++++-
5 files changed, 87 insertions(+), 52 deletions(-)
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 b480441..00f62e5 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
@@ -74,30 +74,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()
@@ -107,35 +107,54 @@ 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();
+ checkProcessInfo();
+
+ // 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();
+ checkProcessInfo();
+
+ // 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 ) );
+ }
+ }
+
+ private void checkProcessInfo()
+ {
+ if ( isStopped() )
+ {
+ throw new IllegalStateException( "error [STOPPED] to read process " + ppid );
+ }
+
+ if ( parentProcessInfo.isError() )
+ {
+ throw new IllegalStateException( "error to read process " + ppid );
}
- throw new IllegalStateException();
+ if ( !parentProcessInfo.canUse() )
+ {
+ throw new IllegalStateException( "Cannot use PPID " + ppid + " process information. "
+ + "Going to use NOOP events." );
+ }
}
// https://www.freebsd.org/cgi/man.cgi?ps(1)
@@ -150,9 +169,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() )
@@ -161,14 +180,14 @@ final class PpidChecker
+ fromHours( matcher )
+ fromMinutes( matcher )
+ fromSeconds( matcher );
- return unixProcessInfo( pluginPid, pidUptime );
+ return 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()
@@ -180,7 +199,7 @@ final class PpidChecker
@Override
ProcessInfo consumeLine( String line, ProcessInfo previousProcessInfo ) throws Exception
{
- if ( !previousProcessInfo.isValid() && !line.isEmpty() )
+ if ( previousProcessInfo.isInvalid() && !line.isEmpty() )
{
if ( hasHeader )
{
@@ -195,7 +214,7 @@ final class PpidChecker
long startTimestampMillisUTC =
WMIC_CREATION_DATE_FORMAT.parse( startTimestamp ).getTime()
- parseInt( line.substring( indexOfTimeZone ) ) * MINUTES_TO_MILLIS;
- return windowsProcessInfo( pluginPid, startTimestampMillisUTC );
+ return windowsProcessInfo( ppid, startTimestampMillisUTC );
}
else
{
@@ -207,7 +226,7 @@ final class PpidChecker
};
String wmicPath = hasWmicStandardSystemPath() ? SYSTEM_PATH_TO_WMIC : "";
return reader.execute( "CMD", "/A", "/X", "/C",
- wmicPath + "wmic process where (ProcessId=" + pluginPid + ") get " + WMIC_CREATION_DATE
+ wmicPath + "wmic process where (ProcessId=" + ppid + ") get " + WMIC_CREATION_DATE
);
}
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 2e48599..a73c33f 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
@@ -23,9 +23,9 @@ package org.apache.maven.surefire.booter;
* 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
@@ -59,9 +59,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()
@@ -90,16 +95,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.