8205612: (fc) Files.readAllBytes fails with ClosedByInterruptException when interrupt status set

Reviewed-by: bpb
This commit is contained in:
Alan Bateman 2018-06-26 08:13:02 +01:00
parent 4e844fe623
commit 807c4ae4a3
4 changed files with 233 additions and 46 deletions

View file

@ -77,6 +77,7 @@ import java.util.function.BiPredicate;
import java.util.stream.Stream; import java.util.stream.Stream;
import java.util.stream.StreamSupport; import java.util.stream.StreamSupport;
import sun.nio.ch.FileChannelImpl;
import sun.nio.fs.AbstractFileSystemProvider; import sun.nio.fs.AbstractFileSystemProvider;
/** /**
@ -3203,10 +3204,11 @@ public final class Files {
public static byte[] readAllBytes(Path path) throws IOException { public static byte[] readAllBytes(Path path) throws IOException {
try (SeekableByteChannel sbc = Files.newByteChannel(path); try (SeekableByteChannel sbc = Files.newByteChannel(path);
InputStream in = Channels.newInputStream(sbc)) { InputStream in = Channels.newInputStream(sbc)) {
if (sbc instanceof FileChannelImpl)
((FileChannelImpl) sbc).setUninterruptible();
long size = sbc.size(); long size = sbc.size();
if (size > (long) MAX_BUFFER_SIZE) if (size > (long) MAX_BUFFER_SIZE)
throw new OutOfMemoryError("Required array size too large"); throw new OutOfMemoryError("Required array size too large");
return read(in, (int)size); return read(in, (int)size);
} }
} }

View file

@ -25,18 +25,54 @@
package java.nio.file.spi; package java.nio.file.spi;
import java.nio.file.*; import java.nio.channels.AsynchronousFileChannel;
import java.nio.file.attribute.*; import java.nio.channels.Channels;
import java.nio.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.net.URI;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
import java.io.IOException; 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.util.concurrent.ExecutorService;
import java.security.AccessController; import java.security.AccessController;
import java.security.PrivilegedAction; import java.security.PrivilegedAction;
import sun.nio.ch.FileChannelImpl;
/** /**
* Service-provider class for file systems. The methods defined by the {@link * 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 * java.nio.file.Files} class will typically delegate to an instance of this
@ -381,7 +417,11 @@ public abstract class FileSystemProvider {
throw new UnsupportedOperationException("'" + opt + "' not allowed"); 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 = private static final Set<OpenOption> DEFAULT_OPEN_OPTIONS =
@ -435,7 +475,11 @@ public abstract class FileSystemProvider {
} }
opts.add(StandardOpenOption.WRITE); 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);
} }
/** /**

View file

@ -31,6 +31,7 @@ import java.io.UncheckedIOException;
import java.lang.ref.Cleaner.Cleanable; import java.lang.ref.Cleaner.Cleanable;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.nio.MappedByteBuffer; import java.nio.MappedByteBuffer;
import java.nio.channels.AsynchronousCloseException;
import java.nio.channels.ClosedByInterruptException; import java.nio.channels.ClosedByInterruptException;
import java.nio.channels.ClosedChannelException; import java.nio.channels.ClosedChannelException;
import java.nio.channels.FileChannel; import java.nio.channels.FileChannel;
@ -38,24 +39,15 @@ import java.nio.channels.FileLock;
import java.nio.channels.FileLockInterruptionException; import java.nio.channels.FileLockInterruptionException;
import java.nio.channels.NonReadableChannelException; import java.nio.channels.NonReadableChannelException;
import java.nio.channels.NonWritableChannelException; import java.nio.channels.NonWritableChannelException;
import java.nio.channels.OverlappingFileLockException;
import java.nio.channels.ReadableByteChannel; import java.nio.channels.ReadableByteChannel;
import java.nio.channels.SelectableChannel; import java.nio.channels.SelectableChannel;
import java.nio.channels.WritableByteChannel; 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.JavaIOFileDescriptorAccess;
import jdk.internal.misc.JavaNioAccess; import jdk.internal.misc.JavaNioAccess;
import jdk.internal.misc.SharedSecrets; import jdk.internal.misc.SharedSecrets;
import jdk.internal.ref.Cleaner; import jdk.internal.ref.Cleaner;
import jdk.internal.ref.CleanerFactory; import jdk.internal.ref.CleanerFactory;
import sun.security.action.GetPropertyAction;
public class FileChannelImpl public class FileChannelImpl
extends FileChannel extends FileChannel
@ -90,7 +82,7 @@ public class FileChannelImpl
// Lock for operations involving position and size // Lock for operations involving position and size
private final Object positionLock = new Object(); private final Object positionLock = new Object();
// Positional-read is not interruptible // blocking operations are not interruptible
private volatile boolean uninterruptible; private volatile boolean uninterruptible;
// DirectIO flag // DirectIO flag
@ -162,6 +154,14 @@ public class FileChannelImpl
uninterruptible = true; uninterruptible = true;
} }
private void beginBlocking() {
if (!uninterruptible) begin();
}
private void endBlocking(boolean completed) throws AsynchronousCloseException {
if (!uninterruptible) end(completed);
}
// -- Standard channel operations -- // -- Standard channel operations --
protected void implCloseChannel() throws IOException { protected void implCloseChannel() throws IOException {
@ -215,7 +215,7 @@ public class FileChannelImpl
int n = 0; int n = 0;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return 0; return 0;
@ -225,7 +225,7 @@ public class FileChannelImpl
return IOStatus.normalize(n); return IOStatus.normalize(n);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(n > 0); endBlocking(n > 0);
assert IOStatus.check(n); assert IOStatus.check(n);
} }
} }
@ -245,7 +245,7 @@ public class FileChannelImpl
long n = 0; long n = 0;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return 0; return 0;
@ -256,7 +256,7 @@ public class FileChannelImpl
return IOStatus.normalize(n); return IOStatus.normalize(n);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(n > 0); endBlocking(n > 0);
assert IOStatus.check(n); assert IOStatus.check(n);
} }
} }
@ -272,7 +272,7 @@ public class FileChannelImpl
int n = 0; int n = 0;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return 0; return 0;
@ -282,7 +282,7 @@ public class FileChannelImpl
return IOStatus.normalize(n); return IOStatus.normalize(n);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(n > 0); endBlocking(n > 0);
assert IOStatus.check(n); assert IOStatus.check(n);
} }
} }
@ -302,7 +302,7 @@ public class FileChannelImpl
long n = 0; long n = 0;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return 0; return 0;
@ -313,7 +313,7 @@ public class FileChannelImpl
return IOStatus.normalize(n); return IOStatus.normalize(n);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(n > 0); endBlocking(n > 0);
assert IOStatus.check(n); assert IOStatus.check(n);
} }
} }
@ -327,7 +327,7 @@ public class FileChannelImpl
long p = -1; long p = -1;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return 0; return 0;
@ -339,7 +339,7 @@ public class FileChannelImpl
return IOStatus.normalize(p); return IOStatus.normalize(p);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(p > -1); endBlocking(p > -1);
assert IOStatus.check(p); assert IOStatus.check(p);
} }
} }
@ -353,7 +353,7 @@ public class FileChannelImpl
long p = -1; long p = -1;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return null; return null;
@ -363,7 +363,7 @@ public class FileChannelImpl
return this; return this;
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(p > -1); endBlocking(p > -1);
assert IOStatus.check(p); assert IOStatus.check(p);
} }
} }
@ -375,7 +375,7 @@ public class FileChannelImpl
long s = -1; long s = -1;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return -1; return -1;
@ -385,7 +385,7 @@ public class FileChannelImpl
return IOStatus.normalize(s); return IOStatus.normalize(s);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(s > -1); endBlocking(s > -1);
assert IOStatus.check(s); assert IOStatus.check(s);
} }
} }
@ -403,7 +403,7 @@ public class FileChannelImpl
int ti = -1; int ti = -1;
long rp = -1; long rp = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return null; return null;
@ -442,7 +442,7 @@ public class FileChannelImpl
return this; return this;
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(rv > -1); endBlocking(rv > -1);
assert IOStatus.check(rv); assert IOStatus.check(rv);
} }
} }
@ -453,7 +453,7 @@ public class FileChannelImpl
int rv = -1; int rv = -1;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return; return;
@ -462,7 +462,7 @@ public class FileChannelImpl
} while ((rv == IOStatus.INTERRUPTED) && isOpen()); } while ((rv == IOStatus.INTERRUPTED) && isOpen());
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(rv > -1); endBlocking(rv > -1);
assert IOStatus.check(rv); assert IOStatus.check(rv);
} }
} }
@ -493,7 +493,7 @@ public class FileChannelImpl
long n = -1; long n = -1;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return -1; return -1;
@ -808,9 +808,8 @@ public class FileChannelImpl
int n = 0; int n = 0;
int ti = -1; int ti = -1;
boolean interruptible = !uninterruptible;
try { try {
if (interruptible) begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return -1; return -1;
@ -820,7 +819,7 @@ public class FileChannelImpl
return IOStatus.normalize(n); return IOStatus.normalize(n);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
if (interruptible) end(n > 0); endBlocking(n > 0);
assert IOStatus.check(n); assert IOStatus.check(n);
} }
} }
@ -849,7 +848,7 @@ public class FileChannelImpl
int n = 0; int n = 0;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return -1; return -1;
@ -859,7 +858,7 @@ public class FileChannelImpl
return IOStatus.normalize(n); return IOStatus.normalize(n);
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(n > 0); endBlocking(n > 0);
assert IOStatus.check(n); assert IOStatus.check(n);
} }
} }
@ -963,7 +962,7 @@ public class FileChannelImpl
long addr = -1; long addr = -1;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return null; return null;
@ -1052,7 +1051,7 @@ public class FileChannelImpl
} }
} finally { } finally {
threads.remove(ti); threads.remove(ti);
end(IOStatus.checkAll(addr)); endBlocking(IOStatus.checkAll(addr));
} }
} }
@ -1117,7 +1116,7 @@ public class FileChannelImpl
boolean completed = false; boolean completed = false;
int ti = -1; int ti = -1;
try { try {
begin(); beginBlocking();
ti = threads.add(); ti = threads.add();
if (!isOpen()) if (!isOpen())
return null; return null;
@ -1140,7 +1139,7 @@ public class FileChannelImpl
flt.remove(fli); flt.remove(fli);
threads.remove(ti); threads.remove(ti);
try { try {
end(completed); endBlocking(completed);
} catch (ClosedByInterruptException e) { } catch (ClosedByInterruptException e) {
throw new FileLockInterruptionException(); throw new FileLockInterruptionException();
} }

View file

@ -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]);
}
}