8027351: (ref) Private finalize method invoked in preference to protected superclass method

Reviewed-by: alanb, dholmes, mr, plevart, psandoz

jdk/makefiles/lib/CoreLibraries.gmk

@@ -115,7 +115,6 @@ BUILD_LIBRARIES += $(BUILD_LIBVERIFY)
 
 LIBJAVA_SRC_DIRS := $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS_API_DIR)/native/java/lang \
     $(JDK_TOPDIR)/src/share/native/java/lang \
-    $(JDK_TOPDIR)/src/share/native/java/lang/ref \
     $(JDK_TOPDIR)/src/share/native/java/lang/reflect \
     $(JDK_TOPDIR)/src/share/native/java/io \
     $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS_API_DIR)/native/java/io \

jdk/makefiles/mapfiles/libjava/mapfile-vers

@@ -140,7 +140,6 @@ SUNWprivate_1.1 {
 		Java_java_lang_Double_doubleToRawLongBits;
 		Java_java_lang_reflect_Proxy_defineClass0;
 		Java_java_lang_Shutdown_runAllFinalizers;
-		Java_java_lang_ref_Finalizer_invokeFinalizeMethod;
 		Java_java_lang_Float_intBitsToFloat;
 		Java_java_lang_Float_floatToRawIntBits;
 		Java_java_lang_StrictMath_IEEEremainder;

jdk/makefiles/mapfiles/libjava/reorder-sparc

@@ -88,7 +88,6 @@ text: .text%Java_java_lang_Throwable_getStackTraceElement;
 text: .text%throwFileNotFoundException;
 text: .text%JNU_NotifyAll;
 # Test LoadFrame
-text: .text%Java_java_lang_ref_Finalizer_invokeFinalizeMethod;
 text: .text%JNU_CallMethodByName;
 text: .text%JNU_CallMethodByNameV;
 text: .text%Java_java_io_UnixFileSystem_createDirectory;

jdk/makefiles/mapfiles/libjava/reorder-sparcv9

@@ -78,7 +78,6 @@ text: .text%writeBytes;
 text: .text%Java_java_security_AccessController_doPrivileged__Ljava_security_PrivilegedAction_2Ljava_security_AccessControlContext_2;
 text: .text%JNU_GetEnv;
 text: .text%Java_java_io_UnixFileSystem_checkAccess;
-text: .text%Java_java_lang_ref_Finalizer_invokeFinalizeMethod;
 text: .text%Java_java_lang_reflect_Array_newArray;
 text: .text%Java_java_lang_Throwable_getStackTraceDepth;
 text: .text%Java_java_lang_Throwable_getStackTraceElement;

jdk/makefiles/mapfiles/libjava/reorder-x86

@@ -78,7 +78,6 @@ text: .text%Java_java_security_AccessController_doPrivileged__Ljava_security_Pri
 text: .text%JNU_GetEnv;
 text: .text%Java_java_io_UnixFileSystem_checkAccess;
 text: .text%Java_sun_reflect_NativeMethodAccessorImpl_invoke0;
-text: .text%Java_java_lang_ref_Finalizer_invokeFinalizeMethod;
 text: .text%Java_java_io_FileInputStream_available;
 text: .text%Java_java_lang_reflect_Array_newArray;
 text: .text%Java_java_lang_Throwable_getStackTraceDepth;

jdk/src/share/classes/java/lang/System.java

@@ -1263,6 +1263,9 @@ public final class System {
             public Thread newThreadWithAcc(Runnable target, AccessControlContext acc) {
                 return new Thread(target, acc);
             }
+            public void invokeFinalize(Object o) throws Throwable {
+                o.finalize();
+            }
         });
     }
 }

jdk/src/share/classes/java/lang/ref/Finalizer.java

@@ -27,17 +27,14 @@ package java.lang.ref;
 
 import java.security.PrivilegedAction;
 import java.security.AccessController;
