From fcf6ce20010f5d2b5bea97c9a488b678d53e91f6 Mon Sep 17 00:00:00 2001 From: coleenp Date: Thu, 13 Oct 2016 11:57:45 -0400 Subject: [PATCH] 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 | 96 +++++++++---------- .../lambda-features/TestInterfaceInit.java | 35 +++++-- 2 files changed, 74 insertions(+), 57 deletions(-) diff --git a/src/share/vm/oops/instanceKlass.cpp b/src/share/vm/oops/instanceKlass.cpp index 481742db9..4ecc87e95 100644 --- a/src/share/vm/oops/instanceKlass.cpp +++ b/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); - 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_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()); - } - } +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); + + // 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); } } } @@ -897,30 +886,36 @@ void InstanceKlass::initialize_impl(instanceKlassHandle this_oop, TRAPS) { } // Step 7 - Klass* super_klass = this_oop->super(); - if (super_klass != NULL && !this_oop->is_interface() && super_klass->should_be_initialized()) { - super_klass->initialize(THREAD); + // 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 && 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); - this_oop->set_init_state(state); - this_oop->fence_and_clear_init_lock(); - ol.notify_all(CHECK); + 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. diff --git a/test/runtime/lambda-features/TestInterfaceInit.java b/test/runtime/lambda-features/TestInterfaceInit.java index 0493a60bb..a6f057c04 100644 --- a/test/runtime/lambda-features/TestInterfaceInit.java +++ b/test/runtime/lambda-features/TestInterfaceInit.java @@ -1,5 +1,5 @@ /* - * 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> expectedCInitOrder = Arrays.asList(I.class,J.class,K.class,L.class); + List> 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) { -- GitLab