RFR (XS): 8022595: JSR292: deadlock during class loading of MethodHandles, MethodHandleImpl & MethodHandleNatives

[Coleen Phillimore]

Vladimir,

You are right, it's broken.  Calvin is working on a similar issue in bug 
https://jbs.oracle.com/bugs/browse/JDK-8020675.

I think you should check in your code and we'll fix all the 
CHECK_0/EXCEPTION_MARK problems at once.

Thanks for checking this out.
Coleen

On 08/20/2013 05:55 PM, Vladimir Ivanov wrote:
> Coleen,
>
> Thank you for the review.
> It doesn't cause a deadlock anymore because class loading is 
> guaranteed to be performed in singe-threaded mode.
>
> Regarding CHECK_0 -> CHECK_(JNI_ERR) change you propose, there's one 
> problem. All pending exceptions are fatal right now: even if it 
> returns JNI_OK, there's an ExceptionMark on the stack which crashes VM 
> if any pending exceptions are present.
>
> Considering that JNI_CreateJavaVM uses Thread::create_vm and failure 
> during VM initialization can crash the whole app, I don't think such 
> behavior is correct.
>
> So, I have 2 questions here:
>     1) what behavior do we prefer?
>     2) if we want to change current behavior, doesn't it deserve a 
> separate fix?
>
> Best regards,
> Vladimir Ivanov
>
> On 8/20/13 11:02 PM, Coleen Phillimore wrote:
>>
>> Vladimir,
>>
>> The code that you added is copied from the initialization code above.  I
>> think they have a problem.  If initialization fails, there is a CHECK_0
>> at the end of the call.  CHECK_0 returns JNI_OK to the caller, which
>> goes ahead with the initialization.
>>
>> Can you change all of these to CHECK_(JNI_ERR) so that the caller can
>> deal with the error?  It would be good to see if the caller does the
>> right thing with a JNI_ERR.
>>
>> Otherwise, I think the code is fine if the initialization of these
>> classes doesn't cause deadlocks in their new place.
>>
>> Thanks,
>> Coleen
>>
>> On 08/20/2013 01:05 PM, Vladimir Ivanov wrote:
>>> http://cr.openjdk.java.net/~vlivanov/8022595/webrev.01/
>>> 201 lines changed: 201 ins; 0 del; 0 mod;
>>>
>>> Some classes in j.l.invoke have circular dependencies between them
>>> which leads to deadlocks during class initialization when multiple
>>> threads exercise JSR292 functionality.
>>>
>>> JSR292 core classes are among "well-known" classes to VM and they are
>>> preloaded during VM startup (see
>>> SystemDictionary::initialize_preloaded_classes). However, it doesn't
>>> force preloaded classes to be initialized (doesn't execute static
>>> initializers). Thus, these classes should be explicitly
>>> pre-initialized in order to avoid deadlocks.
>>>
>>> Based on my observations of possible deadlock scenarios, I chose 3
>>> classes to pre-initialize:
>>>     - MethodHandle
>>>     - MemberName
>>>     - MethodHandleNatives
>>>
>>> I placed the code right after compilers initialization because during
>>> method resolution for signature polymorphic intrinsics, compilation
>>> tasks are issued (see SystemDictionary::find_method_handle_intrinsic
>>> for details) and they are missed if compiler subsystem isn't ready yet.
>>>
>>> Testing: failing test, regression test, JPRT, jdk/test/j/l/invoke,
>>> vm.mlvm.testlist, octane.
>>>
>>> Thanks!
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>