-
+import sun.misc.JavaLangAccess;
+import sun.misc.SharedSecrets;
+import sun.misc.VM;
 
 final class Finalizer extends FinalReference<Object> { /* Package-private; must be in
                                                           same package as the Reference
                                                           class */
 
-    /* A native method that invokes an arbitrary object's finalize method is
-       required since the finalize method is protected
-     */
-    static native void invokeFinalizeMethod(Object o) throws Throwable;
-
     private static ReferenceQueue<Object> queue = new ReferenceQueue<>();
     private static Finalizer unfinalized = null;
     private static final Object lock = new Object();
@@ -90,7 +87,7 @@ final class Finalizer extends FinalReference<Object> { /* Package-private; must
         new Finalizer(finalizee);
     }
 
-    private void runFinalizer() {
+    private void runFinalizer(JavaLangAccess jla) {
         synchronized (this) {
             if (hasBeenFinalized()) return;
             remove();
@@ -98,7 +95,8 @@ final class Finalizer extends FinalReference<Object> { /* Package-private; must
         try {
             Object finalizee = this.get();
             if (finalizee != null && !(finalizee instanceof java.lang.Enum)) {
-                invokeFinalizeMethod(finalizee);
+                jla.invokeFinalize(finalizee);
+
                 /* Clear stack slot containing this variable, to decrease
                    the chances of false retention with a conservative GC */
                 finalizee = null;
@@ -141,16 +139,21 @@ final class Finalizer extends FinalReference<Object> { /* Package-private; must
 
     /* Called by Runtime.runFinalization() */
     static void runFinalization() {
+        if (!VM.isBooted()) {
+            return;
+        }
+
         forkSecondaryFinalizer(new Runnable() {
             private volatile boolean running;
             public void run() {
                 if (running)
                     return;
+                final JavaLangAccess jla = SharedSecrets.getJavaLangAccess();
                 running = true;
                 for (;;) {
                     Finalizer f = (Finalizer)queue.poll();
                     if (f == null) break;
-                    f.runFinalizer();
+                    f.runFinalizer(jla);
                 }
             }
         });
@@ -158,11 +161,16 @@ final class Finalizer extends FinalReference<Object> { /* Package-private; must
 
     /* Invoked by java.lang.Shutdown */
     static void runAllFinalizers() {
+        if (!VM.isBooted()) {
+            return;
+        }
+
         forkSecondaryFinalizer(new Runnable() {
             private volatile boolean running;
             public void run() {
                 if (running)
                     return;
+                final JavaLangAccess jla = SharedSecrets.getJavaLangAccess();
                 running = true;
                 for (;;) {
                     Finalizer f;
@@ -171,7 +179,7 @@ final class Finalizer extends FinalReference<Object> { /* Package-private; must
                         if (f == null) break;
                         unfinalized = f.next;
                     }
-                    f.runFinalizer();
+                    f.runFinalizer(jla);
                 }}});
     }
 
@@ -183,13 +191,25 @@ final class Finalizer extends FinalReference<Object> { /* Package-private; must
         public void run() {
             if (running)
                 return;
+
+            // Finalizer thread starts before System.initializeSystemClass
+            // is called.  Wait until JavaLangAccess is available
+            while (!VM.isBooted()) {
+                // delay until VM completes initialization
+                try {
+                    VM.awaitBooted();
+                } catch (InterruptedException x) {
+                    // ignore and continue
+                }
+            }
+            final JavaLangAccess jla = SharedSecrets.getJavaLangAccess();
             running = true;
             for (;;) {
                 try {
                     Finalizer f = (Finalizer)queue.remove();
-                    f.runFinalizer();
+                    f.runFinalizer(jla);
                 } catch (InterruptedException x) {
-                    continue;
+                    // ignore and continue
                 }
             }
         }

jdk/src/share/classes/sun/misc/JavaLangAccess.java

@@ -127,4 +127,9 @@ public interface JavaLangAccess {
      * inherited AccessControlContext.
      */
     Thread newThreadWithAcc(Runnable target, AccessControlContext acc);
+
+    /**
+     * Invokes the finalize method of the given object.
+     */
+    void invokeFinalize(Object o) throws Throwable;
 }

jdk/src/share/classes/sun/misc/VM.java

@@ -148,6 +148,7 @@ public class VM {
 
 
     private static volatile boolean booted = false;
+    private static final Object lock = new Object();
 
     // Invoked by by System.initializeSystemClass just before returning.
     // Subsystems that are invoked during initialization can check this
@@ -155,13 +156,27 @@ public class VM {
     // application class loader has been set up.
     //
     public static void booted() {
+        synchronized (lock) {
             booted = true;
+            lock.notifyAll();
+        }
     }
 
     public static boolean isBooted() {
         return booted;
     }
 
+    // Waits until VM completes initialization
+    //
+    // This method is invoked by the Finalizer thread
+    public static void awaitBooted() throws InterruptedException {
+        synchronized (lock) {
+            while (!booted) {
+                lock.wait();
+            }
+        }
+    }
+
     // A user-settable upper limit on the maximum amount of allocatable direct
     // buffer memory.  This value may be changed during VM initialization if
     // "java" is launched with "-XX:MaxDirectMemorySize=<size>".

jdk/test/java/lang/ref/FinalizeOverride.java

@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2013, 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.
+ */
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/* @test
+ * @bug 8027351
+ * @summary Basic test of the finalize method
+ */
+
+public class FinalizeOverride {
+    // finalizedCount is incremented when the finalize method is invoked
+    private static AtomicInteger finalizedCount = new AtomicInteger();
+
+    // finalizedSum and privateFinalizedInvoke are used to verify
+    // the right overrided finalize method is invoked
+    private static AtomicInteger finalizedSum = new AtomicInteger();
+    private static volatile boolean privateFinalizeInvoked = false;
+
+    public static void main(String[] argvs) throws IOException {
+        patchPrivateFinalize();
+
+        test(new Base(10), 10);
+        test(new Subclass(20), 0);
+        test(new SubSubclass(30), 30);
+        test(new PublicFinalize(40), 40*100+40);
+        test(new PrivateFinalize(50), 50);
+        test(new NoOverride(60), 60);
+    }
+
+    static void test(Object o, int expected) {
+        int count = finalizedCount.get();
+        int sum = finalizedSum.get();
+        privateFinalizeInvoked = false;
+
+        // force GC and finalization
+        o = null;
+        while (finalizedCount.get() != (count+1)) {
+            System.gc();
+            System.runFinalization();
+        }
+
+        if (privateFinalizeInvoked) {
+            throw new RuntimeException("private finalize method invoked");
+        }
+        if (finalizedCount.get() != (count+1)) {
+            throw new RuntimeException("Unexpected count=" + finalizedCount +
+                " expected=" + (count+1));
+        }
+        if (finalizedSum.get() != (sum+expected)) {
+            throw new RuntimeException("Unexpected sum=" + finalizedSum +
+                " prev=" + sum + " value=" + expected);
+        }
+    }
+
+    static void patchPrivateFinalize() throws IOException {
+        // patch the private f_nal_ze method name to "finalize"
+        String testClasses = System.getProperty("test.classes", ".");
+        Path p = Paths.get(testClasses, "FinalizeOverride$PrivateFinalize.class");
+        byte[] bytes = Files.readAllBytes(p);
+        int len = "f_nal_ze".length();
+        for (int i=0; i < bytes.length-len; i++) {
+            if (bytes[i] == 'f' &&
+                bytes[i+1] == '_' &&
+                bytes[i+2] == 'n' &&
+                bytes[i+3] == 'a' &&
+                bytes[i+4] == 'l' &&
+                bytes[i+5] == '_' &&
+                bytes[i+6] == 'z' &&
+                bytes[i+7] == 'e')
+            {
+                // s%_%i%
+                bytes[i+1] = 'i';
+                bytes[i+5] = 'i';
+                break;
+            }
+        }
+        Files.write(p, bytes);
+    }
+
+    static class Base {
+        protected int value;
+        Base(int v) {
+            this.value = v;
+        }
+        int called() {
+            finalizedSum.addAndGet(value);
+            return value;
+        }
+        protected void finalize() {
+            System.out.println("Base.finalize() sum += " + called());
+            finalizedCount.incrementAndGet();
+        }
+    }
+    static class PublicFinalize extends Base {
+        PublicFinalize(int v) {
+            super(v);
+        }
+        public void finalize() {
+            finalizedSum.addAndGet(value * 100);
+            System.out.println("PublicFinalize.finalize() sum += " + called() +
+                "+"+value+"*100");
+            finalizedCount.incrementAndGet();
+        }
+    }
+    static class Subclass extends Base {
+        Subclass(int v) {
+            super(v);
+        }
+        protected void finalize() {
+            // no value added to sum
+            System.out.println("Subclass.finalize() sum += 0");
+            finalizedCount.incrementAndGet();
+        }
+    }
+    static class SubSubclass extends Subclass {
+        SubSubclass(int v) {
+            super(v);
+        }
+        protected final void finalize() {
+            finalizedSum.addAndGet(value);
+            System.out.println("SubSubclass.finalize() sum +=" +value);
+            finalizedCount.incrementAndGet();
+        }
+    }
+    static class PrivateFinalize extends Base {
+        PrivateFinalize(int v) {
+            super(v);
+        }
+        private void f_nal_ze() {
+            // finalization catches any exception
+            System.out.println("Error: private finalize invoked!!");
+            privateFinalizeInvoked = true;
+            finalizedCount.incrementAndGet();
+        }
+    }
+    static class NoOverride extends PrivateFinalize {
+        NoOverride(int v) {
+            super(v);
+        }
+    }
+}