RFR(S): 8251923: "Invalid JNI handle" assertion failure in JVMCICompiler::force_comp_at_level_simple()

[Vladimir Kozlov]

+1

Thanks,
Vladimir K

On 8/19/20 12:16 PM, Doug Simon wrote:
> Looks good to me.
> 
> -Doug
> 
>> On 19 Aug 2020, at 10:37, Nick Gasson <Nick.Gasson at arm.com> wrote:
>>
>> Hi,
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8251923
>> Webrev: http://cr.openjdk.java.net/~ngasson/8251923/webrev.1/
>>
>> We see this crash occasionally when testing with Graal on some AArch64
>> systems:
>>
>> #
>> #  Internal Error (/home/ent-user/jdk_src/src/hotspot/share/runtime/jniHandles.inline.hpp:63), pid=92161, tid=92593
>> #  assert(external_guard || result != __null) failed: Invalid JNI handle
>> #
>>
>> V  [libjvm.so+0xdfaa84]  JNIHandles::resolve(_jobject*)+0x19c
>> V  [libjvm.so+0xf25104]  HotSpotJVMCI::resolve(JVMCIObject)+0x14
>> V  [libjvm.so+0xe9bd20]  JVMCICompiler::force_comp_at_level_simple(methodHandle const&)+0xa0
>> V  [libjvm.so+0x174bd6c]  TieredThresholdPolicy::is_mature(Method*)+0x51c
>> V  [libjvm.so+0x76e68c]  ciMethodData::load_data()+0x9cc
>>
>> The full hs_err file is attached to the JBS entry.
>>
>> The handle here is _HotSpotJVMCIRuntime_instance which is initialised in
>> JVMCIRuntime::initialize_HotSpotJVMCIRuntime():
>>
>>    JVMCIObject result = JVMCIENV->call_HotSpotJVMCIRuntime_runtime(JVMCI_CHECK);
>>    _HotSpotJVMCIRuntime_instance = JVMCIENV->make_global(result);
>>
>> JVMCICompiler::force_comp_at_level_simple() checks whether the _object
>> field inside the handle is null before calling JNIHandles::resolve() on
>> it, which should avoid the above assertion failure where the pointee is
>> null. However on a non-TSO architecture another thread may observe the
>> store to _object when assigning _HotSpotJVMCIRuntime_instance before the
>> store in JVMCIEnv::make_global() that initialises the pointed-to oop. We
>> need to add a store-store barrier here to force the expected ordering.
>>
>> Tested with jcstress and Graal on the affected machine, which used to
>> reproduce it quite reliably.
>>
>> --
>> Thanks,
>> Nick
>