8163969: Cyclic interface initialization causes JVM crash
authorcoleenp
Wed, 21 Sep 2016 09:56:18 -0400
changeset 41293 871b2f487dc0
parent 41291 e9a1638b8cea
child 41294 61b129b989f3
8163969: Cyclic interface initialization causes JVM crash Summary: Fix interface initialization to follow spec: interface initializations do not set initialization state of interfaces that extend them. Reviewed-by: dholmes, acorn, lfoltan
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/test/runtime/lambda-features/CyclicInterfaceInit.java
hotspot/test/runtime/lambda-features/InterfaceInitializationStates.java
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Wed Sep 21 11:31:23 2016 +0200
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Wed Sep 21 09:56:18 2016 -0400
@@ -517,7 +517,11 @@
 
 bool InstanceKlass::link_class_impl(
     instanceKlassHandle this_k, bool throw_verifyerror, TRAPS) {
-  // check for error state
+  // check for error state.
+  // This is checking for the wrong state.  If the state is initialization_error,
+  // then this class *was* linked.  The CDS code does a try_link_class and uses
+  // initialization_error to mark classes to not include in the archive during
+  // DumpSharedSpaces.  This should be removed when the CDS bug is fixed.
   if (this_k->is_in_error_state()) {
     ResourceMark rm(THREAD);
     THROW_MSG_(vmSymbols::java_lang_NoClassDefFoundError(),
@@ -670,36 +674,21 @@
 
 // Eagerly initialize superinterfaces that declare default methods (concrete instance: any access)
 void InstanceKlass::initialize_super_interfaces(instanceKlassHandle this_k, TRAPS) {
-  if (this_k->has_default_methods()) {
-    for (int i = 0; i < this_k->local_interfaces()->length(); ++i) {
-      Klass* iface = this_k->local_interfaces()->at(i);
-      InstanceKlass* ik = InstanceKlass::cast(iface);
-      if (ik->should_be_initialized()) {
-        if (ik->has_default_methods()) {
-          ik->initialize_super_interfaces(ik, THREAD);
-        }
-        // Only initialize() interfaces that "declare" concrete methods.
-        // has_default_methods drives searching superinterfaces since it
-        // means has_default_methods in its superinterface hierarchy
-        if (!HAS_PENDING_EXCEPTION && ik->declares_default_methods()) {
-          ik->initialize(THREAD);
-        }
-        if (HAS_PENDING_EXCEPTION) {
-          Handle e(THREAD, PENDING_EXCEPTION);
-          CLEAR_PENDING_EXCEPTION;
-          {
-            EXCEPTION_MARK;
-            // Locks object, set state, and notify all waiting threads
-            this_k->set_initialization_state_and_notify(
-                initialization_error, THREAD);
-
-            // ignore any exception thrown, superclass initialization error is
-            // thrown below
-            CLEAR_PENDING_EXCEPTION;
-          }
-          THROW_OOP(e());
-        }
-      }
+  assert (this_k->has_default_methods(), "caller should have checked this");
+  for (int i = 0; i < this_k->local_interfaces()->length(); ++i) {
+    Klass* iface = this_k->local_interfaces()->at(i);
+    InstanceKlass* ik = InstanceKlass::cast(iface);
+
+    // Initialization is depth first search ie. we start with top of the inheritance tree
+    // has_default_methods drives searching superinterfaces since it
+    // means has_default_methods in its superinterface hierarchy
+    if (ik->has_default_methods()) {
+      ik->initialize_super_interfaces(ik, CHECK);
+    }
+
+    // Only initialize() interfaces that "declare" concrete methods.
+    if (ik->should_be_initialized() && ik->declares_default_methods()) {
+      ik->initialize(CHECK);
     }
   }
 }
@@ -765,32 +754,36 @@
   }
 
   // Step 7
-  Klass* super_klass = this_k->super();
-  if (super_klass != NULL && !this_k->is_interface() && super_klass->should_be_initialized()) {
-    super_klass->initialize(THREAD);
-
+  // Next, if C is a class rather than an interface, initialize it's super class and super
+  // interfaces.
+  if (!this_k->is_interface()) {
+    Klass* super_klass = this_k->super();
+    if (super_klass != NULL && super_klass->should_be_initialized()) {
+      super_klass->initialize(THREAD);
+    }
+    // If C implements any interfaces that declares a non-abstract, non-static method,
+    // the initialization of C triggers initialization of its super interfaces.
+    // Only need to recurse if has_default_methods which includes declaring and
+    // inheriting default methods
+    if (!HAS_PENDING_EXCEPTION && this_k->has_default_methods()) {
+      this_k->initialize_super_interfaces(this_k, THREAD);
+    }
+
+    // If any exceptions, complete abruptly, throwing the same exception as above.
     if (HAS_PENDING_EXCEPTION) {
       Handle e(THREAD, PENDING_EXCEPTION);
       CLEAR_PENDING_EXCEPTION;
       {
         EXCEPTION_MARK;
-        this_k->set_initialization_state_and_notify(initialization_error, THREAD); // Locks object, set state, and notify all waiting threads
-        CLEAR_PENDING_EXCEPTION;   // ignore any exception thrown, superclass initialization error is thrown below
+        // Locks object, set state, and notify all waiting threads
+        this_k->set_initialization_state_and_notify(initialization_error, THREAD);
+        CLEAR_PENDING_EXCEPTION;
       }
       DTRACE_CLASSINIT_PROBE_WAIT(super__failed, this_k(), -1,wait);
       THROW_OOP(e());
     }
   }
 
-  // If C is an interface that declares a non-abstract, non-static method,
-  // the initialization of a class (not an interface) that implements C directly or
-  // indirectly.
-  // Recursively initialize any superinterfaces that declare default methods
-  // Only need to recurse if has_default_methods which includes declaring and
-  // inheriting default methods
-  if (!this_k->is_interface() && this_k->has_default_methods()) {
-    this_k->initialize_super_interfaces(this_k, CHECK);
-  }
 
   // Step 8
   {
@@ -852,10 +845,15 @@
 
 void InstanceKlass::set_initialization_state_and_notify_impl(instanceKlassHandle this_k, ClassState state, TRAPS) {
   oop init_lock = this_k->init_lock();
-  ObjectLocker ol(init_lock, THREAD, init_lock != NULL);
-  this_k->set_init_state(state);
-  this_k->fence_and_clear_init_lock();
-  ol.notify_all(CHECK);
+  if (init_lock != NULL) {
+    ObjectLocker ol(init_lock, THREAD);
+    this_k->set_init_state(state);
+    this_k->fence_and_clear_init_lock();
+    ol.notify_all(CHECK);
+  } else {
+    assert(init_lock != NULL, "The initialization state should never be set twice");
+    this_k->set_init_state(state);
+  }
 }
 
 // The embedded _implementor field can only record one implementor.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/lambda-features/CyclicInterfaceInit.java	Wed Sep 21 09:56:18 2016 -0400
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2016, 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 8163969
+ * @summary Interface initialization was crashing on this because the wrong class was getting
+ * initialization error.
+ * @run main CyclicInterfaceInit
+ */
+/**
+ * This snippet crashes with
+ * - Java(TM) SE Runtime Environment (8.0_101-b13) (build 1.8.0_101-b13)
+ */
+public class CyclicInterfaceInit {
+
+    interface Base {
+        static final Object CONST = new Target(){}.someMethod();
+
+        default void important() {
+            // Super interfaces with default methods get initialized (JLS 12.4.1)
+        }
+    }
+
+   static boolean out(String c) {
+       System.out.println("initializing " + c);
+       return true;
+    }
+
+    interface Target extends Base {
+        boolean v = CyclicInterfaceInit.out("Target");
+        default Object someMethod() {
+            throw new RuntimeException();
+        }
+        // Target can be fully initialized before initializating Base because Target doesn't
+        // initiate the initialization of Base.
+    }
+
+    static class InnerBad implements Target {}
+
+    public static void main(String[] args) {
+        try {
+          new Target() {};  // Creates inner class that causes initialization of super interfaces
+        } catch (ExceptionInInitializerError e) {
+          System.out.println("ExceptionInInitializerError thrown as expected");
+        }
+        // Try again, InnerBad instantiation should throw NoClassdefFoundError
+        // because Base is marked erroneous due to previous exception during initialization
+        try {
+          InnerBad ig = new InnerBad();
+          throw new RuntimeException("FAILED- initialization of InnerBad should throw NCDFE");
+        } catch (NoClassDefFoundError e) {
+          System.out.println("NoClassDefFoundError thrown as expected");
+        }
+        // Target is already initialized.
+        System.out.println("Target.v is " + Target.v);
+        // shouldn't throw any exceptions.
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/lambda-features/InterfaceInitializationStates.java	Wed Sep 21 09:56:18 2016 -0400
@@ -0,0 +1,202 @@
+/*
+ * Copyright (c) 2016, 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 8163969
+ * @summary Test interface initialization states and when certain interfaces are initialized
+ * in the presence of initialization errors.
+ * @run main InterfaceInitializationStates
+ */
+
+import java.util.List;
+import java.util.Arrays;
+import java.util.ArrayList;
+
+public class InterfaceInitializationStates {
+
+    static List<Class<?>> cInitOrder = new ArrayList<>();
+
+    // K interface with a default method has an initialization error
+    interface K {
+        boolean v = InterfaceInitializationStates.out(K.class);
+        static final Object CONST = InterfaceInitializationStates.someMethod();
+        default int method() { return 2; }
+    }
+
+    // I is initialized when CONST is used, and doesn't trigger initialization of K,
+    // I also doesn't get an initialization error just because K has an initialization error.
+    interface I extends K {
+        boolean v = InterfaceInitializationStates.out(I.class);
+        static final Object CONST = InterfaceInitializationStates.someMethod();
+    }
+
+    // L can be fully initialized even though it extends an interface that has an
+    // initialization error
+    interface L extends K {
+        boolean v = InterfaceInitializationStates.out(L.class);
+        default void lx() {}
+        static void func() {
+            System.out.println("Calling function on interface with bad super interface.");
+        }
+    }
+
+    // Another interface needing initialization.
+    // Initialization of this interface does not occur with ClassLIM because K throws
+    // an initialization error, so the interface initialization is abandoned
+    interface M {
+        boolean v = InterfaceInitializationStates.out(M.class);
+        default void mx() {}
+    }
+
+    static class ClassLIM implements L, I, M {
+        boolean v = InterfaceInitializationStates.out(ClassLIM.class);
+        int callMethodInK() { return method(); }
+        static {
+            // Since interface initialization of K fails, this should never be called
+            System.out.println("Initializing C, but L is still good");
+            L.func();
+        }
+    }
+
+    // Finally initialize M
+    static class ClassM implements M {
+        boolean v = InterfaceInitializationStates.out(ClassM.class);
+    }
+
+    // Iunlinked is testing initialization like interface I, except interface I is linked when
+    // ClassLIM is linked.
+    // Iunlinked is not linked already when K gets an initialization error.  Linking Iunlinked
+    // should succeed and not get NoClassDefFoundError because it does not depend on the
+    // initialization state of K for linking.  There's bug now where it gets this error.
+    // See: https://bugs.openjdk.java.net/browse/JDK-8166203.
+    interface Iunlinked extends K {
+        boolean v = InterfaceInitializationStates.out(Iunlinked.class);
+    }
+
+    // More tests.  What happens if we use K for parameters and return types?
+    // K is a symbolic reference in the constant pool and the initialization error only
+    // matters when it's used.
+    interface Iparams {
+        boolean v = InterfaceInitializationStates.out(Iparams.class);
+        K the_k = null;
+        K m(K k); // abstract
+        default K method() { return new K(){}; }
+    }
+
+    static class ClassIparams implements Iparams {
+        boolean v = InterfaceInitializationStates.out(ClassIparams.class);
+        public K m(K k) { return k; }
+    }
+
+    public static void main(java.lang.String[] unused) {
+        // The rule this tests is the last sentence of JLS 12.4.1:
+
+        // When a class is initialized, its superclasses are initialized (if they have not
+        // been previously initialized), as well as any superinterfaces (s8.1.5) that declare any
+        // default methods (s9.4.3) (if they have not been previously initialized). Initialization
+        // of an interface does not, of itself, cause initialization of any of its superinterfaces.
+
+        // Trigger initialization.
+        // Now L is fully_initialized even though K should
+        // throw an error during initialization.
+        boolean v = L.v;
+        L.func();
+
+        try {
+            ClassLIM c  = new ClassLIM();  // is K initialized, with a perfectly good L in the middle
+            // was bug: this used to succeed and be able to callMethodInK().
+            throw new RuntimeException("FAIL exception not thrown for class");
+        } catch (ExceptionInInitializerError e) {
+            System.out.println("ExceptionInInitializerError thrown as expected");
+        }
+
+        // Test that K already has initialization error so gets ClassNotFoundException because
+        // initialization was attempted with ClassLIM.
+        try {
+            Class.forName("InterfaceInitializationStates$K", true, InterfaceInitializationStates.class.getClassLoader());
+            throw new RuntimeException("FAIL exception not thrown for forName(K)");
+        } catch(ClassNotFoundException e) {
+            throw new RuntimeException("ClassNotFoundException should not be thrown");
+        } catch(NoClassDefFoundError e) {
+            System.out.println("NoClassDefFoundError thrown as expected");
+        }
+
+        new ClassM();
+
+        // Initialize I, which doesn't cause K (super interface) to be initialized.
+        // Since the initialization of I does _not_ cause K to be initialized, it does
+        // not get NoClassDefFoundError because K is erroneous.
+        // But the initialization of I throws RuntimeException, so we expect
+        // ExceptionInInitializerError.
+        try {
+            Object ii = I.CONST;
+            throw new RuntimeException("FAIL exception not thrown for I's initialization");
+        } catch (ExceptionInInitializerError e) {
+            System.out.println("ExceptionInInitializerError as expected");
+        }
+
+        // Initialize Iunlinked. This should not get NoClassDefFoundError because K
+        // (its super interface) is in initialization_error state.
+        // This is a bug.  It does now.
+        try {
+            boolean bb = Iunlinked.v;
+            throw new RuntimeException("FAIL exception not thrown for Iunlinked initialization");
+        } catch(NoClassDefFoundError e) {
+            System.out.println("NoClassDefFoundError thrown because of bug");
+        }
+
+        // This should be okay
+        boolean value = Iparams.v;
+        System.out.println("value is " + value);
+
+        ClassIparams p = new ClassIparams();
+        try {
+            // Now we get an error because K got an initialization_error
+            K kk = p.method();
+            throw new RuntimeException("FAIL exception not thrown for calling method for K");
+        } catch(NoClassDefFoundError e) {
+            System.out.println("NoClassDefFoundError thrown as expected");
+        }
+
+         // Check expected class initialization order
+        List<Class<?>> expectedCInitOrder = Arrays.asList(L.class, K.class, M.class, ClassM.class,
+                                                          I.class, Iparams.class,
+                                                          ClassIparams.class);
+        if (!cInitOrder.equals(expectedCInitOrder)) {
+            throw new RuntimeException(
+                String.format("Class initialization array %s not equal to expected array %s",
+                              cInitOrder, expectedCInitOrder));
+        }
+    }
+
+    static boolean out(Class c) {
+        System.out.println("#: initializing " + c.getName());
+        cInitOrder.add(c);
+        return true;
+    }
+    static Object someMethod() {
+        throw new RuntimeException();
+    }
+}