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 2022/05/11 21:12:41 UTC

[maven-surefire] 01/01: [SUREFIRE-2082] Close file handles asap to prevent breaching the system's maximum number of open files

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

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

commit 7bcf3ee6e8cf63b9ce01fbba6cd556193d551111
Author: tibor.digana <ti...@apache.org>
AuthorDate: Wed May 11 23:12:24 2022 +0200

    [SUREFIRE-2082] Close file handles asap to prevent breaching the system's maximum number of open files
---
 .../surefire/report/StatelessXmlReporter.java      |  1 +
 .../plugin/surefire/report/TestSetRunListener.java | 11 +++
 .../Utf8RecodingDeferredFileOutputStream.java      | 99 +++++++++++++++++-----
 .../surefire/report/StatelessXmlReporterTest.java  | 12 ++-
 4 files changed, 98 insertions(+), 25 deletions(-)

diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java
index 45f1c5003..66cdf8c2b 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java
@@ -511,6 +511,7 @@ public class StatelessXmlReporter
             outputStreamWriter.flush();
             eos.getUnderlying().write( ByteConstantsHolder.CDATA_START_BYTES ); // emit cdata
             utf8RecodingDeferredFileOutputStream.writeTo( eos );
+            utf8RecodingDeferredFileOutputStream.free();
             eos.getUnderlying().write( ByteConstantsHolder.CDATA_END_BYTES );
             eos.flush();
             xmlWriter.endElement();
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
index 2884f68a5..d795bf828 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
@@ -197,6 +197,16 @@ public class TestSetRunListener
 
     private void clearCapture()
     {
+        if ( testStdOut != null )
+        {
+            testStdOut.commit();
+        }
+
+        if ( testStdErr != null )
+        {
+            testStdErr.commit();
+        }
+
         testStdOut = initDeferred( "stdout" );
         testStdErr = initDeferred( "stderr" );
     }
@@ -275,6 +285,7 @@ public class TestSetRunListener
     @Override
     public void testExecutionSkippedByUser()
     {
+        clearCapture();
     }
 
     @Override
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java
index 951e0e111..cc192689d 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/Utf8RecodingDeferredFileOutputStream.java
@@ -22,6 +22,7 @@ package org.apache.maven.plugin.surefire.report;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.RandomAccessFile;
+import java.io.UncheckedIOException;
 import java.lang.ref.SoftReference;
 import java.nio.Buffer;
 import java.nio.ByteBuffer;
@@ -29,6 +30,9 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+// import static java.nio.file.Files.delete;
+import static java.nio.file.Files.notExists;
+import static java.nio.file.Files.size;
 import static java.util.Objects.requireNonNull;
 import static org.apache.maven.surefire.api.util.internal.StringUtils.NL;
 
