diff --git a/jdk/src/java.base/share/classes/java/io/FileInputStream.java b/jdk/src/java.base/share/classes/java/io/FileInputStream.java index b501b45707f..cc77e5bd7c4 100644 --- a/jdk/src/java.base/share/classes/java/io/FileInputStream.java +++ b/jdk/src/java.base/share/classes/java/io/FileInputStream.java @@ -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 @@ class FileInputStream extends InputStream */ 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 FileInputStream by @@ -313,14 +313,14 @@ class FileInputStream extends InputStream * @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 @@ class FileInputStream extends InputStream * @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(); diff --git a/jdk/src/java.base/share/classes/java/io/FileOutputStream.java b/jdk/src/java.base/share/classes/java/io/FileOutputStream.java index 43a7d053bbb..3c28433d15e 100644 --- a/jdk/src/java.base/share/classes/java/io/FileOutputStream.java +++ b/jdk/src/java.base/share/classes/java/io/FileOutputStream.java @@ -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 @@ class FileOutputStream extends OutputStream /** * The associated channel, initialized lazily. */ - private FileChannel channel; + private volatile FileChannel channel; /** * The path of the referenced file @@ -76,8 +77,7 @@ class FileOutputStream extends OutputStream */ 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 @@ class FileOutputStream extends OutputStream * @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 @@ class FileOutputStream extends OutputStream * @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; } /** diff --git a/jdk/src/java.base/share/classes/java/io/RandomAccessFile.java b/jdk/src/java.base/share/classes/java/io/RandomAccessFile.java index d6b79b245cd..ef5e76aea76 100644 --- a/jdk/src/java.base/share/classes/java/io/RandomAccessFile.java +++ b/jdk/src/java.base/share/classes/java/io/RandomAccessFile.java @@ -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 @@ import sun.nio.ch.FileChannelImpl; 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 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { */ 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 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { * @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 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { * @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() { diff --git a/jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java b/jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java index 8f18d44d63e..5981fd883db 100644 --- a/jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java +++ b/jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java @@ -110,6 +110,9 @@ public class FileChannelImpl // -- 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()) { diff --git a/jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java b/jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java index 50abd3077b0..0c3111464d7 100644 --- a/jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java +++ b/jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java @@ -38,13 +38,9 @@ public class FileKey { 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; } diff --git a/jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java b/jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java index 95f78d406e7..a3fe2550232 100644 --- a/jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java +++ b/jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java @@ -39,13 +39,9 @@ public class FileKey { 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; } diff --git a/jdk/test/java/nio/channels/FileChannel/GetClosedChannel.java b/jdk/test/java/nio/channels/FileChannel/GetClosedChannel.java new file mode 100644 index 00000000000..2c9947174fb --- /dev/null +++ b/jdk/test/java/nio/channels/FileChannel/GetClosedChannel.java @@ -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 + "."); + } + } +}