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 <[email protected]> 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:[email protected]";>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 [email protected].
