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

[Tobias Hartmann]

Hi,

thanks to everyone for the feedback.

Is it sufficient then to use synchronized (lambdaForms) { ... } in 
setCachedLambdaForm(..) and a normal read in cachedLambdaForm(..)?

Thanks,
Tobias

>>>
>>>>>>> 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?
>>>>
>>> I thought that since the only fields explicitly set when constructing
>>> a LambdaForm are final fields it should be safe to publish the form
>>> reference as an element in the array?
>>>
>>> Paul.
>>>
>>
>> that's my analysis too.
>
> Sorry didn't realize a LF only had final fields.
>
> David
>
>> Rémi
>>