Clean BufferedFileChannelInputStream's direct ByteBuffer once(#854)

The buffer is now cleaned only on the first close() call.

Author: garydgregory

src/main/java/org/apache/commons/io/input/BufferedFileChannelInputStream.java

@@ -137,6 +137,8 @@ public static Builder builder() {
 
     private final ByteBuffer byteBuffer;
 
+    private boolean clean;
+
     private final FileChannel fileChannel;
 
     @SuppressWarnings("resource")
@@ -209,26 +211,18 @@ public synchronized int available() throws IOException {
      * disposed buffer. However, neither the bytes allocated to direct buffers nor file descriptors opened for memory-mapped buffers put pressure on the garbage
      * collector. Waiting for garbage collection may lead to the depletion of off-heap memory or huge numbers of open files. There's unfortunately no standard
      * API to manually dispose of these kinds of buffers.
-     *
-     * @param buffer the buffer to clean.
-     */
-    private void clean(final ByteBuffer buffer) {
-        if (buffer.isDirect()) {
-            cleanDirectBuffer(buffer);
-        }
-    }
-
-    /**
+     * <p>
      * In Java 8, the type of {@code sun.nio.ch.DirectBuffer.cleaner()} was {@code sun.misc.Cleaner}, and it was possible to access the method
      * {@code sun.misc.Cleaner.clean()} to invoke it. The type changed to {@code jdk.internal.ref.Cleaner} in later JDKs, and the {@code clean()} method is not
      * accessible even with reflection. However {@code sun.misc.Unsafe} added an {@code invokeCleaner()} method in JDK 9+ and this is still accessible with
      * reflection.
-     *
-     * @param buffer the buffer to clean. must be a DirectBuffer.
+     * </p>
+     * @param buffer the buffer to clean.
      */
-    private void cleanDirectBuffer(final ByteBuffer buffer) {
-        if (ByteBufferCleaner.isSupported()) {
+    private void clean(final ByteBuffer buffer) {
+        if (!clean && buffer.isDirect() && ByteBufferCleaner.isSupported()) {
             ByteBufferCleaner.clean(buffer);
+            clean = true;
         }
     }
 
@@ -241,6 +235,10 @@ public synchronized void close() throws IOException {
         }
     }
 
+    boolean isClean() {
+        return clean;
+    }
+
     @Override
     public synchronized int read() throws IOException {
         if (!refill()) {
@@ -301,6 +299,7 @@ public synchronized long skip(final long n) throws IOException {
         return skippedFromBuffer + skipFromFileChannel(toSkipFromFileChannel);
     }
 
+
     private long skipFromFileChannel(final long n) throws IOException {
         final long currentFilePosition = fileChannel.position();
         final long size = fileChannel.size();

src/test/java/org/apache/commons/io/input/BufferedFileChannelInputStreamTest.java

@@ -14,8 +14,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.apache.commons.io.input;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -65,12 +67,27 @@ void testBuilderGet() {
         assertThrows(IllegalStateException.class, () -> BufferedFileChannelInputStream.builder().get());
     }
 
+    /**
+     * Tests that the {@code clean(ByteBuffer)} method only performs buffer cleaning once, even when {@link BufferedFileChannelInputStream#close()} is invoked
+     * multiple times. The private {@code clean} boolean field is inspected via reflection: it must be {@code false} before any close, {@code true} after the
+     * first close, and remain {@code true} after a second close, confirming that {@code ByteBufferCleaner.clean()} is never called more than once.
+     */
+    @Test
+    void testCleanCalledOnlyOnce() throws Exception {
+        try (BufferedFileChannelInputStream stream = BufferedFileChannelInputStream.builder().setPath(InputPath).get()) {
+            assertFalse(stream.isClean());
+            stream.close();
+            assertTrue(stream.isClean());
+            stream.close();
+            assertTrue(stream.isClean());
+        }
+    }
+
     @Test
     void testReadAfterClose() throws Exception {
         for (final InputStream inputStream : inputStreams) {
             inputStream.close();
             assertThrows(IOException.class, inputStream::read);
         }
     }
-
 }