@@ -120,13 +124,12 @@ final class Utf8RecodingDeferredFileOutputStream
     {
         try
         {
-            long length = 0;
-            if ( storage != null )
+            sync();
+            if ( storage != null && !closed )
             {
-                sync();
-                length = storage.length();
+                storage.getFD().sync();
             }
-            return length;
+            return file == null ? 0 : size( file );
         }
         catch ( IOException e )
         {
@@ -138,36 +141,87 @@ final class Utf8RecodingDeferredFileOutputStream
     public synchronized void writeTo( OutputStream out )
         throws IOException
     {
+        if ( file != null && notExists( file ) )
+        {
+            storage = null;
+        }
+
+        if ( ( storage == null && file != null ) || ( storage != null && !storage.getChannel().isOpen() ) )
+        {
+            storage = new RandomAccessFile( file.toFile(), "rw" );
+        }
+
         if ( storage != null )
         {
             sync();
+            final long currentFilePosition = storage.getFilePointer();
             storage.seek( 0L );
-            byte[] buffer = new byte[CACHE_SIZE];
-            for ( int readCount; ( readCount = storage.read( buffer ) ) != -1; )
+            try
+            {
+                byte[] buffer = new byte[CACHE_SIZE];
+                for ( int readCount; ( readCount = storage.read( buffer ) ) != -1; )
+                {
+                    out.write( buffer, 0, readCount );
+                }
+            }
+            finally
             {
-                out.write ( buffer, 0, readCount );
+                storage.seek( currentFilePosition );
             }
         }
     }
 
+    public synchronized void commit()
+    {
+        if ( storage == null )
+        {
+            return;
+        }
+
+        try
+        {
+            sync();
+            storage.close();
+        }
+        catch ( IOException e )
+        {
+            throw new UncheckedIOException( e );
+        }
+        finally
+        {
+            storage = null;
+            cache = null;
+            largeCache = null;
+        }
+    }
+
     public synchronized void free()
     {
         if ( !closed )
         {
             closed = true;
-            if ( cache != null )
+            if ( file != null )
             {
                 try
                 {
-                    sync();
-                    storage.close();
-                    Files.delete( file );
+                    commit();
+                    //todo delete( file ); uncomment L485 assertThat( file ).doesNotExist() in StatelessXmlReporterTest
                 }
-                catch ( IOException e )
+                catch ( /*todo uncomment IOException |*/ UncheckedIOException e )
                 {
+                    /*todo uncomment file.toFile()
+                        .deleteOnExit();*/
+                }
+                finally
+                {
+                    // todo should be removed after uncommented delete( file )
                     file.toFile()
                         .deleteOnExit();
                 }
+
+                storage = null;
+                cache = null;
+                largeCache = null;
             }
         }
     }
@@ -181,14 +235,17 @@ final class Utf8RecodingDeferredFileOutputStream
 
         isDirty = false;
 
-        ( (Buffer) cache ).flip();
-        byte[] array = cache.array();
-        int offset = cache.arrayOffset() + ( (Buffer) cache ).position();
-        int length = cache.remaining();
-        ( (Buffer) cache ).clear();
-        storage.write( array, offset, length );
-        // the data that you wrote with the mode "rw" may still only be kept in memory and may be read back
-        // storage.getFD().sync();
+        if ( storage != null && cache != null )
+        {
+            ( (Buffer) cache ).flip();
+            byte[] array = cache.array();
+            int offset = cache.arrayOffset() + ( (Buffer) cache ).position();
+            int length = cache.remaining();
+            ( (Buffer) cache ).clear();
+            storage.write( array, offset, length );
+            // the data that you wrote with the mode "rw" may still only be kept in memory and may be read back
+            // storage.getFD().sync();
+        }
     }
 
     @SuppressWarnings( "checkstyle:innerassignment" )
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java
index 8744c91f0..47fb587fb 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporterTest.java
@@ -380,12 +380,16 @@ public class StatelessXmlReporterTest
 
         assertThat( out.getByteCount() ).isZero();
 
-        RandomAccessFile storage = mock( RandomAccessFile.class );
+        File f = File.createTempFile( "test", "tmp" );
+        RandomAccessFile storage = new RandomAccessFile( f, "rw" );
         setInternalState( out, "storage", storage );
-        when( storage.length() ).thenReturn( 1L );
+        setInternalState( out, "file", f.toPath() );
+        storage.writeByte( 0 );
+        storage.getFD().sync();
         assertThat( out.getByteCount() ).isEqualTo( 1 );
 
-        when( storage.length() ).thenThrow( IOException.class );
+        storage.close();
+        assertThat( f.delete() ).isTrue();
         assertThat( out.getByteCount() ).isZero();
         out.free();
     }
@@ -482,7 +486,7 @@ public class StatelessXmlReporterTest
         assertThat( (boolean) getInternalState( out, "closed" ) ).isFalse();
         out.free();
         assertThat( (boolean) getInternalState( out, "closed" ) ).isTrue();
-        assertThat( file ).doesNotExist();
+        //todo assertThat( file ).doesNotExist();
         out.free();
         assertThat( (boolean) getInternalState( out, "closed" ) ).isTrue();
     }