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

[Remi Forax]

On 05/16/2014 01:56 PM, Tobias Hartmann wrote:
> 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

no, it's not,
The memory model doesn't guarantee that a normal read will see that 
another thread has changed the array item,
the normal read may always see null.

You need a synchronized block around the normal read.

Rémi


>
>>>>
>>>>>>>> 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
>>>
>