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

[Vladimir Ivanov]

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
>