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:51 UTC

[maven-surefire] branch SUREFIRE-1503 updated (ff4fd1f -> e66c64e)

This is an automated email from the ASF dual-hosted git repository.

tibordigana pushed a change to branch SUREFIRE-1503
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git.


 discard ff4fd1f  [SUREFIRE-1503] Forked JVM immediately crashed on Unix/Linux due to new shutdown mechanism does not turn to the old shutdown mechanism
     add b617a75  [SUREFIRE-1479] Force UNIX Standard mode for ps command on HP-UX OS
     add f63d6ec  jenkinsfile branches with jdk 7 and 10
     add a530017  [SUREFIRE-1506] Sporadic NullPointerException in ConsoleOutputFileReporter#close()
     add 882cbb6  [SUREFIRE-1487] ParallelComputerBuilderTest fails on overloaded system because internal delay are shorter than blocking time of JVM
     add 02d7656  [ForkModeIT] prolonged execution time of IT. Probably virtualization issue of CPU scheduler on Win NT in ASF Jenkins CI.
     add ee02966  [SUREFIRE-1510] Jenkins CI fails due to performance of Windows break concurrency of forked JVMs
     add c546235  [SUREFIRE-1512] ProcessInfo for Windows is prone to timezone offset changes
     add b6debff  [SUREFIRE-1515] Fix for use case when an empty array is passed.
     new e66c64e  [SUREFIRE-1503] Forked JVM immediately crashed on Unix/Linux due to new shutdown mechanism does not turn to the old shutdown mechanism

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (ff4fd1f)
            \
             N -- N -- N   refs/heads/SUREFIRE-1503 (e66c64e)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 Jenkinsfile                                        |  39 +++---
 .../surefire/report/ConsoleOutputFileReporter.java | 139 ++++++++++++++------
 .../report/ConsoleOutputFileReporterTest.java      | 106 +++++++++++++---
 .../maven/surefire/util/internal/StringUtils.java  |   6 +
 .../surefire/util/internal/StringUtilsTest.java    |   9 ++
 .../apache/maven/surefire/booter/PpidChecker.java  | 140 +++++++++++----------
 .../apache/maven/surefire/booter/ProcessInfo.java  |   6 +-
 .../org/apache/maven/surefire/its/ForkModeIT.java  |   6 +-
 .../src/test/resources/fail-fast-junit/pom.xml     |   1 +
 .../fail-fast-junit/src/test/java/pkg/ATest.java   |   5 +-
 .../fail-fast-junit/src/test/java/pkg/BTest.java   |   5 +-
 .../fail-fast-junit/src/test/java/pkg/CTest.java   |   5 +-
 .../fail-fast-junit/src/test/java/pkg/DTest.java   |   5 +-
 .../fork-mode/src/test/java/forkMode/Test1.java    |  14 ++-
 .../fork-mode/src/test/java/forkMode/Test2.java    |   4 +-
 .../fork-mode/src/test/java/forkMode/Test3.java    |   4 +-
 .../junitcore/pc/ParallelComputerBuilderTest.java  |  95 ++++++++------
 .../junitcore/pc/ParallelComputerUtilTest.java     |  49 ++++----
 18 files changed, 422 insertions(+), 216 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
tibordigana@apache.org.

[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

Posted by ti...@apache.org.
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.