8025619: (fc) FileInputStream.getChannel on closed stream returns FileChannel that doesn't know that stream is closed

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
This commit is contained in:
Brian Burkhalter 2014-12-15 12:09:49 -08:00
parent c85244cab9
commit 871abc48e4
7 changed files with 194 additions and 55 deletions

View File

@ -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 <code>FileInputStream</code> 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();

View File

@ -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;
}
/**

View File

@ -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() {

View File

@ -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()) {

View File

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

View File

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

View File

@ -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 + ".");
}
}
}