Clear direct byte buffers (#855)

* Add Buffers clear() methods

* BufferedFileChannelInputStream now clears its direct memory byte buffer
on close.

MemoryMappedFileInputStream now clears its direct memory byte buffer on
close.

FileChannels.contentEquals(ReadableByteChannel, ReadableByteChannel,
int) now clears its direct memory byte buffer on close.

BufferedFileChannelInputStream now clears its direct memory byte buffer
on close.

MemoryMappedFileInputStream now clears its direct memory byte buffer on
close.

Author: garydgregory

src/main/java/org/apache/commons/io/channels/FileChannels.java

@@ -24,6 +24,7 @@
 import java.nio.channels.SeekableByteChannel;
 import java.util.Objects;
 
+import org.apache.commons.io.Buffers;
 import org.apache.commons.io.IOUtils;
 
 /**
@@ -67,33 +68,38 @@ public static boolean contentEquals(final ReadableByteChannel channel1, final Re
         // Don't use ByteBuffer#compact() to avoid extra copying.
         final ByteBuffer c1Buffer = ByteBuffer.allocateDirect(bufferCapacity);
         final ByteBuffer c2Buffer = ByteBuffer.allocateDirect(bufferCapacity);
-        int c1NumRead = 0;
-        int c2NumRead = 0;
-        boolean c1Read0 = false;
-        boolean c2Read0 = false;
-        // If a channel is a non-blocking channel, it may return 0 bytes read for any given call.
-        while (true) {
-            if (!c2Read0) {
-                c1NumRead = readToLimit(channel1, c1Buffer);
-                c1Buffer.clear();
-                c1Read0 = c1NumRead == 0;
-            }
-            if (!c1Read0) {
-                c2NumRead = readToLimit(channel2, c2Buffer);
-                c2Buffer.clear();
-                c2Read0 = c2NumRead == 0;
-            }
-            if (c1NumRead == IOUtils.EOF && c2NumRead == IOUtils.EOF) {
-                return c1Buffer.equals(c2Buffer);
-            }
-            if (c1NumRead == 0 || c2NumRead == 0) {
-                // 0 may be returned from a non-blocking channel.
-                Thread.yield();
-                continue;
-            }
-            if (c1NumRead != c2NumRead || !c1Buffer.equals(c2Buffer)) {
-                return false;
+        try {
+            int c1NumRead = 0;
+            int c2NumRead = 0;
+            boolean c1Read0 = false;
+            boolean c2Read0 = false;
+            // If a channel is a non-blocking channel, it may return 0 bytes read for any given call.
+            while (true) {
+                if (!c2Read0) {
+                    c1NumRead = readToLimit(channel1, c1Buffer);
+                    c1Buffer.clear();
+                    c1Read0 = c1NumRead == 0;
+                }
+                if (!c1Read0) {
+                    c2NumRead = readToLimit(channel2, c2Buffer);
+                    c2Buffer.clear();
+                    c2Read0 = c2NumRead == 0;
+                }
+                if (c1NumRead == IOUtils.EOF && c2NumRead == IOUtils.EOF) {
+                    return c1Buffer.equals(c2Buffer);
+                }
+                if (c1NumRead == 0 || c2NumRead == 0) {
+                    // 0 may be returned from a non-blocking channel.
+                    Thread.yield();
+                    continue;
+                }
+                if (c1NumRead != c2NumRead || !c1Buffer.equals(c2Buffer)) {
+                    return false;
+                }
             }
+        } finally {
+            Buffers.clear(c1Buffer);
+            Buffers.clear(c2Buffer);
         }
     }
 

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

@@ -220,7 +220,7 @@ public synchronized int available() throws IOException {
      * @param buffer the buffer to clean.
      */
     private void clean(final ByteBuffer buffer) {
-        if (!clean && buffer.isDirect() && ByteBufferCleaner.isSupported()) {
+        if (!clean && ByteBufferCleaner.isSupported()) {
             ByteBufferCleaner.clean(buffer);
             clean = true;
         }

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

@@ -14,19 +14,21 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.apache.commons.io.input;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 
+import org.apache.commons.io.Buffers;
+
 /**
- * Cleans a direct {@link ByteBuffer}. Without manual intervention, direct ByteBuffers will be cleaned eventually upon
- * garbage collection. However, this should not be relied upon since it may not occur in a timely fashion -
- * especially since off heap ByeBuffers don't put pressure on the garbage collector.
+ * Cleans a direct {@link ByteBuffer}. Without manual intervention, direct ByteBuffers will be cleaned eventually upon garbage collection. However, this should
+ * not be relied upon since it may not occur in a timely fashion - especially since off heap ByeBuffers don't put pressure on the garbage collector.
  * <p>
- * <strong>Warning:</strong> Do not attempt to use a direct {@link ByteBuffer} that has been cleaned or bad things will happen.
- * Don't use this class unless you can ensure that the cleaned buffer will not be accessed anymore.
+ * <strong>Warning:</strong> Do not attempt to use a direct {@link ByteBuffer} that has been cleaned or bad things will happen. Don't use this class unless you
+ * can ensure that the cleaned buffer will not be accessed anymore.
  * </p>
  * <p>
  * See <a href=https://bugs.openjdk.java.net/browse/JDK-4724038>JDK-4724038</a>
@@ -35,6 +37,7 @@
 final class ByteBufferCleaner {
 
     private interface Cleaner {
+
         void clean(ByteBuffer buffer) throws ReflectiveOperationException;
     }
 
@@ -86,7 +89,10 @@ public void clean(final ByteBuffer buffer) throws ReflectiveOperationException {
      */
     static void clean(final ByteBuffer buffer) {
         try {
-            INSTANCE.clean(buffer);
+            if (buffer.isDirect()) {
+                Buffers.clearWritable(buffer);
+                INSTANCE.clean(buffer);
+            }
         } catch (final Exception e) {
             throw new IllegalStateException("Failed to clean direct buffer.", e);
         }
@@ -105,8 +111,8 @@ private static Cleaner getCleaner() {
     }
 
     /**
-     * Tests if were able to load a suitable cleaner for the current JVM. Attempting to call
-     * {@code ByteBufferCleaner#clean(ByteBuffer)} when this method returns false will result in an exception.
+     * Tests if were able to load a suitable cleaner for the current JVM. Attempting to call {@code ByteBufferCleaner#clean(ByteBuffer)} when this method
+     * returns false will result in an exception.
      *
      * @return {@code true} if cleaning is supported, {@code false} otherwise.
      */

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

@@ -172,7 +172,7 @@ public int available() throws IOException {
     }
 
     private void cleanBuffer() {
-        if (ByteBufferCleaner.isSupported() && buffer.isDirect()) {
+        if (ByteBufferCleaner.isSupported()) {
             ByteBufferCleaner.clean(buffer);
         }
     }