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

[Nick Gasson]

Thank you both for the reviews. I've pushed it.

--
Nick

On 08/20/20 03:18 am, Vladimir Kozlov wrote:
> +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
>>