8025619: (fc) FileInputStream.getChannel on closed stream returns FileChannel that doesn't know that stream is closed
Summary: If the stream is closed ensure getChannel() returns a closed channel. Also, FileKey.create() should throw an IOException directly instead of wrapping it in an Error.
Reviewed-by: alanb
--- a/jdk/src/java.base/share/classes/java/io/FileInputStream.java Mon Dec 15 19:21:59 2014 +0100
+++ b/jdk/src/java.base/share/classes/java/io/FileInputStream.java Mon Dec 15 12:09:49 2014 -0800
@@ -26,6 +26,7 @@
package java.io;
import java.nio.channels.FileChannel;
+import java.util.concurrent.atomic.AtomicBoolean;
import sun.nio.ch.FileChannelImpl;
@@ -57,10 +58,9 @@
*/
private final String path;
- private FileChannel channel = null;
+ private volatile FileChannel channel;
- private final Object closeLock = new Object();
- private volatile boolean closed = false;
+ private final AtomicBoolean closed = new AtomicBoolean(false);
/**
* Creates a <code>FileInputStream</code> by
@@ -313,14 +313,14 @@
* @spec JSR-51
*/
public void close() throws IOException {
- synchronized (closeLock) {
- if (closed) {
- return;
- }
- closed = true;
+ if (!closed.compareAndSet(false, true)) {
+ // if compareAndSet() returns false closed was already true
+ return;
}
- if (channel != null) {
- channel.close();
+
+ FileChannel fc = channel;
+ if (fc != null) {
+ fc.close();
}
fd.closeAll(new Closeable() {
@@ -364,12 +364,23 @@
* @spec JSR-51
*/
public FileChannel getChannel() {
- synchronized (this) {
- if (channel == null) {
- channel = FileChannelImpl.open(fd, path, true, false, this);
+ FileChannel fc = this.channel;
+ if (fc == null) {
+ synchronized (this) {
+ fc = this.channel;
+ if (fc == null) {
+ this.channel = fc = FileChannelImpl.open(fd, path, true, false, this);
+ if (closed.get()) {
+ try {
+ fc.close();
+ } catch (IOException ioe) {
+ throw new InternalError(ioe); // should not happen
+ }
+ }
+ }
}
- return channel;
}
+ return fc;
}
private static native void initIDs();
--- a/jdk/src/java.base/share/classes/java/io/FileOutputStream.java Mon Dec 15 19:21:59 2014 +0100
+++ b/jdk/src/java.base/share/classes/java/io/FileOutputStream.java Mon Dec 15 12:09:49 2014 -0800
@@ -26,6 +26,7 @@
package java.io;
import java.nio.channels.FileChannel;
+import java.util.concurrent.atomic.AtomicBoolean;
import sun.misc.SharedSecrets;
import sun.misc.JavaIOFileDescriptorAccess;
import sun.nio.ch.FileChannelImpl;
@@ -68,7 +69,7 @@
/**
* The associated channel, initialized lazily.
*/
- private FileChannel channel;
+ private volatile FileChannel channel;
/**
* The path of the referenced file
@@ -76,8 +77,7 @@
*/
private final String path;
- private final Object closeLock = new Object();
- private volatile boolean closed = false;
+ private final AtomicBoolean closed = new AtomicBoolean(false);
/**
* Creates a file output stream to write to the file with the
@@ -341,15 +341,14 @@
* @spec JSR-51
*/
public void close() throws IOException {
- synchronized (closeLock) {
- if (closed) {
- return;
- }
- closed = true;
+ if (!closed.compareAndSet(false, true)) {
+ // if compareAndSet() returns false closed was already true
+ return;
}
- if (channel != null) {
- channel.close();
+ FileChannel fc = channel;
+ if (fc != null) {
+ fc.close();
}
fd.closeAll(new Closeable() {
@@ -394,12 +393,23 @@
* @spec JSR-51
*/
public FileChannel getChannel() {
- synchronized (this) {
- if (channel == null) {
- channel = FileChannelImpl.open(fd, path, false, true, this);
+ FileChannel fc = this.channel;
+ if (fc == null) {
+ synchronized (this) {
+ fc = this.channel;
+ if (fc == null) {
+ this.channel = fc = FileChannelImpl.open(fd, path, false, true, this);
+ if (closed.get()) {
+ try {
+ fc.close();
+ } catch (IOException ioe) {
+ throw new InternalError(ioe); // should not happen
+ }
+ }
+ }
}
- return channel;
}
+ return fc;
}
/**
--- a/jdk/src/java.base/share/classes/java/io/RandomAccessFile.java Mon Dec 15 19:21:59 2014 +0100
+++ b/jdk/src/java.base/share/classes/java/io/RandomAccessFile.java Mon Dec 15 12:09:49 2014 -0800
@@ -26,6 +26,7 @@
package java.io;
import java.nio.channels.FileChannel;
+import java.util.concurrent.atomic.AtomicBoolean;
import sun.nio.ch.FileChannelImpl;
@@ -59,7 +60,7 @@
public class RandomAccessFile implements DataOutput, DataInput, Closeable {
private FileDescriptor fd;
- private FileChannel channel = null;
+ private volatile FileChannel channel;
private boolean rw;
/**
@@ -68,8 +69,7 @@
*/
private final String path;
- private Object closeLock = new Object();
- private volatile boolean closed = false;
+ private final AtomicBoolean closed = new AtomicBoolean(false);
private static final int O_RDONLY = 1;
private static final int O_RDWR = 2;
@@ -276,13 +276,24 @@
* @since 1.4
* @spec JSR-51
*/
- public final FileChannel getChannel() {
- synchronized (this) {
- if (channel == null) {
- channel = FileChannelImpl.open(fd, path, true, rw, this);
+ public FileChannel getChannel() {
+ FileChannel fc = this.channel;
+ if (fc == null) {
+ synchronized (this) {
+ fc = this.channel;
+ if (fc == null) {
+ this.channel = fc = FileChannelImpl.open(fd, path, true, rw, this);
+ if (closed.get()) {
+ try {
+ fc.close();
+ } catch (IOException ioe) {
+ throw new InternalError(ioe); // should not happen
+ }
+ }
+ }
}
- return channel;
}
+ return fc;
}
/**
@@ -604,14 +615,14 @@
* @spec JSR-51
*/
public void close() throws IOException {
- synchronized (closeLock) {
- if (closed) {
- return;
- }
- closed = true;
+ if (!closed.compareAndSet(false, true)) {
+ // if compareAndSet() returns false closed was already true
+ return;
}
- if (channel != null) {
- channel.close();
+
+ FileChannel fc = channel;
+ if (fc != null) {
+ fc.close();
}
fd.closeAll(new Closeable() {
--- a/jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java Mon Dec 15 19:21:59 2014 +0100
+++ b/jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java Mon Dec 15 12:09:49 2014 -0800
@@ -110,6 +110,9 @@
// -- Standard channel operations --
protected void implCloseChannel() throws IOException {
+ if (!fd.valid())
+ return; // nothing to do
+
// Release and invalidate any locks that we still hold
if (fileLockTable != null) {
for (FileLock fl: fileLockTable.removeAll()) {
--- a/jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java Mon Dec 15 19:21:59 2014 +0100
+++ b/jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java Mon Dec 15 12:09:49 2014 -0800
@@ -38,13 +38,9 @@
private FileKey() { }
- public static FileKey create(FileDescriptor fd) {
+ public static FileKey create(FileDescriptor fd) throws IOException {
FileKey fk = new FileKey();
- try {
- fk.init(fd);
- } catch (IOException ioe) {
- throw new Error(ioe);
- }
+ fk.init(fd);
return fk;
}
--- a/jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java Mon Dec 15 19:21:59 2014 +0100
+++ b/jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java Mon Dec 15 12:09:49 2014 -0800
@@ -39,13 +39,9 @@
private FileKey() { }
- public static FileKey create(FileDescriptor fd) {
+ public static FileKey create(FileDescriptor fd) throws IOException {
FileKey fk = new FileKey();
- try {
- fk.init(fd);
- } catch (IOException ioe) {
- throw new Error(ioe);
- }
+ fk.init(fd);
return fk;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/nio/channels/FileChannel/GetClosedChannel.java Mon Dec 15 12:09:49 2014 -0800
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2014, 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 8025619
+ * @summary Verify that a channel obtained from a closed stream is truly closed.
+ */
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.FileChannel;
+
+public class GetClosedChannel {
+ private static final int NUM_CHANNELS = 3;
+ private static final int NUM_EXCEPTIONS = 2*NUM_CHANNELS;
+
+ public static void main(String[] args) throws IOException {
+ int exceptions = 0;
+ int openChannels = 0;
+
+ for (int i = 0; i < NUM_CHANNELS; i++) {
+ File f = File.createTempFile("fcbug", ".tmp");
+ f.deleteOnExit();
+
+ FileChannel fc = null;
+ boolean shared = false;
+ switch (i) {
+ case 0:
+ System.out.print("FileInputStream...");
+ FileInputStream fis = new FileInputStream(f);
+ fis.close();
+ fc = fis.getChannel();
+ if (fc.isOpen()) {
+ System.err.println("FileInputStream channel should not be open");
+ openChannels++;
+ }
+ shared = true;
+ break;
+ case 1:
+ System.out.print("FileOutputStream...");
+ FileOutputStream fos = new FileOutputStream(f);
+ fos.close();
+ fc = fos.getChannel();
+ if (fc.isOpen()) {
+ System.err.println("FileOutputStream channel should not be open");
+ openChannels++;
+ }
+ break;
+ case 2:
+ System.out.print("RandomAccessFile...");
+ RandomAccessFile raf = new RandomAccessFile(f, "rw");
+ raf.close();
+ fc = raf.getChannel();
+ if (fc.isOpen()) {
+ System.err.println("RandomAccessFile channel should not be open");
+ openChannels++;
+ }
+ break;
+ default:
+ assert false : "Should not get here";
+ }
+
+ try {
+ long position = fc.position();
+ System.err.println("Channel "+i+" position is "+position);
+ } catch (ClosedChannelException cce) {
+ exceptions++;
+ }
+
+ try {
+ fc.tryLock(0, Long.MAX_VALUE, shared);
+ } catch (ClosedChannelException e) {
+ System.out.println("OK");
+ exceptions++;
+ } catch (Error err) {
+ System.err.println(err);
+ }
+ }
+
+ if (exceptions != NUM_EXCEPTIONS || openChannels != 0) {
+ throw new RuntimeException("FAILED:" +
+ " ClosedChannelExceptions: expected: " + NUM_EXCEPTIONS +
+ " actual: " + exceptions + ";" + System.lineSeparator() +
+ " number of open channels: expected: 0 " +
+ " actual: " + openChannels + ".");
+ }
+ }
+}