[9] RFR(S): 8005873: JRuby test_respond_to.rb asserts with: MT-unsafe modification of inline cache

[Vladimir Ivanov]

Tobias, I agree with your evaluation.

My only concern is that @Stable doesn't work for AtomicReferenceArray, 
so JIT doesn't see what is stored in the array. Maybe use a lock instead?

Best regards,
Vladimir Ivanov

On 5/14/14 2:33 PM, Tobias Hartmann wrote:
> Hi Remi,
>
> sorry, I accidentally reverted that part.. Here is the correct webrev:
>
> http://cr.openjdk.java.net/~anoll/8005873/webrev.01/
>
> Thanks,
> Tobias
>
> On 14.05.2014 12:04, Remi Forax wrote:
>> your patch doesn't work !
>>
>> the array is still allocated as an classical array in the constructor.
>>
>> cheers,
>> Remi
>>
>> Envoyé avec AquaMail pour Android
>> http://www.aqua-mail.com
>>
>>
>> Le 14 mai 2014 11:04:41 Tobias Hartmann <tobias.hartmann at oracle.com> a
>> écrit :
>>
>>> Hi,
>>>
>>> please review the following patch for bug 8005873.
>>>
>>> *Problem*
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8005873
>>>
>>> The test creates multiple threads (~500) that repeatedly execute
>>> invokeExact on a method handle referencing a static method. The call
>>> to invokeExact is performed through an optimized inline cache
>>> (CompiledStaticCall) that points to the LambdaForm generated for the
>>> method handle. The IC is first set to interpreted code by
>>> CompiledStaticCall::set_to_interpreted(...) but soon updated to refer
>>> to the compiled version of the lambda form (-Xcomp).
>>>
>>> Because tiered compilation is enabled, the VM decides to compile the
>>> LambdaForm at a different level and sets the old version to
>>> not-entrant. The next call through the IC therefore triggers a
>>> re-resolving of the call site finally performing a Java upcall to
>>> java.lang.invoke.MethodHandleNatives::linkMethod(...) (see call
>>> sequence [1]). A *new* LambdaForm is returned and
>>> CompiledStaticCall::set_to_interpreted(...) is executed again to
>>> update the IC. The assert is hit because the callee differs.
>>>
>>> The problem is that there are multiple LambdaForms created for the
>>> same invokeExact instruction. Caching in the Java runtime code should
>>> guarantee that only one LambdaForm is created and reused.
>>> Instrumenting Invokers::invokeHandleForm(...) shows that almost all
>>> requests result in a cache miss and return a new LambdaForm.
>>>
>>> This behaviour is caused by a race condition in
>>> Invokers::invokeHandleForm(...). Threads may initially see a cache
>>> miss when invoking MethodTypeForm::cachedLambdaForm(...), then create
>>> a new LambdaForm and call MethodTypeForm::setCachedLambdaForm(...) to
>>> update the cache without checking it again. In a high concurrency
>>> setting, this leads to multiple LambdaForms being created for the
>>> same invokeExact instruction because the cache entry is overwritten
>>> by multiple threads.
>>>
>>> *Solution*
>>> Webrev: http://cr.openjdk.java.net/~anoll/8005873/webrev.00/
>>>
>>> An AtomicReferenceArray is used to cache the LambdaForms and
>>> .get(...) and .compareAndSet(...) are used to retrieve and update the
>>> cache entries. This allows only one thread to set the LambdaForm that
>>> is then being used by all instances of the invokeExact.
>>>
>>> *Testing*
>>> - Failing test (vm/mlvm/meth/stress/jni/nativeAndMH)
>>> - Nashorn + Octane
>>> - JPRT
>>>
>>> Many thanks to Christian Thalinger and Vladimir Ivanov!
>>>
>>> Best,
>>> Tobias
>>>
>>> [1] Call sequence of reresolving the IC target
>>> SharedRuntime::handle_wrong_method(...)
>>> -> SharedRuntime::reresolve_call_site(...)
>>> -> SharedRuntime::find_callee_method(...)
>>> -> SharedRuntime::find_callee_info_helper(...)
>>> -> LinkResolver::resolve_invoke(...)
>>> -> LinkResolver::resolve_invokehandle(...)
>>> -> LinkResolver::resolve_invokehandle(...)
>>> -> LinkResolver::lookup_polymorphic_method(...)
>>> -> SystemDictionary::find_method_handle_invoker(...)
>>> -> Java upcall to java.lang.invoke.MethodHandleNatives::linkMethod(...)
>>> -> Invokers::methodHandleInvokeLinkerMethod(...)
>>> -> Invokers::invokeHandleForm(...)
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> Backport?
>>
>>
>