8192939: Remove Finalize methods from FileInputStream and FileOutputStream

Reviewed-by: alanb, iris, mchung

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 2018, 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
@@ -79,8 +79,6 @@ class FileInputStream extends InputStream
 
     private volatile boolean closed;
 
-    private final Object altFinalizer;
-
     /**
      * Creates a <code>FileInputStream</code> by
      * opening a connection to an actual file,
@@ -155,11 +153,8 @@ class FileInputStream extends InputStream
         fd.attach(this);
         path = name;
         open(name);
-        altFinalizer = getFinalizer(this);
-        if (altFinalizer == null) {
         FileCleanable.register(fd);       // open set the fd, register the cleanup
     }
-    }
 
     /**
      * Creates a <code>FileInputStream</code> by using the file descriptor
@@ -195,7 +190,6 @@ class FileInputStream extends InputStream
         }
         fd = fdObj;
         path = null;
-        altFinalizer = null;
 
         /*
          * FileDescriptor is being shared by streams.
@@ -438,85 +432,4 @@ class FileInputStream extends InputStream
     static {
         initIDs();
     }
-
-    /**
-     * Ensures that the {@link #close} method of this file input stream is
-     * called when there are no more references to it.
-     * The {@link #finalize} method does not call {@link #close} directly.
-     *
-     * @apiNote
-     * To release resources used by this stream {@link #close} should be called
-     * directly or by try-with-resources.
-     *
-     * @implSpec
-     * If this FileInputStream has been subclassed and the {@link #close}
-     * method has been overridden, the {@link #close} method will be
-     * called when the FileInputStream is unreachable.
-     * Otherwise, it is implementation specific how the resource cleanup described in
-     * {@link #close} is performed.
-     *
-     * @deprecated The {@code finalize} method has been deprecated and will be removed.
-     *     Subclasses that override {@code finalize} in order to perform cleanup
-     *     should be modified to use alternative cleanup mechanisms and
-     *     to remove the overriding {@code finalize} method.
-     *     When overriding the {@code finalize} method, its implementation must explicitly
-     *     ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}.
-     *     See the specification for {@link Object#finalize()} for further
-     *     information about migration options.
-     *
-     * @exception  IOException  if an I/O error occurs.
-     * @see        java.io.FileInputStream#close()
-     */
-    @Deprecated(since="9", forRemoval = true)
-    protected void finalize() throws IOException {
-    }
-
-    /*
-     * Returns a finalizer object if the FIS needs a finalizer; otherwise null.
-     * If the FIS has a close method; it needs an AltFinalizer.
-     */
-    private static Object getFinalizer(FileInputStream fis) {
-        Class<?> clazz = fis.getClass();
-        while (clazz != FileInputStream.class) {
-            try {
-                clazz.getDeclaredMethod("close");
-                return new AltFinalizer(fis);
-            } catch (NoSuchMethodException nsme) {
-                // ignore
-            }
-            clazz = clazz.getSuperclass();
-        }
-        return null;
-    }
-    /**
-     * Class to call {@code FileInputStream.close} when finalized.
-     * If finalization of the stream is needed, an instance is created
-     * in its constructor(s).  When the set of instances
-     * related to the stream is unreachable, the AltFinalizer performs
-     * the needed call to the stream's {@code close} method.
-     */
-    static class AltFinalizer {
-        private final FileInputStream fis;
-
-        AltFinalizer(FileInputStream fis) {
-            this.fis = fis;
-        }
-
-        @Override
-        @SuppressWarnings("deprecation")
-        protected final void finalize() {
-            try {
-                if ((fis.fd != null) && (fis.fd != FileDescriptor.in)) {
-                    /* 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()
-                     */
-                    fis.close();
-                }
-            } catch (IOException ioe) {
-                // ignore
-            }
-        }
-    }
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 2018, 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
@@ -95,8 +95,6 @@ class FileOutputStream extends OutputStream
 
     private volatile boolean closed;
 
