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

[David Holmes]

On 15/05/2014 9:29 PM, Vladimir Ivanov wrote:
>  >>> I may be wrong but for me cachedLambdaForm() is used in a fast path.
>>>> If it's not the case, I agree that a lock is enough.
>>> I hold the opinion that we only need to synchronize writers and not
>>> readers (MTF.setCachedLambdaForm() and not MTF.cachedLambdaForm()).
>>>
>>> Current usage pattern is the following:
>>>     MethodType type = ...;
>>>     LambdaForm lform = type.form().cachedLambdaForm(idx);
>>>     if (lform == NULL) {
>>>        // construct new LambaForm
>>>        lform = type.form().setCachedLambdaForm(idx, lform);
>>>     }
>>>     return lform;
>>>
>>> Cache is written only once, so it has only 2 states (null and non-null
>>> value) during it's lifecycle.
>>>
>>> The only stale value cachedLambdaForm() can see is null, but then the
>>> caller tries to initialize the cache and after acquiring the lock in
>>> setCachedLambdaForm() sees actual value.
>>>
>>> So, the worst thing can happen if readers aren't synchronized is
>>> creation of unnecessary LF (which go dead right away) in rare cases.
>>
>> Unless the cache is volatile this would be unsafe publication and the
>> reader may see a non-null reference to the LF but the LF may not be
>> properly initialized.
> Good point. The cache is an array element. How does safe publication of
> array element look like? Is accessing the element through volatile
> reference to the array enough or additional synchronization actions are
> needed?

Additional sync would be needed, such as using an AtomicReferenceArray. 
Otherwise a volatile array reference doesn't make the array elements 
volatile.

David

> Best regards,
> Vladimir Ivanov
>
>>
>> David
>> -----
>>
>>
>>> Best regards,
>>> Vladimir Ivanov
>>>>
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>
>>>> regards,
>>>> Rémi
>>>>