8205612: (fc) Files.readAllBytes fails with ClosedByInterruptException when interrupt status set
authoralanb
Tue, 26 Jun 2018 08:13:02 +0100
changeset 50784 57f5cba78093
parent 50783 0ed32e0d98e1
child 50785 d1b24f2ceca5
8205612: (fc) Files.readAllBytes fails with ClosedByInterruptException when interrupt status set Reviewed-by: bpb
src/java.base/share/classes/java/nio/file/Files.java
src/java.base/share/classes/java/nio/file/spi/FileSystemProvider.java
src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java
test/jdk/java/nio/file/Files/CallWithInterruptSet.java
--- a/src/java.base/share/classes/java/nio/file/Files.java	Mon Jun 25 22:28:04 2018 -0700
+++ b/src/java.base/share/classes/java/nio/file/Files.java	Tue Jun 26 08:13:02 2018 +0100
@@ -77,6 +77,7 @@
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
+import sun.nio.ch.FileChannelImpl;
 import sun.nio.fs.AbstractFileSystemProvider;
 
 /**
@@ -3203,10 +3204,11 @@
     public static byte[] readAllBytes(Path path) throws IOException {
         try (SeekableByteChannel sbc = Files.newByteChannel(path);
              InputStream in = Channels.newInputStream(sbc)) {
+            if (sbc instanceof FileChannelImpl)
+                ((FileChannelImpl) sbc).setUninterruptible();
             long size = sbc.size();
-            if (size > (long)MAX_BUFFER_SIZE)
+            if (size > (long) MAX_BUFFER_SIZE)
                 throw new OutOfMemoryError("Required array size too large");
-
             return read(in, (int)size);
         }
     }
--- a/src/java.base/share/classes/java/nio/file/spi/FileSystemProvider.java	Mon Jun 25 22:28:04 2018 -0700
+++ b/src/java.base/share/classes/java/nio/file/spi/FileSystemProvider.java	Tue Jun 26 08:13:02 2018 +0100
@@ -25,18 +25,54 @@
 
 package java.nio.file.spi;
 
-import java.nio.file.*;
-import java.nio.file.attribute.*;
-import java.nio.channels.*;
+import java.nio.channels.AsynchronousFileChannel;
+import java.nio.channels.Channels;
+import java.nio.channels.FileChannel;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
+import java.nio.channels.WritableByteChannel;
+import java.nio.file.AccessDeniedException;
+import java.nio.file.AccessMode;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.CopyOption;
+import java.nio.file.DirectoryNotEmptyException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.FileStore;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystemAlreadyExistsException;
+import java.nio.file.FileSystemNotFoundException;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.LinkPermission;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.NotDirectoryException;
+import java.nio.file.NotLinkException;
+import java.nio.file.OpenOption;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.net.URI;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.IOException;
-import java.util.*;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.FileAttributeView;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
+import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 
+import sun.nio.ch.FileChannelImpl;
+
 /**
  * Service-provider class for file systems. The methods defined by the {@link
  * java.nio.file.Files} class will typically delegate to an instance of this
@@ -381,7 +417,11 @@
                     throw new UnsupportedOperationException("'" + opt + "' not allowed");
             }
         }
-        return Channels.newInputStream(Files.newByteChannel(path, options));
+        ReadableByteChannel rbc = Files.newByteChannel(path, options);
+        if (rbc instanceof FileChannelImpl) {
+            ((FileChannelImpl) rbc).setUninterruptible();
+        }
+        return Channels.newInputStream(rbc);
     }
 
     private static final Set<OpenOption> DEFAULT_OPEN_OPTIONS =
@@ -435,7 +475,11 @@
             }
             opts.add(StandardOpenOption.WRITE);
         }
-        return Channels.newOutputStream(newByteChannel(path, opts));
+        WritableByteChannel wbc = newByteChannel(path, opts);
+        if (wbc instanceof FileChannelImpl) {
+            ((FileChannelImpl) wbc).setUninterruptible();
+        }
+        return Channels.newOutputStream(wbc);
     }
 
     /**
--- a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java	Mon Jun 25 22:28:04 2018 -0700
+++ b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java	Tue Jun 26 08:13:02 2018 +0100
@@ -31,6 +31,7 @@
 import java.lang.ref.Cleaner.Cleanable;
 import java.nio.ByteBuffer;
 import java.nio.MappedByteBuffer;
+import java.nio.channels.AsynchronousCloseException;
 import java.nio.channels.ClosedByInterruptException;
 import java.nio.channels.ClosedChannelException;
 import java.nio.channels.FileChannel;
@@ -38,24 +39,15 @@
 import java.nio.channels.FileLockInterruptionException;
 import java.nio.channels.NonReadableChannelException;
 import java.nio.channels.NonWritableChannelException;
-import java.nio.channels.OverlappingFileLockException;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.channels.SelectableChannel;
 import java.nio.channels.WritableByteChannel;
-import java.nio.file.Files;
-import java.nio.file.FileStore;
-import java.nio.file.FileSystemException;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.List;
 
 import jdk.internal.misc.JavaIOFileDescriptorAccess;
 import jdk.internal.misc.JavaNioAccess;
 import jdk.internal.misc.SharedSecrets;
 import jdk.internal.ref.Cleaner;
 import jdk.internal.ref.CleanerFactory;
-import sun.security.action.GetPropertyAction;
 
 public class FileChannelImpl
     extends FileChannel
@@ -90,7 +82,7 @@
     // Lock for operations involving position and size
     private final Object positionLock = new Object();
 
-    // Positional-read is not interruptible
+    // blocking operations are not interruptible
     private volatile boolean uninterruptible;
 
     // DirectIO flag
@@ -162,6 +154,14 @@
         uninterruptible = true;
     }
 
+    private void beginBlocking() {
+        if (!uninterruptible) begin();
+    }
+
+    private void endBlocking(boolean completed) throws AsynchronousCloseException {
+        if (!uninterruptible) end(completed);
+    }
+
     // -- Standard channel operations --
 
     protected void implCloseChannel() throws IOException {
@@ -215,7 +215,7 @@
             int n = 0;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return 0;
@@ -225,7 +225,7 @@
                 return IOStatus.normalize(n);
             } finally {
                 threads.remove(ti);
-                end(n > 0);
+                endBlocking(n > 0);
                 assert IOStatus.check(n);
             }
         }
@@ -245,7 +245,7 @@
             long n = 0;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return 0;
@@ -256,7 +256,7 @@
                 return IOStatus.normalize(n);
             } finally {
                 threads.remove(ti);
-                end(n > 0);
+                endBlocking(n > 0);
                 assert IOStatus.check(n);
             }
         }
@@ -272,7 +272,7 @@
             int n = 0;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return 0;
@@ -282,7 +282,7 @@
                 return IOStatus.normalize(n);
             } finally {
                 threads.remove(ti);
-                end(n > 0);
+                endBlocking(n > 0);
                 assert IOStatus.check(n);
             }
         }
@@ -302,7 +302,7 @@
             long n = 0;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return 0;
@@ -313,7 +313,7 @@
                 return IOStatus.normalize(n);
             } finally {
                 threads.remove(ti);
-                end(n > 0);
+                endBlocking(n > 0);
                 assert IOStatus.check(n);
             }
         }
@@ -327,7 +327,7 @@
             long p = -1;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return 0;
@@ -339,7 +339,7 @@
                 return IOStatus.normalize(p);
             } finally {
                 threads.remove(ti);
-                end(p > -1);
+                endBlocking(p > -1);
                 assert IOStatus.check(p);
             }
         }
@@ -353,7 +353,7 @@
             long p = -1;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return null;
@@ -363,7 +363,7 @@
                 return this;
             } finally {
                 threads.remove(ti);
-                end(p > -1);
+                endBlocking(p > -1);
                 assert IOStatus.check(p);
             }
         }
@@ -375,7 +375,7 @@
             long s = -1;
             int ti = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return -1;
@@ -385,7 +385,7 @@
                 return IOStatus.normalize(s);
             } finally {
                 threads.remove(ti);
-                end(s > -1);
+                endBlocking(s > -1);
                 assert IOStatus.check(s);
             }
         }
@@ -403,7 +403,7 @@
             int ti = -1;
             long rp = -1;
             try {
-                begin();
+                beginBlocking();
                 ti = threads.add();
                 if (!isOpen())
                     return null;
@@ -442,7 +442,7 @@
                 return this;
             } finally {
                 threads.remove(ti);
-                end(rv > -1);
+                endBlocking(rv > -1);
                 assert IOStatus.check(rv);
             }
         }
@@ -453,7 +453,7 @@
         int rv = -1;
         int ti = -1;
         try {
-            begin();
+            beginBlocking();
             ti = threads.add();
             if (!isOpen())
                 return;
@@ -462,7 +462,7 @@
             } while ((rv == IOStatus.INTERRUPTED) && isOpen());
         } finally {
             threads.remove(ti);
-            end(rv > -1);
+            endBlocking(rv > -1);
             assert IOStatus.check(rv);
         }
     }
@@ -493,7 +493,7 @@
         long n = -1;
         int ti = -1;
         try {
-            begin();
+            beginBlocking();
             ti = threads.add();
             if (!isOpen())
                 return -1;
@@ -808,9 +808,8 @@
         int n = 0;
         int ti = -1;
 
-        boolean interruptible = !uninterruptible;
         try {
-            if (interruptible) begin();
+            beginBlocking();
             ti = threads.add();
             if (!isOpen())
                 return -1;
@@ -820,7 +819,7 @@
             return IOStatus.normalize(n);
         } finally {
             threads.remove(ti);
-            if (interruptible) end(n > 0);
+            endBlocking(n > 0);
             assert IOStatus.check(n);
         }
     }
@@ -849,7 +848,7 @@
         int n = 0;
         int ti = -1;
         try {
-            begin();
+            beginBlocking();
             ti = threads.add();
             if (!isOpen())
                 return -1;
@@ -859,7 +858,7 @@
             return IOStatus.normalize(n);
         } finally {
             threads.remove(ti);
-            end(n > 0);
+            endBlocking(n > 0);
             assert IOStatus.check(n);
         }
     }
@@ -963,7 +962,7 @@
         long addr = -1;
         int ti = -1;
         try {
-            begin();
+            beginBlocking();
             ti = threads.add();
             if (!isOpen())
                 return null;
@@ -1052,7 +1051,7 @@
             }
         } finally {
             threads.remove(ti);
-            end(IOStatus.checkAll(addr));
+            endBlocking(IOStatus.checkAll(addr));
         }
     }
 
@@ -1117,7 +1116,7 @@
         boolean completed = false;
         int ti = -1;
         try {
-            begin();
+            beginBlocking();
             ti = threads.add();
             if (!isOpen())
                 return null;
@@ -1140,7 +1139,7 @@
                 flt.remove(fli);
             threads.remove(ti);
             try {
-                end(completed);
+                endBlocking(completed);
             } catch (ClosedByInterruptException e) {
                 throw new FileLockInterruptionException();
             }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/nio/file/Files/CallWithInterruptSet.java	Tue Jun 26 08:13:02 2018 +0100
@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2018, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8205612
+ * @run testng CallWithInterruptSet
+ * @summary Test invoking Files methods with the interrupt status set
+ */
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.IOException;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+public class CallWithInterruptSet {
+
+    @Test
+    public void testReadAllBytes() throws Exception {
+        Path file = mkfile(100);
+        Thread.currentThread().interrupt();
+        try {
+            byte[] bytes = Files.readAllBytes(file);
+            assertTrue(bytes.length == 100);
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+    }
+
+    @Test
+    public void testInputStream() throws IOException {
+        Path file = mkfile(100);
+        Thread.currentThread().interrupt();
+        try (InputStream in = Files.newInputStream(file)) {
+            int n = in.read(new byte[10]);
+            assertTrue(n > 0);
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+    }
+
+    @Test
+    public void testOutputStream() throws Exception {
+        Path file = mkfile();
+        try (OutputStream out = Files.newOutputStream(file)) {
+            Thread.currentThread().interrupt();
+            out.write(new byte[10]);
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+        assertTrue(Files.size(file) == 10);
+    }
+
+    @Test
+    public void testReadString() throws Exception {
+        Path file = mkfile();
+        Files.writeString(file, "hello");
+        Thread.currentThread().interrupt();
+        try {
+            String msg = Files.readString(file);
+            assertEquals(msg, "hello");
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+    }
+
+    @Test
+    public void testWriteString() throws Exception {
+        Path file = mkfile();
+        Thread.currentThread().interrupt();
+        try {
+            Files.writeString(file, "hello");
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+        String msg = Files.readString(file);
+        assertEquals(msg, "hello");
+    }
+
+    @Test
+    public void testBufferedReader() throws Exception {
+        Path file = mkfile();
+        Files.writeString(file, "hello");
+        Thread.currentThread().interrupt();
+        try (BufferedReader reader = Files.newBufferedReader(file)) {
+            String msg = reader.readLine();
+            assertEquals(msg, "hello");
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+    }
+
+    @Test
+    public void testBufferedWriter() throws Exception {
+        Path file = mkfile();
+        Thread.currentThread().interrupt();
+        try (BufferedWriter writer = Files.newBufferedWriter(file)) {
+            writer.write("hello");
+        } finally {
+            assertTrue(Thread.interrupted());  // clear interrupt
+        }
+        String msg = Files.readString(file);
+        assertEquals(msg, "hello");
+    }
+
+    private Path mkfile() throws IOException {
+        return Files.createTempFile(Path.of("."), "tmp", "tmp");
+    }
+
+    private Path mkfile(int size) throws IOException {
+        return Files.write(mkfile(), new byte[size]);
+    }
+
+}