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

[Vladimir Ivanov]

On 5/14/14 4:12 PM, Paul Sandoz wrote:
>
> On May 14, 2014, at 12:47 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>
>> Tobias, I agree with your evaluation.
>>
>
> V. tricky one to track down!
>
>  From @Stable:
>
>   * It is (currently) undefined what happens if a field annotated as stable
>   * is given a third value.  In practice, if the JVM relies on this annotation
>   * to promote a field reference to a constant, it may be that the Java memory
>   * model would appear to be broken, if such a constant (the second value of the field)
>   * is used as the value of the field even after the field value has changed.
>
> I dunno if that was a contributing factor in this case.
No, @Stable doesn't contribute to the problem.

>
>> My only concern is that @Stable doesn't work for AtomicReferenceArray, so JIT doesn't see what is stored in the array.
>
> Yes, stability needs to be associated with the array elements.
>
>
>> Maybe use a lock instead?
>>
>
> Or perhaps use Unsafe.CAS directly within setCachedLambdaForm?
Yes, Unsafe is another option here. But since cache updates should be 
rare, Unsafe is an overkill here IMO - locking should be fine.

>
> Also, as a consequence of using AtomicReferenceArray the following change may result in a memory barrier on some architectures:
>
>       public LambdaForm cachedLambdaForm(int which) {
> -        return lambdaForms[which];
>
> +        return lambdaForms.get(which);
>       }
>
> since lambdaForms.get will call Unsafe.getObjectVolatile.
MTF.cachedLambaForm isn't on a fast path, so it shouldn't be a problem.

> Separately, i think code that calls setCachedLambdaForm needs to be double checked to ensure that the return value is used. For example, in MethodHandleImpl.makeGuardWithCatchForm i see:
>
>          basicType.form().setCachedLambdaForm(MethodTypeForm.LF_GWC, lform);
>          return lform;
>
> which i think needs to be:
>
>          return basicType.form().setCachedLambdaForm(MethodTypeForm.LF_GWC, lform);
Good catch! MTF.setCachedLambdaForm usages should be fixed as well.

Best regards,
Vladimir Ivanov

>
> Paul.
>
>
>
>