8163969: Cyclic interface initialization causes JVM crash

Summary: Backport change to correct interface initialization.
Reviewed-by: gtriantafill, sspitsyn, dholmes

src/share/vm/oops/instanceKlass.cpp

@@ -616,7 +616,11 @@ bool InstanceKlass::link_class_or_fail(TRAPS) {
 
 bool InstanceKlass::link_class_impl(
     instanceKlassHandle this_oop, 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_oop->is_in_error_state()) {
     ResourceMark rm(THREAD);
     THROW_MSG_(vmSymbols::java_lang_NoClassDefFoundError(),
@@ -801,37 +805,22 @@ void InstanceKlass::link_methods(TRAPS) {
 }
 
 // Eagerly initialize superinterfaces that declare default methods (concrete instance: any access)
-void InstanceKlass::initialize_super_interfaces(instanceKlassHandle this_oop, TRAPS) {
-  if (this_oop->has_default_methods()) {
-    for (int i = 0; i < this_oop->local_interfaces()->length(); ++i) {
-      Klass* iface = this_oop->local_interfaces()->at(i);
+void InstanceKlass::initialize_super_interfaces(instanceKlassHandle this_k, TRAPS) {
+  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);
-      if (ik->should_be_initialized()) {
-        if (ik->has_default_methods()) {
-          ik->initialize_super_interfaces(ik, THREAD);
-        }
-        // Only initialize() interfaces that "declare" concrete methods.
+
+    // 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 (!HAS_PENDING_EXCEPTION && ik->declares_default_methods()) {
-          ik->initialize(THREAD);
+    if (ik->has_default_methods()) {
+      ik->initialize_super_interfaces(ik, CHECK);
     }
-        if (HAS_PENDING_EXCEPTION) {
-          Handle e(THREAD, PENDING_EXCEPTION);
-          CLEAR_PENDING_EXCEPTION;
-          {
-            EXCEPTION_MARK;
-            // Locks object, set state, and notify all waiting threads
-            this_oop->set_initialization_state_and_notify(
-                initialization_error, THREAD);
 
-            // ignore any exception thrown, superclass initialization error is
-            // thrown below
-            CLEAR_PENDING_EXCEPTION;
-          }
-          THROW_OOP(e());
-        }
-      }
+    // Only initialize() interfaces that "declare" concrete methods.
+    if (ik->should_be_initialized() && ik->declares_default_methods()) {
+      ik->initialize(CHECK);
     }
   }
 }
@@ -897,30 +886,36 @@ void InstanceKlass::initialize_impl(instanceKlassHandle this_oop, TRAPS) {
   }
 
   // Step 7
+  // Next, if C is a class rather than an interface, initialize its super class and super
+  // interfaces.
+  if (!this_oop->is_interface()) {
     Klass* super_klass = this_oop->super();
-  if (super_klass != NULL && !this_oop->is_interface() && super_klass->should_be_initialized()) {
+    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_oop->has_default_methods()) {
+      this_oop->initialize_super_interfaces(this_oop, 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_oop->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_oop->set_initialization_state_and_notify(initialization_error, THREAD);
+        CLEAR_PENDING_EXCEPTION;
       }
       DTRACE_CLASSINIT_PROBE_WAIT(super__failed, InstanceKlass::cast(this_oop()), -1,wait);
       THROW_OOP(e());
     }
   }
 
-  // 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_oop->has_default_methods()) {
-    this_oop->initialize_super_interfaces(this_oop, CHECK);
-  }
-
   // Step 8
   {
     assert(THREAD->is_Java_thread(), "non-JavaThread in initialize_impl");
@@ -981,10 +976,15 @@ void InstanceKlass::set_initialization_state_and_notify(ClassState state, TRAPS)
 
 void InstanceKlass::set_initialization_state_and_notify_impl(instanceKlassHandle this_oop, ClassState state, TRAPS) {
   oop init_lock = this_oop->init_lock();
-  ObjectLocker ol(init_lock, THREAD, init_lock != NULL);
+  if (init_lock != NULL) {
+    ObjectLocker ol(init_lock, THREAD);
     this_oop->set_init_state(state);
     this_oop->fence_and_clear_init_lock();
     ol.notify_all(CHECK);
+  } else {
+    assert(init_lock != NULL, "The initialization state should never be set twice");
+    this_oop->set_init_state(state);
+  }
 }
 
 // The embedded _implementor field can only record one implementor.

test/runtime/lambda-features/TestInterfaceInit.java

 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 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
@@ -25,7 +25,8 @@
 /*
  * @test
  * @bug 8034275
- * @summary [JDK 8u40] Test interface initialization: only for interfaces declaring default methods
+ * @bug 8163969
+ * @summary [JDK 8u40] Test interface init: only for interfaces declaring default methods, when subclass inits
  * @run main TestInterfaceInit
  */
 import java.util.List;
@@ -39,43 +40,59 @@ public class TestInterfaceInit {
    // Declares a default method and initializes
    interface I {
        boolean v = TestInterfaceInit.out(I.class);
-        default void x() {}
+        default void ix() {}
    }
 
    // Declares a default method and initializes
    interface J extends I {
        boolean v = TestInterfaceInit.out(J.class);
-       default void x() {}
+       default void jx() {}
    }
-   // No default method, does not initialize
+   // No default method, has an abstract method, does not initialize
    interface JN extends J {
        boolean v = TestInterfaceInit.out(JN.class);
+       public abstract void jnx();
    }
 
    // Declares a default method and initializes
    interface K extends I {
        boolean v = TestInterfaceInit.out(K.class);
-        default void x() {}
+        default void kx() {}
    }
 
-   // No default method, does not initialize
+   // No default method, has a static method, does not initialize
    interface KN extends K {
        boolean v = TestInterfaceInit.out(KN.class);
+       static void knx() {}
    }
 
    interface L extends JN, KN {
        boolean v = TestInterfaceInit.out(L.class);
-        default void x() {}
+        default void lx() {}
+   }
+
+   static class ChildClass implements JN, KN {
+       boolean v = TestInterfaceInit.out(ChildClass.class);
+       public void jnx() {}
    }
 
    public static void main(String[] args) {
        // Trigger initialization
        boolean v = L.v;
 
-       List<Class<?>> expectedCInitOrder = Arrays.asList(I.class,J.class,K.class,L.class);
+       List<Class<?>> expectedCInitOrder = Arrays.asList(L.class);
+       if (!cInitOrder.equals(expectedCInitOrder)) {
+         throw new RuntimeException(String.format("Class initialization array %s not equal to expected array %s", cInitOrder, expectedCInitOrder));
+       }
+
+       ChildClass myC = new ChildClass();
+       boolean w = myC.v;
+
+       expectedCInitOrder = Arrays.asList(L.class,I.class,J.class,K.class,ChildClass.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) {