Repository: maven-surefire Updated Branches: refs/heads/SUREFIRE-1422 [created] 27f68718b
SUREFIRE-1422 Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/27f68718 Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/27f68718 Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/27f68718 Branch: refs/heads/SUREFIRE-1422 Commit: 27f68718b7cd7d73ae7488fb8b971c3d74becdef Parents: aee2bb4 Author: Tibor17 <tibordig...@apache.org> Authored: Sat Mar 10 17:10:35 2018 +0100 Committer: Tibor17 <tibordig...@apache.org> Committed: Sat Mar 10 17:10:35 2018 +0100 ---------------------------------------------------------------------- .../surefire/util/internal/DumpFileUtils.java | 2 +- .../maven/surefire/booter/ForkedBooter.java | 6 +- .../maven/surefire/booter/PpidChecker.java | 118 ++++++++++++------- .../maven/surefire/booter/ProcessInfo.java | 19 +-- .../maven/surefire/booter/PpidCheckerTest.java | 19 ++- 5 files changed, 108 insertions(+), 56 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/27f68718/surefire-api/src/main/java/org/apache/maven/surefire/util/internal/DumpFileUtils.java ---------------------------------------------------------------------- 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 ); } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/27f68718/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 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." ); } } }; http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/27f68718/surefire-booter/src/main/java/org/apache/maven/surefire/booter/PpidChecker.java ---------------------------------------------------------------------- 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 ) { http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/27f68718/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ProcessInfo.java ---------------------------------------------------------------------- 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:tibordig...@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" ); } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/27f68718/surefire-booter/src/test/java/org/apache/maven/surefire/booter/PpidCheckerTest.java ---------------------------------------------------------------------- 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
