# HG changeset patch
# User bpb
# Date 1418153553 28800
# Tue Dec 09 11:32:33 2014 -0800
# Node ID c23fbe4ca9d7256644eb8b7df1704807f069fcf4
# Parent cb475099ceacabba8f5f650b050e8d1c32a6cb68
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: TBD
diff --git a/src/java.base/share/classes/java/io/FileInputStream.java b/src/java.base/share/classes/java/io/FileInputStream.java
--- a/src/java.base/share/classes/java/io/FileInputStream.java
+++ b/src/java.base/share/classes/java/io/FileInputStream.java
@@ -367,6 +367,13 @@
synchronized (this) {
if (channel == null) {
channel = FileChannelImpl.open(fd, path, true, false, this);
+ if (closed) {
+ try {
+ channel.close();
+ } catch (IOException ioe) {
+ throw new InternalError(ioe); // should not happen
+ }
+ }
}
return channel;
}
diff --git a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java
--- a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java
+++ b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java
@@ -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()) {
diff --git a/src/java.base/unix/classes/sun/nio/ch/FileKey.java b/src/java.base/unix/classes/sun/nio/ch/FileKey.java
--- a/src/java.base/unix/classes/sun/nio/ch/FileKey.java
+++ b/src/java.base/unix/classes/sun/nio/ch/FileKey.java
@@ -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;
}
diff --git a/src/java.base/windows/classes/sun/nio/ch/FileKey.java b/src/java.base/windows/classes/sun/nio/ch/FileKey.java
--- a/src/java.base/windows/classes/sun/nio/ch/FileKey.java
+++ b/src/java.base/windows/classes/sun/nio/ch/FileKey.java
@@ -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;
}
diff --git a/test/java/nio/channels/FileChannel/InvalidFileChannel.java b/test/java/nio/channels/FileChannel/InvalidFileChannel.java
new file mode 100644
--- /dev/null
+++ b/test/java/nio/channels/FileChannel/InvalidFileChannel.java
@@ -0,0 +1,83 @@
+/*
+ * 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 Test for IOException when using a channel from a closed stream.
+ */
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.channels.FileChannel;
+
+public class InvalidFileChannel {
+
+ public static void main(String[] args) throws IOException {
+ int exceptions = 0;
+
+ for (int i = 0; i < 3; 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();
+ shared = true;
+ break;
+ case 1:
+ System.out.print("FileOutputStream...");
+ FileOutputStream fos = new FileOutputStream(f);
+ fos.close();
+ fc = fos.getChannel();
+ break;
+ case 2:
+ System.out.print("RandomAccessFile...");
+ RandomAccessFile raf = new RandomAccessFile(f, "rw");
+ raf.close();
+ fc = raf.getChannel();
+ break;
+ }
+
+ try {
+ fc.tryLock(0, Long.MAX_VALUE, shared);
+ } catch (IOException e) {
+ System.out.println("OK");
+ exceptions++;
+ } catch (Error err) {
+ System.err.println(err);
+ }
+ }
+
+ if (exceptions != 3) {
+ throw new RuntimeException("FAILED: expected 3 IOExceptions but obtained "+exceptions);
+ }
+ }
+}
# HG changeset patch
# User bpb
# Date 1418153564 28800
# Tue Dec 09 11:32:44 2014 -0800
# Node ID 4561b3018b8d6e7b7bb608131f85af55dd7c6548
# Parent c23fbe4ca9d7256644eb8b7df1704807f069fcf4
8025619.01
diff --git a/src/java.base/share/classes/java/io/FileInputStream.java b/src/java.base/share/classes/java/io/FileInputStream.java
--- a/src/java.base/share/classes/java/io/FileInputStream.java
+++ b/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;
@@ -59,8 +60,7 @@
private FileChannel channel = null;
- private final Object closeLock = new Object();
- private volatile boolean closed = false;
+ private AtomicBoolean closed = new AtomicBoolean(false);
/**
* Creates a FileInputStream by
@@ -313,12 +313,11 @@
* @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();
}
@@ -367,7 +366,7 @@
synchronized (this) {
if (channel == null) {
channel = FileChannelImpl.open(fd, path, true, false, this);
- if (closed) {
+ if (closed.get()) {
try {
channel.close();
} catch (IOException ioe) {
diff --git a/src/java.base/share/classes/java/io/FileOutputStream.java b/src/java.base/share/classes/java/io/FileOutputStream.java
--- a/src/java.base/share/classes/java/io/FileOutputStream.java
+++ b/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;
@@ -76,8 +77,7 @@
*/
private final String path;
- private final Object closeLock = new Object();
- private volatile boolean closed = false;
+ private AtomicBoolean closed = new AtomicBoolean(false);
/**
* Creates a file output stream to write to the file with the
@@ -341,11 +341,9 @@
* @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) {
@@ -397,6 +395,13 @@
synchronized (this) {
if (channel == null) {
channel = FileChannelImpl.open(fd, path, false, true, this);
+ if (closed.get()) {
+ try {
+ channel.close();
+ } catch (IOException ioe) {
+ throw new InternalError(ioe); // should not happen
+ }
+ }
}
return channel;
}
diff --git a/src/java.base/share/classes/java/io/RandomAccessFile.java b/src/java.base/share/classes/java/io/RandomAccessFile.java
--- a/src/java.base/share/classes/java/io/RandomAccessFile.java
+++ b/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;
@@ -68,8 +69,7 @@
*/
private final String path;
- private Object closeLock = new Object();
- private volatile boolean closed = false;
+ private AtomicBoolean closed = new AtomicBoolean(false);
private static final int O_RDONLY = 1;
private static final int O_RDWR = 2;
@@ -280,6 +280,13 @@
synchronized (this) {
if (channel == null) {
channel = FileChannelImpl.open(fd, path, true, rw, this);
+ if (closed.get()) {
+ try {
+ channel.close();
+ } catch (IOException ioe) {
+ throw new InternalError(ioe); // should not happen
+ }
+ }
}
return channel;
}
@@ -604,12 +611,11 @@
* @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();
}
# HG changeset patch
# User bpb
# Date 1418153564 28800
# Tue Dec 09 11:32:44 2014 -0800
# Node ID 7fcc24f800f54d40c8608575e3cbac8485cd687d
# Parent 4561b3018b8d6e7b7bb608131f85af55dd7c6548
8025619.02
diff --git a/src/java.base/share/classes/java/io/FileInputStream.java b/src/java.base/share/classes/java/io/FileInputStream.java
--- a/src/java.base/share/classes/java/io/FileInputStream.java
+++ b/src/java.base/share/classes/java/io/FileInputStream.java
@@ -58,9 +58,9 @@
*/
private final String path;
- private FileChannel channel = null;
+ private volatile FileChannel channel = null;
- private AtomicBoolean closed = new AtomicBoolean(false);
+ private final AtomicBoolean closed = new AtomicBoolean(false);
/**
* Creates a FileInputStream by
@@ -363,8 +363,8 @@
* @spec JSR-51
*/
public FileChannel getChannel() {
- synchronized (this) {
- if (channel == null) {
+ if (channel == null) {
+ synchronized (this) {
channel = FileChannelImpl.open(fd, path, true, false, this);
if (closed.get()) {
try {
@@ -374,8 +374,9 @@
}
}
}
- return channel;
}
+
+ return channel;
}
private static native void initIDs();
diff --git a/src/java.base/share/classes/java/io/FileOutputStream.java b/src/java.base/share/classes/java/io/FileOutputStream.java
--- a/src/java.base/share/classes/java/io/FileOutputStream.java
+++ b/src/java.base/share/classes/java/io/FileOutputStream.java
@@ -69,7 +69,7 @@
/**
* The associated channel, initialized lazily.
*/
- private FileChannel channel;
+ private volatile FileChannel channel;
/**
* The path of the referenced file
@@ -77,7 +77,7 @@
*/
private final String path;
- private AtomicBoolean closed = new AtomicBoolean(false);
+ private final AtomicBoolean closed = new AtomicBoolean(false);
/**
* Creates a file output stream to write to the file with the
@@ -392,8 +392,8 @@
* @spec JSR-51
*/
public FileChannel getChannel() {
- synchronized (this) {
- if (channel == null) {
+ if (channel == null) {
+ synchronized (this) {
channel = FileChannelImpl.open(fd, path, false, true, this);
if (closed.get()) {
try {
@@ -403,8 +403,9 @@
}
}
}
- return channel;
}
+
+ return channel;
}
/**
diff --git a/src/java.base/share/classes/java/io/RandomAccessFile.java b/src/java.base/share/classes/java/io/RandomAccessFile.java
--- a/src/java.base/share/classes/java/io/RandomAccessFile.java
+++ b/src/java.base/share/classes/java/io/RandomAccessFile.java
@@ -60,7 +60,7 @@
public class RandomAccessFile implements DataOutput, DataInput, Closeable {
private FileDescriptor fd;
- private FileChannel channel = null;
+ private volatile FileChannel channel = null;
private boolean rw;
/**
@@ -69,7 +69,7 @@
*/
private final String path;
- private AtomicBoolean closed = new AtomicBoolean(false);
+ private final AtomicBoolean closed = new AtomicBoolean(false);
private static final int O_RDONLY = 1;
private static final int O_RDWR = 2;
@@ -277,8 +277,8 @@
* @spec JSR-51
*/
public final FileChannel getChannel() {
- synchronized (this) {
- if (channel == null) {
+ if (channel == null) {
+ synchronized (this) {
channel = FileChannelImpl.open(fd, path, true, rw, this);
if (closed.get()) {
try {
@@ -288,8 +288,9 @@
}
}
}
- return channel;
}
+
+ return channel;
}
/**
diff --git a/test/java/nio/channels/FileChannel/InvalidFileChannel.java b/test/java/nio/channels/FileChannel/InvalidFileChannel.java
--- a/test/java/nio/channels/FileChannel/InvalidFileChannel.java
+++ b/test/java/nio/channels/FileChannel/InvalidFileChannel.java
@@ -31,14 +31,17 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
+import java.nio.channels.ClosedChannelException;
import java.nio.channels.FileChannel;
public class InvalidFileChannel {
+ 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;
- for (int i = 0; i < 3; i++) {
+ for (int i = 0; i < NUM_CHANNELS; i++) {
File f = File.createTempFile("fcbug", ".tmp");
f.deleteOnExit();
@@ -64,6 +67,15 @@
raf.close();
fc = raf.getChannel();
break;
+ default:
+ assert false : "Should not get here";
+ }
+
+ try {
+ long position = fc.position();
+ System.out.println("Channel "+i+" position is "+position);
+ } catch (ClosedChannelException cce) {
+ exceptions++;
}
try {
@@ -76,8 +88,9 @@
}
}
- if (exceptions != 3) {
- throw new RuntimeException("FAILED: expected 3 IOExceptions but obtained "+exceptions);
+ if (exceptions != NUM_EXCEPTIONS) {
+ throw new RuntimeException("FAILED: expected "+NUM_EXCEPTIONS+
+ " IOExceptions but obtained "+exceptions);
}
}
}
# HG changeset patch
# User bpb
# Date 1418155579 28800
# Tue Dec 09 12:06:19 2014 -0800
# Node ID 8eaa52ab1a3eb060d9fa5cdf65cc3ec2fb9f3c18
# Parent 7fcc24f800f54d40c8608575e3cbac8485cd687d
8025619.03
diff --git a/src/java.base/share/classes/java/io/FileInputStream.java b/src/java.base/share/classes/java/io/FileInputStream.java
--- a/src/java.base/share/classes/java/io/FileInputStream.java
+++ b/src/java.base/share/classes/java/io/FileInputStream.java
@@ -58,7 +58,7 @@
*/
private final String path;
- private volatile FileChannel channel = null;
+ private volatile FileChannel channel;
private final AtomicBoolean closed = new AtomicBoolean(false);
@@ -318,8 +318,9 @@
return;
}
- if (channel != null) {
- channel.close();
+ FileChannel fc = channel;
+ if (fc != null) {
+ fc.close();
}
fd.closeAll(new Closeable() {
@@ -363,20 +364,23 @@
* @spec JSR-51
*/
public FileChannel getChannel() {
- if (channel == null) {
+ FileChannel fc = this.channel;
+ if (fc == null) {
synchronized (this) {
- channel = FileChannelImpl.open(fd, path, true, false, this);
- if (closed.get()) {
- try {
- channel.close();
- } catch (IOException ioe) {
- throw new InternalError(ioe); // should not happen
+ 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/src/java.base/share/classes/java/io/FileOutputStream.java b/src/java.base/share/classes/java/io/FileOutputStream.java
--- a/src/java.base/share/classes/java/io/FileOutputStream.java
+++ b/src/java.base/share/classes/java/io/FileOutputStream.java
@@ -346,8 +346,9 @@
return;
}
- if (channel != null) {
- channel.close();
+ FileChannel fc = channel;
+ if (fc != null) {
+ fc.close();
}
fd.closeAll(new Closeable() {
@@ -392,20 +393,23 @@
* @spec JSR-51
*/
public FileChannel getChannel() {
- if (channel == null) {
+ FileChannel fc = this.channel;
+ if (fc == null) {
synchronized (this) {
- channel = FileChannelImpl.open(fd, path, false, true, this);
- if (closed.get()) {
- try {
- channel.close();
- } catch (IOException ioe) {
- throw new InternalError(ioe); // should not happen
+ 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/src/java.base/share/classes/java/io/RandomAccessFile.java b/src/java.base/share/classes/java/io/RandomAccessFile.java
--- a/src/java.base/share/classes/java/io/RandomAccessFile.java
+++ b/src/java.base/share/classes/java/io/RandomAccessFile.java
@@ -60,7 +60,7 @@
public class RandomAccessFile implements DataOutput, DataInput, Closeable {
private FileDescriptor fd;
- private volatile FileChannel channel = null;
+ private volatile FileChannel channel;
private boolean rw;
/**
@@ -276,21 +276,24 @@
* @since 1.4
* @spec JSR-51
*/
- public final FileChannel getChannel() {
- if (channel == null) {
+ public FileChannel getChannel() {
+ FileChannel fc = this.channel;
+ if (fc == null) {
synchronized (this) {
- channel = FileChannelImpl.open(fd, path, true, rw, this);
- if (closed.get()) {
- try {
- channel.close();
- } catch (IOException ioe) {
- throw new InternalError(ioe); // should not happen
+ 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;
}
/**
@@ -617,8 +620,9 @@
return;
}
- if (channel != null) {
- channel.close();
+ FileChannel fc = channel;
+ if (fc != null) {
+ fc.close();
}
fd.closeAll(new Closeable() {
diff --git a/test/java/nio/channels/FileChannel/InvalidFileChannel.java b/test/java/nio/channels/FileChannel/GetClosedChannel.java
rename from test/java/nio/channels/FileChannel/InvalidFileChannel.java
rename to test/java/nio/channels/FileChannel/GetClosedChannel.java
--- a/test/java/nio/channels/FileChannel/InvalidFileChannel.java
+++ b/test/java/nio/channels/FileChannel/GetClosedChannel.java
@@ -24,7 +24,7 @@
/*
* @test
* @bug 8025619
- * @summary Test for IOException when using a channel from a closed stream.
+ * @summary Verify that a channel obtained from a closed stream is truly closed.
*/
import java.io.File;
import java.io.FileInputStream;
@@ -34,12 +34,13 @@
import java.nio.channels.ClosedChannelException;
import java.nio.channels.FileChannel;
-public class InvalidFileChannel {
+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");
@@ -53,6 +54,10 @@
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:
@@ -60,12 +65,20 @@
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";
@@ -73,14 +86,14 @@
try {
long position = fc.position();
- System.out.println("Channel "+i+" position is "+position);
+ System.err.println("Channel "+i+" position is "+position);
} catch (ClosedChannelException cce) {
exceptions++;
}
try {
fc.tryLock(0, Long.MAX_VALUE, shared);
- } catch (IOException e) {
+ } catch (ClosedChannelException e) {
System.out.println("OK");
exceptions++;
} catch (Error err) {
@@ -88,9 +101,12 @@
}
}
- if (exceptions != NUM_EXCEPTIONS) {
- throw new RuntimeException("FAILED: expected "+NUM_EXCEPTIONS+
- " IOExceptions but obtained "+exceptions);
+ 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 + ".");
}
}
}