7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file...

7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use
Reviewed-by: alanb

src/share/classes/java/io/FileInputStream.java

@@ -56,16 +56,6 @@ class FileInputStream extends InputStream
     private final Object closeLock = new Object();
     private volatile boolean closed = false;
 
-    private static final ThreadLocal<Boolean> runningFinalize =
-        new ThreadLocal<>();
-
-    private static boolean isRunningFinalize() {
-        Boolean val;
-        if ((val = runningFinalize.get()) != null)
-            return val.booleanValue();
-        return false;
-    }
-
     /**
      * Creates a <code>FileInputStream</code> by
      * opening a connection to an actual file,
@@ -319,10 +309,10 @@ class FileInputStream extends InputStream
         int useCount = fd.decrementAndGetUseCount();
 
         /*
-         * If FileDescriptor is still in use by another stream, the finalizer
+         * If FileDescriptor is still in use by another stream, we
          * will not close it.
          */
-        if ((useCount <= 0) || !isRunningFinalize()) {
+        if (useCount <= 0) {
             close0();
         }
     }
@@ -391,18 +381,7 @@ class FileInputStream extends InputStream
      */
     protected void finalize() throws IOException {
         if ((fd != null) &&  (fd != FileDescriptor.in)) {
-
-            /*
-             * Finalizer should not release the FileDescriptor if another
-             * stream is still using it. If the user directly invokes
-             * close() then the FileDescriptor is also released.
-             */
-            runningFinalize.set(Boolean.TRUE);
-            try {
                 close();
-            } finally {
-                runningFinalize.set(Boolean.FALSE);
-            }
         }
     }
 }

src/share/classes/java/io/FileOutputStream.java

@@ -63,21 +63,12 @@ class FileOutputStream extends OutputStream
     private final boolean append;
 
     /**
-     * The associated channel, initalized lazily.
+     * The associated channel, initialized lazily.
      */
     private FileChannel channel;
 
     private final Object closeLock = new Object();
     private volatile boolean closed = false;
-    private static final ThreadLocal<Boolean> runningFinalize =
-        new ThreadLocal<>();
-
-    private static boolean isRunningFinalize() {
-        Boolean val;
-        if ((val = runningFinalize.get()) != null)
-            return val.booleanValue();
-        return false;
-    }
 
     /**
      * Creates a file output stream to write to the file with the
@@ -355,10 +346,10 @@ class FileOutputStream extends OutputStream
         int useCount = fd.decrementAndGetUseCount();
 
         /*
-         * If FileDescriptor is still in use by another stream, the finalizer
+         * If FileDescriptor is still in use by another stream, we
          * will not close it.
          */
-        if ((useCount <= 0) || !isRunningFinalize()) {
+        if (useCount <= 0) {
             close0();
         }
     }
@@ -424,18 +415,7 @@ class FileOutputStream extends OutputStream
             if (fd == FileDescriptor.out || fd == FileDescriptor.err) {
                 flush();
             } else {
-
-                /*
-                 * Finalizer should not release the FileDescriptor if another
-                 * stream is still using it. If the user directly invokes
-                 * close() then the FileDescriptor is also released.
-                 */
-                runningFinalize.set(Boolean.TRUE);
-                try {
                     close();
-                } finally {
-                    runningFinalize.set(Boolean.FALSE);
-                }
             }
         }
     }

src/share/classes/java/io/RandomAccessFile.java

 /*
- * Copyright (c) 1994, 2007, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 2011, 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
@@ -590,9 +590,16 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable {
          * Decrement FD use count associated with this stream.
          * The count got incremented by FileDescriptor during its construction.
          */
-        fd.decrementAndGetUseCount();
+        int useCount = fd.decrementAndGetUseCount();
+
+        /*
+         * If FileDescriptor is still in use by another stream, we
+         * will not close it.
+         */
+        if (useCount <= 0) {
             close0();
         }
+    }
 
     //
     //  Some "reading/writing Java data types" methods stolen from

test/java/io/etc/FileDescriptorSharing.java

