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

[Remi Forax]

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.

Rémi