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

[Paul Sandoz]

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.


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

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.

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

Paul.