8129776: The optimized Stream returned from Files.lines should unmap the mapped byte buffer (if created) when closed

Reviewed-by: rriggs, psandoz, alanb

Author: Brian Burkhalter

src/java.base/share/classes/java/nio/MappedByteBuffer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -32,6 +32,7 @@
 import jdk.internal.access.foreign.MemorySegmentProxy;
 import jdk.internal.access.foreign.UnmapperProxy;
 import jdk.internal.misc.ScopedMemoryAccess;
+import jdk.internal.misc.Unsafe;
 
 
 /**
@@ -132,7 +133,7 @@ public boolean isSync() {
 
                     @Override
                     public void unmap() {
-                        throw new UnsupportedOperationException();
+                        Unsafe.getUnsafe().invokeCleaner(MappedByteBuffer.this);
                     }
                 } : null;
     }

src/java.base/share/classes/java/nio/file/FileChannelLinesSpliterator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -39,8 +39,12 @@
 import java.util.HashSet;
 import java.util.Set;
 import java.util.Spliterator;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 
+import jdk.internal.access.SharedSecrets;
+import jdk.internal.access.JavaNioAccess;
+
 /**
  * A file-based lines spliterator, leveraging a shared mapped byte buffer and
  * associated file channel, covering lines of a file for character encodings
@@ -84,19 +88,31 @@ final class FileChannelLinesSpliterator implements Spliterator<String> {
     // Non-null when traversing
     private BufferedReader reader;
 
+    // Number of references to the shared mapped buffer.  Initialized to unity
+    // when the buffer is created by the root spliterator.  Incremented in the
+    // sub-spliterator constructor.  Decremented when 'buffer' transitions from
+    // non-null to null, either when traversing begins or if the spliterator is
+    // closed before traversal.  If the count is zero after decrementing, then
+    // the buffer is unmapped.
+    private final AtomicInteger bufRefCount;
+
     FileChannelLinesSpliterator(FileChannel fc, Charset cs, int index, int fence) {
         this.fc = fc;
         this.cs = cs;
         this.index = index;
         this.fence = fence;
+        this.bufRefCount = new AtomicInteger();
     }
 
-    private FileChannelLinesSpliterator(FileChannel fc, Charset cs, int index, int fence, ByteBuffer buffer) {
+    private FileChannelLinesSpliterator(FileChannel fc, Charset cs, int index,
+        int fence, ByteBuffer buffer, AtomicInteger bufRefCount) {
         this.fc = fc;
-        this.buffer = buffer;
         this.cs = cs;
         this.index = index;
         this.fence = fence;
+        this.buffer = buffer;
+        this.bufRefCount = bufRefCount;
+        this.bufRefCount.incrementAndGet();
     }
 
     @Override
@@ -167,7 +183,7 @@ public void close() throws IOException {
     private String readLine() {
         if (reader == null) {
             reader = getBufferedReader();
-            buffer = null;
+            unmap();
         }
 
         try {
@@ -178,13 +194,6 @@ private String readLine() {
     }
 
     private ByteBuffer getMappedByteBuffer() {
-        // TODO can the mapped byte buffer be explicitly unmapped?
-        // It's possible, via a shared-secret mechanism, when either
-        // 1) the spliterator starts traversing, although traversal can
-        //    happen concurrently for mulitple spliterators, so care is
-        //    needed in this case; or
-        // 2) when the stream is closed using some shared holder to pass
-        //    the mapped byte buffer when it is created.
         try {
             return fc.map(FileChannel.MapMode.READ_ONLY, 0, fence);
         } catch (IOException e) {
@@ -201,6 +210,7 @@ public Spliterator<String> trySplit() {
         ByteBuffer b;
         if ((b = buffer) == null) {
             b = buffer = getMappedByteBuffer();
+            bufRefCount.set(1);
         }
 
         final int hi = fence, lo = index;
@@ -246,7 +256,8 @@ public Spliterator<String> trySplit() {
 
         // The left spliterator will have the line-separator at the end
         return (mid > lo && mid < hi)
-               ? new FileChannelLinesSpliterator(fc, cs, lo, index = mid, b)
+               ? new FileChannelLinesSpliterator(fc, cs, lo, index = mid,
+                                                 b, bufRefCount)
                : null;
     }
 
@@ -267,4 +278,22 @@ public long getExactSizeIfKnown() {
     public int characteristics() {
         return Spliterator.ORDERED | Spliterator.NONNULL;
     }
+
+    private void unmap() {
+        if (buffer != null) {
+            ByteBuffer b = buffer;
+            buffer = null;
+            if (bufRefCount.decrementAndGet() == 0) {
+                JavaNioAccess nioAccess = SharedSecrets.getJavaNioAccess();
+                try {
+                    nioAccess.unmapper(b).unmap();
+                } catch (UnsupportedOperationException ignored) {
+                }
+            }
+        }
+    }
+
+    void close() {
+        unmap();
+    }
 }

src/java.base/share/classes/java/nio/file/Files.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -4121,9 +4121,11 @@ private static Stream<String> createFileChannelLinesStream(FileChannel fc, Chars
             // FileChannel.size() may in certain circumstances return zero
             // for a non-zero length file so disallow this case.
             if (length > 0 && length <= Integer.MAX_VALUE) {
-                Spliterator<String> s = new FileChannelLinesSpliterator(fc, cs, 0, (int) length);
-                return StreamSupport.stream(s, false)
-                        .onClose(Files.asUncheckedRunnable(fc));
+                FileChannelLinesSpliterator fcls =
+                    new FileChannelLinesSpliterator(fc, cs, 0, (int) length);
+                return StreamSupport.stream(fcls, false)
+                        .onClose(Files.asUncheckedRunnable(fc))
+                        .onClose(() -> fcls.close());
             }
         } catch (Error|RuntimeException|IOException e) {
             try {