diff --git a/jdk/src/share/classes/java/io/FileInputStream.java b/jdk/src/share/classes/java/io/FileInputStream.java index 9dc19557e54..d796bf090f0 100644 --- a/jdk/src/share/classes/java/io/FileInputStream.java +++ b/jdk/src/share/classes/java/io/FileInputStream.java @@ -124,7 +124,7 @@ class FileInputStream extends InputStream throw new NullPointerException(); } fd = new FileDescriptor(); - fd.incrementAndGetUseCount(); + fd.attach(this); open(name); } @@ -164,10 +164,9 @@ class FileInputStream extends InputStream /* * FileDescriptor is being shared by streams. - * Ensure that it's GC'ed only when all the streams/channels are done - * using it. + * Register this stream with FileDescriptor tracker. */ - fd.incrementAndGetUseCount(); + fd.attach(this); } /** @@ -294,27 +293,14 @@ class FileInputStream extends InputStream closed = true; } if (channel != null) { - /* - * Decrement the FD use count associated with the channel - * The use count is incremented whenever a new channel - * is obtained from this stream. - */ - fd.decrementAndGetUseCount(); channel.close(); } - /* - * Decrement the FD use count associated with this stream - */ - int useCount = fd.decrementAndGetUseCount(); - - /* - * If FileDescriptor is still in use by another stream, we - * will not close it. - */ - if (useCount <= 0) { - close0(); - } + fd.closeAll(new Closeable() { + public void close() throws IOException { + close0(); + } + }); } /** @@ -328,7 +314,9 @@ class FileInputStream extends InputStream * @see java.io.FileDescriptor */ public final FileDescriptor getFD() throws IOException { - if (fd != null) return fd; + if (fd != null) { + return fd; + } throw new IOException(); } @@ -352,13 +340,6 @@ class FileInputStream extends InputStream synchronized (this) { if (channel == null) { channel = FileChannelImpl.open(fd, true, false, this); - - /* - * Increment fd's use count. Invoking the channel's close() - * method will result in decrementing the use count set for - * the channel. - */ - fd.incrementAndGetUseCount(); } return channel; } @@ -381,7 +362,12 @@ class FileInputStream extends InputStream */ protected void finalize() throws IOException { if ((fd != null) && (fd != FileDescriptor.in)) { - close(); + /* if fd is shared, the references in FileDescriptor + * will ensure that finalizer is only called when + * safe to do so. All references using the fd have + * become unreachable. We can call close() + */ + close(); } } } diff --git a/jdk/src/share/classes/java/io/FileOutputStream.java b/jdk/src/share/classes/java/io/FileOutputStream.java index 4a8a724ba15..50bd29bef5f 100644 --- a/jdk/src/share/classes/java/io/FileOutputStream.java +++ b/jdk/src/share/classes/java/io/FileOutputStream.java @@ -197,9 +197,9 @@ class FileOutputStream extends OutputStream throw new NullPointerException(); } this.fd = new FileDescriptor(); + fd.attach(this); this.append = append; - fd.incrementAndGetUseCount(); open(name, append); } @@ -237,12 +237,7 @@ class FileOutputStream extends OutputStream this.fd = fdObj; this.append = false; - /* - * FileDescriptor is being shared by streams. - * Ensure that it's GC'ed only when all the streams/channels are done - * using it. - */ - fd.incrementAndGetUseCount(); + fd.attach(this); } /** @@ -331,27 +326,14 @@ class FileOutputStream extends OutputStream } if (channel != null) { - /* - * Decrement FD use count associated with the channel - * The use count is incremented whenever a new channel - * is obtained from this stream. - */ - fd.decrementAndGetUseCount(); channel.close(); } - /* - * Decrement FD use count associated with this stream - */ - int useCount = fd.decrementAndGetUseCount(); - - /* - * If FileDescriptor is still in use by another stream, we - * will not close it. - */ - if (useCount <= 0) { - close0(); - } + fd.closeAll(new Closeable() { + public void close() throws IOException { + close0(); + } + }); } /** @@ -365,7 +347,9 @@ class FileOutputStream extends OutputStream * @see java.io.FileDescriptor */ public final FileDescriptor getFD() throws IOException { - if (fd != null) return fd; + if (fd != null) { + return fd; + } throw new IOException(); } @@ -390,13 +374,6 @@ class FileOutputStream extends OutputStream synchronized (this) { if (channel == null) { channel = FileChannelImpl.open(fd, false, true, append, this); - - /* - * Increment fd's use count. Invoking the channel's close() - * method will result in decrementing the use count set for - * the channel. - */ - fd.incrementAndGetUseCount(); } return channel; } @@ -415,7 +392,12 @@ class FileOutputStream extends OutputStream if (fd == FileDescriptor.out || fd == FileDescriptor.err) { flush(); } else { - close(); + /* if fd is shared, the references in FileDescriptor + * will ensure that finalizer is only called when + * safe to do so. All references using the fd have + * become unreachable. We can call close() + */ + close(); } } } diff --git a/jdk/src/share/classes/java/io/RandomAccessFile.java b/jdk/src/share/classes/java/io/RandomAccessFile.java index a2863c59a08..65bfbf46c79 100644 --- a/jdk/src/share/classes/java/io/RandomAccessFile.java +++ b/jdk/src/share/classes/java/io/RandomAccessFile.java @@ -229,7 +229,7 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { throw new NullPointerException(); } fd = new FileDescriptor(); - fd.incrementAndGetUseCount(); + fd.attach(this); open(name, imode); } @@ -242,7 +242,9 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { * @see java.io.FileDescriptor */ public final FileDescriptor getFD() throws IOException { - if (fd != null) return fd; + if (fd != null) { + return fd; + } throw new IOException(); } @@ -268,17 +270,6 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { synchronized (this) { if (channel == null) { channel = FileChannelImpl.open(fd, true, rw, this); - - /* - * FileDescriptor could be shared by FileInputStream or - * FileOutputStream. - * Ensure that FD is GC'ed only when all the streams/channels - * are done using it. - * Increment fd's use count. Invoking the channel's close() - * method will result in decrementing the use count set for - * the channel. - */ - fd.incrementAndGetUseCount(); } return channel; } @@ -577,28 +568,14 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { closed = true; } if (channel != null) { - /* - * Decrement FD use count associated with the channel. The FD use - * count is incremented whenever a new channel is obtained from - * this stream. - */ - fd.decrementAndGetUseCount(); channel.close(); } - /* - * Decrement FD use count associated with this stream. - * The count got incremented by FileDescriptor during its construction. - */ - int useCount = fd.decrementAndGetUseCount(); - - /* - * If FileDescriptor is still in use by another stream, we - * will not close it. - */ - if (useCount <= 0) { - close0(); - } + fd.closeAll(new Closeable() { + public void close() throws IOException { + close0(); + } + }); } // diff --git a/jdk/src/solaris/classes/java/io/FileDescriptor.java b/jdk/src/solaris/classes/java/io/FileDescriptor.java index 9e7390d8548..1f0d086d745 100644 --- a/jdk/src/solaris/classes/java/io/FileDescriptor.java +++ b/jdk/src/solaris/classes/java/io/FileDescriptor.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1995, 2008, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1995, 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 @@ -25,7 +25,8 @@ package java.io; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.ArrayList; +import java.util.List; /** * Instances of the file descriptor class serve as an opaque handle @@ -46,12 +47,9 @@ public final class FileDescriptor { private int fd; - /** - * A counter for tracking the FIS/FOS/RAF instances that - * use this FileDescriptor. The FIS/FOS.finalize() will not release - * the FileDescriptor if it is still under user by a stream. - */ - private AtomicInteger useCount; + private Closeable parent; + private List otherParents; + private boolean closed; /** * Constructs an (invalid) FileDescriptor @@ -59,12 +57,10 @@ public final class FileDescriptor { */ public /**/ FileDescriptor() { fd = -1; - useCount = new AtomicInteger(); } private /* */ FileDescriptor(int fd) { this.fd = fd; - useCount = new AtomicInteger(); } /** @@ -164,13 +160,67 @@ public final class FileDescriptor { ); } - // package private methods used by FIS, FOS and RAF + /* + * Package private methods to track referents. + * If multiple streams point to the same FileDescriptor, we cycle + * through the list of all referents and call close() + */ - int incrementAndGetUseCount() { - return useCount.incrementAndGet(); + /** + * Attach a Closeable to this FD for tracking. + * parent reference is added to otherParents when + * needed to make closeAll simpler. + */ + synchronized void attach(Closeable c) { + if (parent == null) { + // first caller gets to do this + parent = c; + } else if (otherParents == null) { + otherParents = new ArrayList<>(); + otherParents.add(parent); + otherParents.add(c); + } else { + otherParents.add(c); + } } - int decrementAndGetUseCount() { - return useCount.decrementAndGet(); + /** + * Cycle through all Closeables sharing this FD and call + * close() on each one. + * + * The caller closeable gets to call close0(). + */ + @SuppressWarnings("try") + synchronized void closeAll(Closeable releaser) throws IOException { + if (!closed) { + closed = true; + IOException ioe = null; + try (Closeable c = releaser) { + if (otherParents != null) { + for (Closeable referent : otherParents) { + try { + referent.close(); + } catch(IOException x) { + if (ioe == null) { + ioe = x; + } else { + ioe.addSuppressed(x); + } + } + } + } + } catch(IOException ex) { + /* + * If releaser close() throws IOException + * add other exceptions as suppressed. + */ + if (ioe != null) + ex.addSuppressed(ioe); + ioe = ex; + } finally { + if (ioe != null) + throw ioe; + } + } } } diff --git a/jdk/src/windows/classes/java/io/FileDescriptor.java b/jdk/src/windows/classes/java/io/FileDescriptor.java index 606d080989e..bfcda5b497e 100644 --- a/jdk/src/windows/classes/java/io/FileDescriptor.java +++ b/jdk/src/windows/classes/java/io/FileDescriptor.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2008, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 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 @@ -25,7 +25,8 @@ package java.io; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.ArrayList; +import java.util.List; /** * Instances of the file descriptor class serve as an opaque handle @@ -45,13 +46,9 @@ public final class FileDescriptor { private long handle; - /** - * A use counter for tracking the FIS/FOS/RAF instances that - * use this FileDescriptor. The FIS/FOS.finalize() will not release - * the FileDescriptor if it is still under use by any stream. - */ - private AtomicInteger useCount; - + private Closeable parent; + private List otherParents; + private boolean closed; /** * Constructs an (invalid) FileDescriptor @@ -60,7 +57,6 @@ public final class FileDescriptor { public /**/ FileDescriptor() { fd = -1; handle = -1; - useCount = new AtomicInteger(); } static { @@ -168,13 +164,67 @@ public final class FileDescriptor { return desc; } - // package private methods used by FIS, FOS and RAF. + /* + * Package private methods to track referents. + * If multiple streams point to the same FileDescriptor, we cycle + * through the list of all referents and call close() + */ - int incrementAndGetUseCount() { - return useCount.incrementAndGet(); + /** + * Attach a Closeable to this FD for tracking. + * parent reference is added to otherParents when + * needed to make closeAll simpler. + */ + synchronized void attach(Closeable c) { + if (parent == null) { + // first caller gets to do this + parent = c; + } else if (otherParents == null) { + otherParents = new ArrayList<>(); + otherParents.add(parent); + otherParents.add(c); + } else { + otherParents.add(c); + } } - int decrementAndGetUseCount() { - return useCount.decrementAndGet(); + /** + * Cycle through all Closeables sharing this FD and call + * close() on each one. + * + * The caller closeable gets to call close0(). + */ + @SuppressWarnings("try") + synchronized void closeAll(Closeable releaser) throws IOException { + if (!closed) { + closed = true; + IOException ioe = null; + try (Closeable c = releaser) { + if (otherParents != null) { + for (Closeable referent : otherParents) { + try { + referent.close(); + } catch(IOException x) { + if (ioe == null) { + ioe = x; + } else { + ioe.addSuppressed(x); + } + } + } + } + } catch(IOException ex) { + /* + * If releaser close() throws IOException + * add other exceptions as suppressed. + */ + if (ioe != null) + ex.addSuppressed(ioe); + ioe = ex; + } finally { + if (ioe != null) + throw ioe; + } + } } } diff --git a/jdk/test/java/io/FileDescriptor/FileChannelFDTest.java b/jdk/test/java/io/FileDescriptor/FileChannelFDTest.java deleted file mode 100644 index 8a44859079b..00000000000 --- a/jdk/test/java/io/FileDescriptor/FileChannelFDTest.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright (c) 2006, 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 - * @summary Test for making sure that fd is closed during - * finalization of a stream, when an associated - * file channel is not available - */ - -import java.io.*; -import java.nio.*; -import java.nio.channels.*; - -public class FileChannelFDTest { - - static byte data[] = new byte[] {48, 49, 50, 51, 52, 53, 54, 55, 56, 57,}; - static String inFileName = "fd-in-test.txt"; - static String outFileName = "fd-out-test.txt"; - static File inFile; - static File outFile; - - private static void writeToInFile() throws IOException { - FileOutputStream out = new FileOutputStream(inFile); - out.write(data); - out.close(); - } - - public static void main(String[] args) - throws Exception { - - inFile= new File(System.getProperty("test.dir", "."), - inFileName); - inFile.createNewFile(); - inFile.deleteOnExit(); - writeToInFile(); - - outFile = new File(System.getProperty("test.dir", "."), - outFileName); - outFile.createNewFile(); - outFile.deleteOnExit(); - - doFileChannel(); - } - - private static void doFileChannel() throws Exception { - - FileInputStream fis = new FileInputStream(inFile); - FileDescriptor fd = fis.getFD(); - FileChannel fc = fis.getChannel(); - System.out.println("Created fis:" + fis); - - /** - * Encourage the GC - */ - fis = null; - fc = null; - System.gc(); - Thread.sleep(500); - - if (fd.valid()) { - throw new Exception("Finalizer either didn't run --" + - "try increasing the Thread's sleep time after System.gc();" + - "or the finalizer didn't close the file"); - } - - System.out.println("File Closed successfully"); - System.out.println(); - } -} diff --git a/jdk/test/java/io/etc/FileDescriptorSharing.java b/jdk/test/java/io/FileDescriptor/Sharing.java similarity index 72% rename from jdk/test/java/io/etc/FileDescriptorSharing.java rename to jdk/test/java/io/FileDescriptor/Sharing.java index d46f61a5282..c340a983ebe 100644 --- a/jdk/test/java/io/etc/FileDescriptorSharing.java +++ b/jdk/test/java/io/FileDescriptor/Sharing.java @@ -23,10 +23,9 @@ /* * @test - * @bug 6322678 7082769 - * @summary FileInputStream/FileOutputStream/RandomAccessFile allow file descriptor - * to be closed while still in use. - * @run main/othervm FileDescriptorSharing + * @bug 7105952 6322678 7082769 + * @summary Improve finalisation for FileInputStream/FileOutputStream/RandomAccessFile + * @run main/othervm Sharing */ import java.io.*; @@ -34,7 +33,7 @@ import java.nio.channels.FileChannel; import java.nio.channels.FileLock; import java.util.concurrent.CountDownLatch; -public class FileDescriptorSharing { +public class Sharing { final static int numFiles = 10; volatile static boolean fail; @@ -44,11 +43,12 @@ public class FileDescriptorSharing { TestMultipleFD(); TestIsValid(); MultiThreadedFD(); + TestCloseAll(); } /** - * We shouldn't discard a file descriptor until all streams have - * finished with it + * Finalizer shouldn't discard a file descriptor until all streams have + * finished with it. */ private static void TestFinalizer() throws Exception { FileDescriptor fd = null; @@ -96,9 +96,6 @@ public class FileDescriptorSharing { System.out.print("."); ret = fis3.read(); } - if(!fd.valid()) { - throw new RuntimeException("TestFinalizer() : FileDescriptor should be valid"); - } } finally { testFinalizerFile.delete(); } @@ -194,23 +191,19 @@ public class FileDescriptorSharing { } 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"); + throw new RuntimeException("[FIS close()] FileDescriptor shouldn't be valid"); } - } finally { + if (fos != null) fos.close(); if (raf != null) raf.close(); + } finally { test1.delete(); } } /* - * Close out in different order to ensure FD is not - * closed out too early + * Close out in different order to ensure FD is + * closed correctly. */ File test2 = new File("test2"); try { @@ -221,14 +214,11 @@ public class FileDescriptorSharing { } 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"); + throw new RuntimeException("[RAF close()] FileDescriptor shouldn't be valid"); } + if (fos != null) fos.close(); + if (fis != null) fis.close(); } finally { test2.delete(); } @@ -244,14 +234,11 @@ public class FileDescriptorSharing { } 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"); + throw new RuntimeException("[FOS close()] FileDescriptor shouldn't be valid"); } + if (raf != null) raf.close(); + if (fis != null) fis.close(); } finally { test3.delete(); } @@ -259,7 +246,7 @@ public class FileDescriptorSharing { } /** - * Test concurrent access to the same fd.useCount field + * Test concurrent access to the same FileDescriptor */ private static void MultiThreadedFD() throws Exception { RandomAccessFile raf = null; @@ -293,6 +280,68 @@ public class FileDescriptorSharing { } } + /** + * Test closeAll handling in FileDescriptor + */ + private static void TestCloseAll() throws Exception { + File testFile = new File("test"); + testFile.deleteOnExit(); + RandomAccessFile raf = new RandomAccessFile(testFile, "rw"); + FileInputStream fis = new FileInputStream(raf.getFD()); + fis.close(); + if (raf.getFD().valid()) { + throw new RuntimeException("FD should not be valid."); + } + + // Test the suppressed exception handling - FileInputStream + + raf = new RandomAccessFile(testFile, "rw"); + fis = new FileInputStream(raf.getFD()); + BadFileInputStream bfis1 = new BadFileInputStream(raf.getFD()); + BadFileInputStream bfis2 = new BadFileInputStream(raf.getFD()); + BadFileInputStream bfis3 = new BadFileInputStream(raf.getFD()); + // extra test - set bfis3 to null + bfis3 = null; + try { + fis.close(); + } catch (IOException ioe) { + ioe.printStackTrace(); + if (ioe.getSuppressed().length != 2) { + throw new RuntimeException("[FIS]Incorrect number of suppressed " + + "exceptions received : " + ioe.getSuppressed().length); + } + } + if (raf.getFD().valid()) { + // we should still have closed the FD + // even with the exception. + throw new RuntimeException("[FIS]TestCloseAll : FD still valid."); + } + + // Now test with FileOutputStream + + raf = new RandomAccessFile(testFile, "rw"); + FileOutputStream fos = new FileOutputStream(raf.getFD()); + BadFileOutputStream bfos1 = new BadFileOutputStream(raf.getFD()); + BadFileOutputStream bfos2 = new BadFileOutputStream(raf.getFD()); + BadFileOutputStream bfos3 = new BadFileOutputStream(raf.getFD()); + // extra test - set bfos3 to null + bfos3 = null; + try { + fos.close(); + } catch (IOException ioe) { + ioe.printStackTrace(); + if (ioe.getSuppressed().length != 2) { + throw new RuntimeException("[FOS]Incorrect number of suppressed " + + "exceptions received : " + ioe.getSuppressed().length); + } + } + if (raf.getFD().valid()) { + // we should still have closed the FD + // even with the exception. + throw new RuntimeException("[FOS]TestCloseAll : FD still valid."); + } + } + /** * A thread which will open and close a number of FileInputStreams and * FileOutputStreams referencing the same native file descriptor. @@ -325,12 +374,35 @@ public class FileDescriptorSharing { 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"); + if (fd.valid()) { // fd should not be valid after first close() call + System.out.println("OpenClose: FileDescriptor shouldn't be valid"); fail = true; } done.countDown(); } } } + + private static class BadFileInputStream extends FileInputStream { + + BadFileInputStream(FileDescriptor fd) { + super(fd); + } + + public void close() throws IOException { + throw new IOException("Bad close operation"); + } + } + + private static class BadFileOutputStream extends FileOutputStream { + + BadFileOutputStream(FileDescriptor fd) { + super(fd); + } + + public void close() throws IOException { + throw new IOException("Bad close operation"); + } + } + }