OpenJDK / portola / portola

changeset 28062:52b80a88a63b

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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
author bpb
date Mon, 15 Dec 2014 12:09:49 -0800
parents 2a10901eac1b
children 997c263dff16
files jdk/src/java.base/share/classes/java/io/FileInputStream.java jdk/src/java.base/share/classes/java/io/FileOutputStream.java jdk/src/java.base/share/classes/java/io/RandomAccessFile.java jdk/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java jdk/src/java.base/unix/classes/sun/nio/ch/FileKey.java jdk/src/java.base/windows/classes/sun/nio/ch/FileKey.java jdk/test/java/nio/channels/FileChannel/GetClosedChannel.java
diffstat 7 files changed, 194 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- 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 + ".");
+        }
+    }
+}
© 2007, Oracle and/or its affiliates
Terms of Use · Privacy · Trademarks