RFR 8163969: Cyclic interface initialization causes JVM crash

Coleen Phillimore coleen.phillimore at oracle.com
Tue Sep 20 20:19:43 UTC 2016

On 9/20/16 1:39 PM, Karen Kinnear wrote:
> Coleen,
> Code looks good. Kudos to you and David Holmes for extremely careful reading of the specification.

Thank you for the consultation on this bug!
> In an earlier web rev you had also made a defensive fix to set_initialization_state_and_notify_impl.
> Could you possibly add a fix for that?
> 1) assertion if init_lock is null
> 2) for production time: if init_lock is null, at least do not try to dereference ObjectLocker.

I rewrote the function to assert if init_lock is null but handle the 
case in product for robustness.

this_k, ClassState state, TRAPS) {
   oop init_lock = this_k->init_lock();
   if (init_lock != NULL) {
     ObjectLocker ol(init_lock, THREAD);
   } else {
     assert(init_lock != NULL, "The initialization state should never be 
set twice");
> If UseBiasedLocking is disabled, there is a risk of dereferencing null otherwise.
> Thank you for the detailed tests.
> 3) In InterfaceInitializationStates.java - could you possibly explain the Iunlinked test - what it is testing, what the
> comment means and what the new bug is?

Maybe this is better?

     // 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.

> A couple of minor comments/suggestions to make it easier for the person who changes it next on the InterfaceInitializationStates.java tests:
> 1) line 66 comment refers to ClassL - I think you mean ClassLIM?

> 2) line 61: “Calling function on class with bad super interface.”
>    I think you want “on class” -> “on interface”

Yes, thanks.
> 3) Could you add a comment before the try on Class.forName that the result of this test is NCDFE because
> there was an earlier test (ClassLIM) that already initialized K?

         // Test that K already has initialization error so gets 
ClassNotFoundException because
         // initialization was attempted with ClassLIM.
> 4) line 143: “K (sub interface)” -> “K (super interface)”

> 5) lines 155-156 “K, its superclass state” -> “K (its super interface) is in initialization_error state.

Yes, that reads better.

Thanks for the comments and careful review on this fix!


> thanks
> Karen
>> On Sep 16, 2016, at 11:22 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>> Summary: Fix interface initialization to follow spec: interface initializations do not set initialization state of interfaces that extend them.
>> Tested with: all hotspot jtreg tests, co-located nsk tests, non-colocated nsk tests, and jck tests.
>> open webrev at http://cr.openjdk.java.net/~coleenp/8163969.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8163969
>> Thanks,
>> Coleen

More information about the hotspot-runtime-dev mailing list