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/30 23:33:16 UTC
[maven-surefire] 01/01: [SUREFIRE-1506] Sporadic
NullPointerException in ConsoleOutputFileReporter#close()
This is an automated email from the ASF dual-hosted git repository.
tibordigana pushed a commit to branch 1506
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit a5300171935196e87dda7a03ec8f346628af7062
Author: Tibor17 <ti...@apache.org>
AuthorDate: Sun Mar 25 22:59:17 2018 +0200
[SUREFIRE-1506] Sporadic NullPointerException in ConsoleOutputFileReporter#close()
---
.../surefire/report/ConsoleOutputFileReporter.java | 139 +++++++++++++++------
.../report/ConsoleOutputFileReporterTest.java | 106 +++++++++++++---
2 files changed, 192 insertions(+), 53 deletions(-)
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java
index 84cbabe..bbbd9c4 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java
@@ -22,17 +22,18 @@ package org.apache.maven.plugin.surefire.report;
import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
+import java.io.FilterOutputStream;
import java.io.IOException;
-import java.io.OutputStream;
+import java.util.concurrent.atomic.AtomicStampedReference;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.maven.surefire.booter.DumpErrorSingleton;
import org.apache.maven.surefire.report.ReportEntry;
import static org.apache.maven.plugin.surefire.report.FileReporter.getReportFile;
/**
* Surefire output consumer proxy that writes test output to a {@link java.io.File} for each test suite.
- * <br>
- * This class is not threadsafe, but can be serially handed off from thread to thread.
*
* @author Kristian Rosenvold
* @author Carlos Sanchez
@@ -40,13 +41,20 @@ import static org.apache.maven.plugin.surefire.report.FileReporter.getReportFile
public class ConsoleOutputFileReporter
implements TestcycleConsoleOutputReceiver
{
- private final File reportsDirectory;
+ private static final int STREAM_BUFFER_SIZE = 16 * 1024;
+ private static final int OPEN = 0;
+ private static final int CLOSED_TO_REOPEN = 1;
+ private static final int CLOSED = 2;
+ private final File reportsDirectory;
private final String reportNameSuffix;
- private String reportEntryName;
+ private final AtomicStampedReference<FilterOutputStream> fileOutputStream =
+ new AtomicStampedReference<FilterOutputStream>( null, OPEN );
- private OutputStream fileOutputStream;
+ private final ReentrantLock lock = new ReentrantLock();
+
+ private volatile String reportEntryName;
public ConsoleOutputFileReporter( File reportsDirectory, String reportNameSuffix )
{
@@ -57,8 +65,15 @@ public class ConsoleOutputFileReporter
@Override
public void testSetStarting( ReportEntry reportEntry )
{
- close();
- reportEntryName = reportEntry.getName();
+ lock.lock();
+ try
+ {
+ closeNullReportFile( reportEntry );
+ }
+ finally
+ {
+ lock.unlock();
+ }
}
@Override
@@ -67,53 +82,105 @@ public class ConsoleOutputFileReporter
}
@Override
- @SuppressWarnings( "checkstyle:emptyblock" )
public void close()
{
- if ( fileOutputStream != null )
+ // The close() method is called in main Thread T2.
+ lock.lock();
+ try
{
- //noinspection EmptyCatchBlock
- try
- {
- fileOutputStream.flush();
- }
- catch ( IOException e )
- {
- }
- finally
- {
- try
- {
- fileOutputStream.close();
- }
- catch ( IOException ignored )
- {
- }
- }
- fileOutputStream = null;
+ closeReportFile();
+ }
+ finally
+ {
+ lock.unlock();
}
}
@Override
public void writeTestOutput( byte[] buf, int off, int len, boolean stdout )
{
+ lock.lock();
try
{
- if ( fileOutputStream == null )
+ // This method is called in single thread T1 per fork JVM (see ThreadedStreamConsumer).
+ // The close() method is called in main Thread T2.
+ int[] status = new int[1];
+ FilterOutputStream os = fileOutputStream.get( status );
+ if ( status[0] != CLOSED )
{
- if ( !reportsDirectory.exists() )
+ if ( os == null )
{
- //noinspection ResultOfMethodCallIgnored
- reportsDirectory.mkdirs();
+ if ( !reportsDirectory.exists() )
+ {
+ //noinspection ResultOfMethodCallIgnored
+ reportsDirectory.mkdirs();
+ }
+ File file = getReportFile( reportsDirectory, reportEntryName, reportNameSuffix, "-output.txt" );
+ os = new BufferedOutputStream( new FileOutputStream( file ), STREAM_BUFFER_SIZE );
+ fileOutputStream.set( os, OPEN );
}
- File file = getReportFile( reportsDirectory, reportEntryName, reportNameSuffix, "-output.txt" );
- fileOutputStream = new BufferedOutputStream( new FileOutputStream( file ), 16 * 1024 );
+ os.write( buf, off, len );
}
- fileOutputStream.write( buf, off, len );
}
catch ( IOException e )
{
+ DumpErrorSingleton.getSingleton()
+ .dumpException( e );
+
throw new RuntimeException( e );
}
+ finally
+ {
+ lock.unlock();
+ }
+ }
+
+ @SuppressWarnings( "checkstyle:emptyblock" )
+ private void closeNullReportFile( ReportEntry reportEntry )
+ {
+ try
+ {
+ // close null-output.txt report file
+ close( true );
+ }
+ catch ( IOException ignored )
+ {
+ DumpErrorSingleton.getSingleton()
+ .dumpException( ignored );
+ }
+ finally
+ {
+ // prepare <class>-output.txt report file
+ reportEntryName = reportEntry.getName();
+ }
+ }
+
+ @SuppressWarnings( "checkstyle:emptyblock" )
+ private void closeReportFile()
+ {
+ try
+ {
+ close( false );
+ }
+ catch ( IOException ignored )
+ {
+ DumpErrorSingleton.getSingleton()
+ .dumpException( ignored );
+ }
+ }
+
+ private void close( boolean closeReattempt )
+ throws IOException
+ {
+ int[] status = new int[1];
+ FilterOutputStream os = fileOutputStream.get( status );
+ if ( status[0] != CLOSED )
+ {
+ fileOutputStream.set( null, closeReattempt ? CLOSED_TO_REOPEN : CLOSED );
+ if ( os != null && status[0] == OPEN )
+ {
+ os.close();
+ }
+ }
}
}
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/surefire/report/ConsoleOutputFileReporterTest.java b/maven-surefire-common/src/test/java/org/apache/maven/surefire/report/ConsoleOutputFileReporterTest.java
index f5f3fd5..df011d9 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/surefire/report/ConsoleOutputFileReporterTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/surefire/report/ConsoleOutputFileReporterTest.java
@@ -21,6 +21,10 @@ package org.apache.maven.surefire.report;
import java.io.File;
import java.io.IOException;
+import java.util.ArrayList;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import org.apache.maven.plugin.surefire.report.ConsoleOutputFileReporter;
@@ -33,32 +37,29 @@ import static org.fest.assertions.Assertions.assertThat;
public class ConsoleOutputFileReporterTest
extends TestCase
{
-
- private ConsoleOutputFileReporter reporter;
-
- private ReportEntry reportEntry;
-
- private static final String testName = ConsoleOutputFileReporterTest.class.getName();
-
/*
* Test method for 'org.codehaus.surefire.report.ConsoleOutputFileReporter.testSetCompleted(ReportEntry report)'
*/
public void testFileNameWithoutSuffix() throws IOException
{
- File reportDir = new File( new File( System.getProperty( "user.dir" ), "target" ), "tmp" );
+ File reportDir = new File( new File( System.getProperty( "user.dir" ), "target" ), "tmp1" );
//noinspection ResultOfMethodCallIgnored
reportDir.mkdirs();
- reportEntry = new SimpleReportEntry( getClass().getName(), testName );
- reporter = new ConsoleOutputFileReporter( reportDir, null );
+ ReportEntry reportEntry = new SimpleReportEntry( getClass().getName(), getClass().getName() );
+ ConsoleOutputFileReporter reporter = new ConsoleOutputFileReporter( reportDir, null );
reporter.testSetStarting( reportEntry );
reporter.writeTestOutput( "some text".getBytes( US_ASCII ), 0, 5, true );
reporter.testSetCompleted( reportEntry );
reporter.close();
- File expectedReportFile = new File( reportDir, testName + "-output.txt" );
+ File expectedReportFile = new File( reportDir, getClass().getName() + "-output.txt" );
+
assertTrue( "Report file (" + expectedReportFile.getAbsolutePath() + ") doesn't exist",
expectedReportFile.exists() );
- assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) ).contains( "some " );
+
+ assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) )
+ .contains( "some " );
+
//noinspection ResultOfMethodCallIgnored
expectedReportFile.delete();
}
@@ -68,23 +69,94 @@ public class ConsoleOutputFileReporterTest
*/
public void testFileNameWithSuffix() throws IOException
{
- File reportDir = new File( new File( System.getProperty( "user.dir" ), "target" ), "tmp" );
+ File reportDir = new File( new File( System.getProperty( "user.dir" ), "target" ), "tmp2" );
//noinspection ResultOfMethodCallIgnored
reportDir.mkdirs();
String suffixText = "sampleSuffixText";
- reportEntry = new SimpleReportEntry( getClass().getName(), testName );
- reporter = new ConsoleOutputFileReporter( reportDir, suffixText );
+ ReportEntry reportEntry = new SimpleReportEntry( getClass().getName(), getClass().getName() );
+ ConsoleOutputFileReporter reporter = new ConsoleOutputFileReporter( reportDir, suffixText );
reporter.testSetStarting( reportEntry );
reporter.writeTestOutput( "some text".getBytes( US_ASCII ), 0, 5, true );
reporter.testSetCompleted( reportEntry );
reporter.close();
- File expectedReportFile = new File( reportDir, testName + "-" + suffixText + "-output.txt" );
+ File expectedReportFile = new File( reportDir, getClass().getName() + "-" + suffixText + "-output.txt" );
+
assertTrue( "Report file (" + expectedReportFile.getAbsolutePath() + ") doesn't exist",
expectedReportFile.exists() );
- assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) ).contains( "some " );
+
+ assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) )
+ .contains( "some " );
+
+ //noinspection ResultOfMethodCallIgnored
+ expectedReportFile.delete();
+ }
+
+ public void testNullReportFile() throws IOException
+ {
+ File reportDir = new File( new File( System.getProperty( "user.dir" ), "target" ), "tmp3" );
+ //noinspection ResultOfMethodCallIgnored
+ reportDir.mkdirs();
+ ConsoleOutputFileReporter reporter = new ConsoleOutputFileReporter( reportDir, null );
+ reporter.writeTestOutput( "some text".getBytes( US_ASCII ), 0, 5, true );
+ reporter.testSetCompleted( new SimpleReportEntry( getClass().getName(), getClass().getName() ) );
+ reporter.close();
+
+ File expectedReportFile = new File( reportDir, "null-output.txt" );
+
+ assertTrue( "Report file (" + expectedReportFile.getAbsolutePath() + ") doesn't exist",
+ expectedReportFile.exists() );
+
+ assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) )
+ .contains( "some " );
+
//noinspection ResultOfMethodCallIgnored
expectedReportFile.delete();
}
+ public void testConcurrentAccessReportFile() throws Exception
+ {
+ File reportDir = new File( new File( System.getProperty( "user.dir" ), "target" ), "tmp4" );
+ //noinspection ResultOfMethodCallIgnored
+ reportDir.mkdirs();
+ final ConsoleOutputFileReporter reporter = new ConsoleOutputFileReporter( reportDir, null );
+ reporter.testSetStarting( new SimpleReportEntry( getClass().getName(), getClass().getName() ) );
+ ExecutorService scheduler = Executors.newFixedThreadPool( 10 );
+ final ArrayList<Callable<Void>> jobs = new ArrayList<Callable<Void>>();
+ for ( int i = 0; i < 10; i++ )
+ {
+ jobs.add( new Callable<Void>() {
+ @Override
+ public Void call()
+ {
+ byte[] stream = "some text\n".getBytes( US_ASCII );
+ reporter.writeTestOutput( stream, 0, stream.length, true );
+ return null;
+ }
+ } );
+ }
+ scheduler.invokeAll( jobs );
+ scheduler.shutdown();
+ reporter.close();
+
+ File expectedReportFile = new File( reportDir, getClass().getName() + "-output.txt" );
+
+ assertTrue( "Report file (" + expectedReportFile.getAbsolutePath() + ") doesn't exist",
+ expectedReportFile.exists() );
+
+ assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) )
+ .contains( "some text" );
+
+ StringBuilder expectedText = new StringBuilder();
+ for ( int i = 0; i < 10; i++ )
+ {
+ expectedText.append( "some text\n" );
+ }
+
+ assertThat( FileUtils.fileRead( expectedReportFile, US_ASCII.name() ) )
+ .isEqualTo( expectedText.toString() );
+
+ //noinspection ResultOfMethodCallIgnored
+ expectedReportFile.delete();
+ }
}
--
To stop receiving notification emails like this one, please contact
tibordigana@apache.org.