# HG changeset patch # User coleenp # Date 1474466178 14400 # Node ID 871b2f487dc0606d8811828ad7e3905882a5fd84 # Parent e9a1638b8cea27beaf17217733f1a865bd4dd840 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 diff -r e9a1638b8cea -r 871b2f487dc0 hotspot/src/share/vm/oops/instanceKlass.cpp --- 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. diff -r e9a1638b8cea -r 871b2f487dc0 hotspot/test/runtime/lambda-features/CyclicInterfaceInit.java --- /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. + } +} diff -r e9a1638b8cea -r 871b2f487dc0 hotspot/test/runtime/lambda-features/InterfaceInitializationStates.java --- /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> 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> 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(); + } +}