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
--- 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();
+ }
+}