7082769: FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor be closed when still in use
Reviewed-by: alanb
--- a/jdk/src/share/classes/java/io/FileInputStream.java Sat Sep 10 21:30:20 2011 -0700
+++ b/jdk/src/share/classes/java/io/FileInputStream.java Tue Sep 13 11:21:51 2011 +0100
@@ -56,16 +56,6 @@
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 @@
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 @@
*/
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);
- }
}
}
}
--- a/jdk/src/share/classes/java/io/FileOutputStream.java Sat Sep 10 21:30:20 2011 -0700
+++ b/jdk/src/share/classes/java/io/FileOutputStream.java Tue Sep 13 11:21:51 2011 +0100
@@ -63,21 +63,12 @@
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 @@
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 @@
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);
- }
}
}
}
--- a/jdk/src/share/classes/java/io/RandomAccessFile.java Sat Sep 10 21:30:20 2011 -0700
+++ b/jdk/src/share/classes/java/io/RandomAccessFile.java Tue Sep 13 11:21:51 2011 +0100
@@ -1,5 +1,5 @@
/*
- * 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,8 +590,15 @@
* Decrement FD use count associated with this stream.
* The count got incremented by FileDescriptor during its construction.
*/
- fd.decrementAndGetUseCount();
- close0();
+ int useCount = fd.decrementAndGetUseCount();
+
+ /*
+ * If FileDescriptor is still in use by another stream, we
+ * will not close it.
+ */
+ if (useCount <= 0) {
+ close0();
+ }
}
//
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/io/etc/FileDescriptorSharing.java Tue Sep 13 11:21:51 2011 +0100
@@ -0,0 +1,336 @@
+/*
+ * 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();
+ }
+ }
+ }
+}