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

[Coleen Phillimore]

On 08/20/2013 06:24 PM, Coleen Phillimore wrote:
>
>
> Vladimir,
>
> You are right, it's broken.  Calvin is working on a similar issue in 
> bug https://jbs.oracle.com/bugs/browse/JDK-8020675.

Reading more, I think Calvin's issue is similar but different.  I will 
file a new bug for the Threads::create_vm() EXCEPTION_MARK.

Coleen

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