-    private final Object altFinalizer;
-
     /**
      * Creates a file output stream to write to the file with the
      * specified name. A new <code>FileDescriptor</code> object is
@@ -235,11 +233,8 @@ class FileOutputStream extends OutputStream
         this.path = name;
 
         open(name, append);
-        altFinalizer = getFinalizer(this);
-        if (altFinalizer == null) {
         FileCleanable.register(fd);   // open sets the fd, register the cleanup
     }
-    }
 
     /**
      * Creates a file output stream to write to the specified file
@@ -274,7 +269,6 @@ class FileOutputStream extends OutputStream
         }
         this.fd = fdObj;
         this.path = null;
-        this.altFinalizer = null;
 
         fd.attach(this);
     }
@@ -457,98 +451,9 @@ class FileOutputStream extends OutputStream
         return fc;
     }
 
-    /**
-     * Cleans up the connection to the file, and ensures that the
-     * {@link #close} method of this file output stream is
-     * called when there are no more references to this stream.
-     * The {@link #finalize} method does not call {@link #close} directly.
-     *
-     * @apiNote
-     * To release resources used by this stream {@link #close} should be called
-     * directly or by try-with-resources.
-     *
-     * @implSpec
-     * If this FileOutputStream has been subclassed and the {@link #close}
-     * method has been overridden, the {@link #close} method will be
-     * called when the FileOutputStream is unreachable.
-     * Otherwise, it is implementation specific how the resource cleanup described in
-     * {@link #close} is performed.
-     *
-     * @deprecated The {@code finalize} method has been deprecated and will be removed.
-     *     Subclasses that override {@code finalize} in order to perform cleanup
-     *     should be modified to use alternative cleanup mechanisms and
-     *     to remove the overriding {@code finalize} method.
-     *     When overriding the {@code finalize} method, its implementation must explicitly
-     *     ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}.
-     *     See the specification for {@link Object#finalize()} for further
-     *     information about migration options.
-     *
-     * @exception  IOException  if an I/O error occurs.
-     * @see        java.io.FileInputStream#close()
-     */
-    @Deprecated(since="9", forRemoval = true)
-    protected void finalize() throws IOException {
-    }
-
     private static native void initIDs();
 
     static {
         initIDs();
     }
-
-    /*
-     * Returns a finalizer object if the FOS needs a finalizer; otherwise null.
-     * If the FOS has a close method; it needs an AltFinalizer.
-     */
-    private static Object getFinalizer(FileOutputStream fos) {
-        Class<?> clazz = fos.getClass();
-        while (clazz != FileOutputStream.class) {
-            try {
-                clazz.getDeclaredMethod("close");
-                return new AltFinalizer(fos);
-            } catch (NoSuchMethodException nsme) {
-                // ignore
-            }
-            clazz = clazz.getSuperclass();
-        }
-        return null;
-    }
-
-    /**
-     * Class to call {@code FileOutputStream.close} when finalized.
-     * If finalization of the stream is needed, an instance is created
-     * in its constructor(s).  When the set of instances
-     * related to the stream is unreachable, the AltFinalizer performs
-     * the needed call to the stream's {@code close} method.
-     */
-    static class AltFinalizer {
-        private final FileOutputStream fos;
-
-        AltFinalizer(FileOutputStream fos) {
-            this.fos = fos;
-        }
-
-        @Override
-        @SuppressWarnings("deprecation")
-        protected final void finalize() {
-            try {
-                if (fos.fd != null) {
-                    if (fos.fd == FileDescriptor.out || fos.fd == FileDescriptor.err) {
-                        // Subclass may override flush; otherwise it is no-op
-                        fos.flush();
-                    } else {
-                        /* 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()
-                         */
-                        fos.close();
-                    }
-                }
-            } catch (IOException ioe) {
-                // ignore
-            }
-        }
-    }
-
 }

test/jdk/java/io/FileInputStream/UnreferencedFISClosesFd.java