+/*
+ * Copyright (c) 2011, 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 6322678 7082769
+ * @summary FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor
+ *          to be closed while still in use.
+ * @run main/othervm FileDescriptorSharing
+ */
+
+import java.io.*;
+import java.nio.channels.FileChannel;
+import java.nio.channels.FileLock;
+import java.util.concurrent.CountDownLatch;
+
+public class FileDescriptorSharing {
+
+    final static int numFiles = 10;
+    volatile static boolean fail;
+
+    public static void main(String[] args) throws Exception {
+        TestFinalizer();
+        TestMultipleFD();
+        TestIsValid();
+        MultiThreadedFD();
+    }
+
+    /**
+     * We shouldn't discard a file descriptor until all streams have
+     * finished with it
+     */
+    private static void TestFinalizer() throws Exception {
+        FileDescriptor fd = null;
+        File tempFile = new File("TestFinalizer1.txt");
+        tempFile.deleteOnExit();
+        try (Writer writer = new FileWriter(tempFile)) {
+            for (int i=0; i<5; i++) {
+                writer.write("test file content test file content");
+            }
+        }
+
+        FileInputStream fis1 = new FileInputStream(tempFile);
+        fd = fis1.getFD();
+        // Create a new FIS based on the existing FD (so the two FIS's share the same native fd)
+        try (FileInputStream fis2 = new FileInputStream(fd)) {
+            // allow fis1 to be gc'ed
+            fis1 = null;
+            int ret = 0;
+            while(ret >= 0) {
+                // encourage gc
+                System.gc();
+                // read from fis2 - when fis1 is gc'ed and finalizer is run, read will fail
+                System.out.print(".");
+                ret = fis2.read();
+            }
+        }
+
+        // variation of above. Use RandomAccessFile to obtain a filedescriptor
+        File testFinalizerFile = new File("TestFinalizer");
+        RandomAccessFile raf = new RandomAccessFile(testFinalizerFile, "rw");
+        raf.writeBytes("test file content test file content");
+        raf.seek(0L);
+        fd = raf.getFD();
+        try (FileInputStream fis3 = new FileInputStream(fd)) {
+            // allow raf to be gc'ed
+            raf = null;
+            int ret = 0;
+            while (ret >= 0) {
+                // encourage gc
+                System.gc();
+                /*
+                 * read from fis3 - when raf is gc'ed and finalizer is run,
+                 * fd should still be valid.
+                 */
+                System.out.print(".");
+                ret = fis3.read();
+            }
+            if(!fd.valid()) {
+                throw new RuntimeException("TestFinalizer() : FileDescriptor should be valid");
+            }
+        } finally {
+            testFinalizerFile.delete();
+        }
+    }
+
+    /**
+     * Exercise FileDispatcher close()/preClose()
+     */
+    private static void TestMultipleFD() throws Exception {
+        RandomAccessFile raf = null;
+        FileOutputStream fos = null;
+        FileInputStream fis = null;
+        FileChannel fc = null;
+        FileLock fileLock = null;
+
+        File test1 = new File("test1");
+        try {
+            raf = new RandomAccessFile(test1, "rw");
+            fos = new FileOutputStream(raf.getFD());
+            fis = new FileInputStream(raf.getFD());
+            fc = raf.getChannel();
+            fileLock = fc.lock();
+            raf.setLength(0L);
+            fos.flush();
+            fos.write("TEST".getBytes());
+        } finally {
+            if (fileLock != null) fileLock.release();
+            if (fis != null) fis.close();
+            if (fos != null) fos.close();
+            if (raf != null) raf.close();
+            test1.delete();
+        }
+
+        /*
+         * Close out in different order to ensure FD is not
+         * closed out too early
+         */
+        File test2 = new File("test2");
+        try {
+            raf = new RandomAccessFile(test2, "rw");
+            fos = new FileOutputStream(raf.getFD());
+            fis = new FileInputStream(raf.getFD());
+            fc = raf.getChannel();
+            fileLock = fc.lock();
+            raf.setLength(0L);
+            fos.flush();
+            fos.write("TEST".getBytes());
+        } finally {
+            if (fileLock != null) fileLock.release();
+            if (raf != null) raf.close();
+            if (fos != null) fos.close();
+            if (fis != null) fis.close();
+            test2.delete();
+        }
+
+        // one more time, fos first this time
+        File test3 = new File("test3");
+        try {
+            raf = new RandomAccessFile(test3, "rw");
+            fos = new FileOutputStream(raf.getFD());
+            fis = new FileInputStream(raf.getFD());
+            fc = raf.getChannel();
+            fileLock = fc.lock();
+            raf.setLength(0L);
+            fos.flush();
+            fos.write("TEST".getBytes());
+        } finally {
+            if (fileLock != null) fileLock.release();
+            if (fos != null) fos.close();
+            if (raf != null) raf.close();
+            if (fis != null) fis.close();
+            test3.delete();
+        }
+    }
+
+    /**
+     * Similar to TestMultipleFD() but this time we
+     * just get and use FileDescriptor.valid() for testing.
+     */
+    private static void TestIsValid() throws Exception {
+        FileDescriptor fd = null;
+        RandomAccessFile raf = null;
+        FileOutputStream fos = null;
+        FileInputStream fis = null;
+        FileChannel fc = null;
+
+        File test1 = new File("test1");
+        try {
+            raf = new RandomAccessFile(test1, "rw");
+            fd = raf.getFD();
+            fos = new FileOutputStream(fd);
+            fis = new FileInputStream(fd);
+        } finally {
+            try {
+                if (fis != null) fis.close();
+                if (fos != null) fos.close();
+                if (!fd.valid()) {
+                    throw new RuntimeException("FileDescriptor should be valid");
+                }
+                if (raf != null) raf.close();
+                if (fd.valid()) {
+                    throw new RuntimeException("close() called and FileDescriptor still valid");
+                }
+            } finally {
+                if (raf != null) raf.close();
+                test1.delete();
+            }
+        }
+
+        /*
+         * Close out in different order to ensure FD is not
+         * closed out too early
+         */
+        File test2 = new File("test2");
+        try {
+            raf = new RandomAccessFile(test2, "rw");
+            fd = raf.getFD();
+            fos = new FileOutputStream(fd);
+            fis = new FileInputStream(fd);
+        } finally {
+            try {
+                if (raf != null) raf.close();
+                if (fos != null) fos.close();
+                if (!fd.valid()) {
+                    throw new RuntimeException("FileDescriptor should be valid");
+                }
+                if (fis != null) fis.close();
+                if (fd.valid()) {
+                    throw new RuntimeException("close() called and FileDescriptor still valid");
+                }
+            } finally {
+                test2.delete();
+            }
+        }
+
+        // one more time, fos first this time
+        File test3 = new File("test3");
+        try {
+            raf = new RandomAccessFile(test3, "rw");
+            fd = raf.getFD();
+            fos = new FileOutputStream(fd);
+            fis = new FileInputStream(fd);
+        } finally {
+            try {
+                if (fos != null) fos.close();
+                if (raf != null) raf.close();
+                if (!fd.valid()) {
+                    throw new RuntimeException("FileDescriptor should be valid");
+                }
+                if (fis != null) fis.close();
+                if (fd.valid()) {
+                    throw new RuntimeException("close() called and FileDescriptor still valid");
+                }
+            } finally {
+                test3.delete();
+            }
+        }
+    }
+
+    /**
+     * Test concurrent access to the same fd.useCount field
+     */
+    private static void MultiThreadedFD() throws Exception {
+        RandomAccessFile raf = null;
+        FileDescriptor fd = null;
+        int numThreads = 2;
+        CountDownLatch done = new CountDownLatch(numThreads);
+        OpenClose[] fileOpenClose = new OpenClose[numThreads];
+        File MultipleThreadedFD = new File("MultipleThreadedFD");
+        try {
+            raf = new RandomAccessFile(MultipleThreadedFD, "rw");
+            fd = raf.getFD();
+            for(int count=0;count<numThreads;count++) {
+                fileOpenClose[count] = new OpenClose(fd, done);
+                fileOpenClose[count].start();
+            }
+            done.await();
+        } finally {
+            try {
+                if(raf != null) raf.close();
+                // fd should now no longer be valid
+                if(fd.valid()) {
+                    throw new RuntimeException("FileDescriptor should not be valid");
+                }
+                // OpenClose thread tests failed
+                if(fail) {
+                    throw new RuntimeException("OpenClose thread tests failed.");
+                }
+            } finally {
+                MultipleThreadedFD.delete();
+            }
+        }
+    }
+
+    /**
+     * A thread which will open and close a number of FileInputStreams and
+     * FileOutputStreams referencing the same native file descriptor.
+     */
+    private static class OpenClose extends Thread {
+        private FileDescriptor fd = null;
+        private CountDownLatch done;
+        FileInputStream[] fisArray = new FileInputStream[numFiles];
+        FileOutputStream[] fosArray = new FileOutputStream[numFiles];
+
+        OpenClose(FileDescriptor filedescriptor, CountDownLatch done) {
+            this.fd = filedescriptor;
+            this.done = done;
+        }
+
+        public void run() {
+             try {
+                 for(int i=0;i<numFiles;i++) {
+                     fisArray[i] = new FileInputStream(fd);
+                     fosArray[i] = new FileOutputStream(fd);
+                 }
+
+                 // Now close out
+                 for(int i=0;i<numFiles;i++) {
+                     if(fisArray[i] != null) fisArray[i].close();
+                     if(fosArray[i] != null) fosArray[i].close();
+                 }
+
+             } catch(IOException ioe) {
+                 System.out.println("OpenClose encountered IO issue :" + ioe);
+                 fail = true;
+             } finally {
+                 if (!fd.valid()) { // fd should still be valid given RAF reference
+                     System.out.println("OpenClose: FileDescriptor should be valid");
+                     fail = true;
+                 }
+                 done.countDown();
+             }
+         }
+    }
+}