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

[David Holmes]

On 16/05/2014 4:25 AM, Remi Forax wrote:
>
> On 05/15/2014 04:26 PM, Paul Sandoz wrote:
>> On May 15, 2014, at 1:29 PM, Vladimir Ivanov
>> <vladimir.x.ivanov at oracle.com> 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?
>>>
>> 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
>