@@ -62,15 +62,11 @@ import jdk.test.lib.util.FileUtils;
  */
 public class UnreferencedFISClosesFd {
 
-    enum CleanupType {
-        CLOSE,      // Cleanup is handled via calling close
-        CLEANER}    // Cleanup is handled via Cleaner
-
     static final String FILE_NAME = "empty.txt";
 
     /**
      * Subclass w/ no overrides; not finalize or close.
-     * Cleanup should be via the Cleaner (not close).
+     * Cleanup should be via the Cleaner.
      */
     public static class StreamOverrides extends FileInputStream {
 
@@ -88,7 +84,7 @@ public class UnreferencedFISClosesFd {
 
     /**
      * Subclass overrides close.
-     * Cleanup should be via AltFinalizer calling close().
+     * Cleanup should be via the Cleaner.
      */
     public static class StreamOverridesClose extends StreamOverrides {
 
@@ -104,7 +100,7 @@ public class UnreferencedFISClosesFd {
 
     /**
      * Subclass overrides finalize.
-     * Cleanup should be via the Cleaner (not close).
+     * Cleanup should be via the Cleaner.
      */
     public static class StreamOverridesFinalize extends StreamOverrides {
 
@@ -113,7 +109,7 @@ public class UnreferencedFISClosesFd {
         }
 
         @SuppressWarnings({"deprecation","removal"})
-        protected void finalize() throws IOException {
+        protected void finalize() throws IOException, Throwable {
             super.finalize();
         }
     }
@@ -129,7 +125,7 @@ public class UnreferencedFISClosesFd {
         }
 
         @SuppressWarnings({"deprecation","removal"})
-        protected void finalize() throws IOException {
+        protected void finalize() throws IOException, Throwable {
             super.finalize();
         }
     }
@@ -149,15 +145,15 @@ public class UnreferencedFISClosesFd {
         long fdCount0 = getFdCount();
 
         int failCount = 0;
-        failCount += test(new FileInputStream(name), CleanupType.CLEANER);
+        failCount += test(new FileInputStream(name));
 
-        failCount += test(new StreamOverrides(name), CleanupType.CLEANER);
+        failCount += test(new StreamOverrides(name));
 
-        failCount += test(new StreamOverridesClose(name), CleanupType.CLOSE);
+        failCount += test(new StreamOverridesClose(name));
 
-        failCount += test(new StreamOverridesFinalize(name), CleanupType.CLEANER);
+        failCount += test(new StreamOverridesFinalize(name));
 
-        failCount += test(new StreamOverridesFinalizeClose(name), CleanupType.CLOSE);
+        failCount += test(new StreamOverridesFinalizeClose(name));
 
         if (failCount > 0) {
             throw new AssertionError("Failed test count: " + failCount);
@@ -180,7 +176,7 @@ public class UnreferencedFISClosesFd {
                 : -1L;
     }
 
-    private static int test(FileInputStream fis, CleanupType cleanType) throws Exception {
+    private static int test(FileInputStream fis) throws Exception {
 
         try {
             System.out.printf("%nTesting %s%n", fis.getClass().getName());
@@ -199,35 +195,18 @@ public class UnreferencedFISClosesFd {
             fdField.setAccessible(true);
             int ffd = fdField.getInt(fd);
 
-            Field altFinalizerField = FileInputStream.class.getDeclaredField("altFinalizer");
-            altFinalizerField.setAccessible(true);
-            Object altFinalizer = altFinalizerField.get(fis);
-            if ((altFinalizer != null) ^ (cleanType == CleanupType.CLOSE)) {
-                throw new RuntimeException("Unexpected AltFinalizer: " + altFinalizer
-                + ", for " + cleanType);
-            }
-
             Field cleanupField = FileDescriptor.class.getDeclaredField("cleanup");
             cleanupField.setAccessible(true);
             Object cleanup = cleanupField.get(fd);
-            System.out.printf("  cleanup: %s, alt: %s, ffd: %d, cf: %s%n",
+            System.out.printf("  cleanup: %s, ffd: %d, cf: %s%n", cleanup, ffd, cleanupField);
-                    cleanup, altFinalizer, ffd, cleanupField);
+            if (cleanup == null) {
-            if ((cleanup != null) ^ (cleanType == CleanupType.CLEANER)) {
+                throw new RuntimeException("cleanup should not be null");
-                throw new Exception("unexpected cleanup: "
-                + cleanup + ", for " + cleanType);
             }
-            if (cleanup != null) {
+
             WeakReference<Object> cleanupWeak = new WeakReference<>(cleanup, queue);
             pending.add(cleanupWeak);
             System.out.printf("    fdWeak: %s%n    msWeak: %s%n    cleanupWeak: %s%n",
                     fdWeak, msWeak, cleanupWeak);
-            }
-            if (altFinalizer != null) {
-                WeakReference<Object> altFinalizerWeak = new WeakReference<>(altFinalizer, queue);
-                pending.add(altFinalizerWeak);
-                System.out.printf("    fdWeak: %s%n    msWeak: %s%n    altFinalizerWeak: %s%n",
-                        fdWeak, msWeak, altFinalizerWeak);
-            }
 
             AtomicInteger closeCounter = fis instanceof StreamOverrides
                     ? ((StreamOverrides)fis).closeCounter() : null;
@@ -243,28 +222,16 @@ public class UnreferencedFISClosesFd {
                     fis = null;
                     fd = null;
                     cleanup = null;
-                    altFinalizer = null;
                     System.gc();  // attempt to reclaim them
                 }
             }
             Reference.reachabilityFence(fd);
             Reference.reachabilityFence(fis);
             Reference.reachabilityFence(cleanup);
-            Reference.reachabilityFence(altFinalizer);
 
             // Confirm the correct number of calls to close depending on the cleanup type
-            switch (cleanType) {
-                case CLEANER:
             if (closeCounter != null && closeCounter.get() > 0) {
-                        throw new RuntimeException("Close should not have been called: count: "
+                throw new RuntimeException("Close should not have been called: count: " + closeCounter);
-                                + closeCounter);
-                    }
-                    break;
-                case CLOSE:
-                    if (closeCounter == null || closeCounter.get() == 0) {
-                        throw new RuntimeException("Close should have been called: count: 0");
-                    }
-                    break;
             }
         } catch (Exception ex) {
             ex.printStackTrace(System.out);

test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java

@@ -28,8 +28,7 @@
  * @library /test/lib
  * @build jdk.test.lib.util.FileUtils UnreferencedFOSClosesFd
  * @bug 6524062
- * @summary Test to ensure that FOS.finalize() invokes the close() method as per
+ * @summary Test to ensure that the fd is closed if left unreferenced
- * the specification.
  * @run main/othervm UnreferencedFOSClosesFd
  */
 import java.io.File;
@@ -54,15 +53,11 @@ import jdk.test.lib.util.FileUtils;
 
 public class UnreferencedFOSClosesFd {
 
-    enum CleanupType {
-        CLOSE,      // Cleanup is handled via calling close
-        CLEANER}    // Cleanup is handled via Cleaner
-
     static final String FILE_NAME = "empty.txt";
 
     /**
-     * Subclass w/ no overrides; not finalize or close.
+     * Subclass w/ no overrides; not close.
-     * Cleanup should be via the Cleaner (not close).
+     * Cleanup should be via the Cleaner.
      */
     public static class StreamOverrides extends FileOutputStream {
 
@@ -96,7 +91,7 @@ public class UnreferencedFOSClosesFd {
 
     /**
      * Subclass overrides finalize and close.
-     * Cleanup should be via the Cleaner (not close).
+     * Cleanup should be via the Cleaner.
      */
     public static class StreamOverridesFinalize extends StreamOverrides {
 
@@ -105,14 +100,14 @@ public class UnreferencedFOSClosesFd {
         }
 
         @SuppressWarnings({"deprecation","removal"})
-        protected void finalize() throws IOException {
+        protected void finalize() throws IOException, Throwable {
             super.finalize();
         }
     }
 
     /**
      * Subclass overrides finalize and close.
-     * Cleanup should be via AltFinalizer calling close().
+     * Cleanup should be via the Cleaner.
      */
     public static class StreamOverridesFinalizeClose extends StreamOverridesClose {
 
@@ -121,7 +116,7 @@ public class UnreferencedFOSClosesFd {
         }
 
         @SuppressWarnings({"deprecation","removal"})
-        protected void finalize() throws IOException {
+        protected void finalize() throws IOException, Throwable {
             super.finalize();
         }
     }
@@ -131,8 +126,6 @@ public class UnreferencedFOSClosesFd {
      */
     public static void main(String argv[]) throws Exception {
 
-
-
         File inFile = new File(System.getProperty("test.dir", "."), FILE_NAME);
         inFile.createNewFile();
         inFile.deleteOnExit();
@@ -143,15 +136,15 @@ public class UnreferencedFOSClosesFd {
         long fdCount0 = getFdCount();
 
         int failCount = 0;
-        failCount += test(new FileOutputStream(name), CleanupType.CLEANER);
+        failCount += test(new FileOutputStream(name));
 
-        failCount += test(new StreamOverrides(name), CleanupType.CLEANER);
+        failCount += test(new StreamOverrides(name));
 
-        failCount += test(new StreamOverridesClose(name), CleanupType.CLOSE);
+        failCount += test(new StreamOverridesClose(name));
 
-        failCount += test(new StreamOverridesFinalize(name), CleanupType.CLEANER);
+        failCount += test(new StreamOverridesFinalize(name));
 
-        failCount += test(new StreamOverridesFinalizeClose(name), CleanupType.CLOSE);
+        failCount += test(new StreamOverridesFinalizeClose(name));
 
         if (failCount > 0) {
             throw new AssertionError("Failed test count: " + failCount);
@@ -174,7 +167,7 @@ public class UnreferencedFOSClosesFd {
                 : -1L;
     }
 
-    private static int test(FileOutputStream fos, CleanupType cleanType) throws Exception {
+    private static int test(FileOutputStream fos) throws Exception {
 
         try {
             System.out.printf("%nTesting %s%n", fos.getClass().getName());
@@ -193,35 +186,18 @@ public class UnreferencedFOSClosesFd {
             fdField.setAccessible(true);
             int ffd = fdField.getInt(fd);
 
-            Field altFinalizerField = FileOutputStream.class.getDeclaredField("altFinalizer");
-            altFinalizerField.setAccessible(true);
-            Object altFinalizer = altFinalizerField.get(fos);
-            if ((altFinalizer != null) ^ (cleanType == CleanupType.CLOSE)) {
-                throw new RuntimeException("Unexpected AltFinalizer: " + altFinalizer
-                        + ", for " + cleanType);
-            }
-
             Field cleanupField = FileDescriptor.class.getDeclaredField("cleanup");
             cleanupField.setAccessible(true);
             Object cleanup = cleanupField.get(fd);
-            System.out.printf("  cleanup: %s, alt: %s, ffd: %d, cf: %s%n",
+            System.out.printf("  cleanup: %s, ffd: %d, cf: %s%n", cleanup, ffd, cleanupField);
-                    cleanup, altFinalizer, ffd, cleanupField);
+            if (cleanup == null) {
-            if ((cleanup != null) ^ (cleanType == CleanupType.CLEANER)) {
+                throw new RuntimeException("cleanup should not be null");
-                throw new Exception("unexpected cleanup: "
-                        + cleanup + ", for " + cleanType);
             }
-            if (cleanup != null) {
+
             WeakReference<Object> cleanupWeak = new WeakReference<>(cleanup, queue);
             pending.add(cleanupWeak);
             System.out.printf("    fdWeak: %s%n    msWeak: %s%n    cleanupWeak: %s%n",
                     fdWeak, msWeak, cleanupWeak);
-            }
-            if (altFinalizer != null) {
-                WeakReference<Object> altFinalizerWeak = new WeakReference<>(altFinalizer, queue);
-                pending.add(altFinalizerWeak);
-                System.out.printf("    fdWeak: %s%n    msWeak: %s%n    altFinalizerWeak: %s%n",
-                        fdWeak, msWeak, altFinalizerWeak);
-            }
 
             AtomicInteger closeCounter = fos instanceof StreamOverrides
                     ? ((StreamOverrides) fos).closeCounter() : null;
@@ -237,28 +213,16 @@ public class UnreferencedFOSClosesFd {
                     fos = null;
                     fd = null;
                     cleanup = null;
-                    altFinalizer = null;
                     System.gc();  // attempt to reclaim them
                 }
             }
             Reference.reachabilityFence(fd);
             Reference.reachabilityFence(fos);
             Reference.reachabilityFence(cleanup);
-            Reference.reachabilityFence(altFinalizer);
 
             // Confirm the correct number of calls to close depending on the cleanup type
-            switch (cleanType) {
-                case CLEANER:
             if (closeCounter != null && closeCounter.get() > 0) {
-                        throw new RuntimeException("Close should not have been called: count: "
+                throw new RuntimeException("Close should not have been called: count: " + closeCounter);
-                                + closeCounter);
-                    }
-                    break;
-                case CLOSE:
-                    if (closeCounter == null || closeCounter.get() == 0) {
-                        throw new RuntimeException("Close should have been called: count: 0");
-                    }
-                    break;
             }
         } catch (Exception ex) {
             ex.printStackTrace(System.out);