JDK-8145371 ClassCastException thrown in LambdaFormEditor.getInCache

[Martin Buchholz]

On Tue, Jan 9, 2018 at 4:12 AM, Vladimir Ivanov <
vladimir.x.ivanov at oracle.com> wrote:

>
> In the hotspot sources I find
>>        // An instance field can be constant if it's a final static field
>> or if
>>        // it's a final non-static field of a trusted class (classes in
>>        // java.lang.invoke and sun.invoke packages and subpackages).
>> BUT
>> you can't trust MethodHandle.form!
>> Yes, it's
>>          final LambdaForm form;
>> but it's also frobbed via Unsafe, and in an unusual unsafe way:
>>          UNSAFE.putObject(this, FORM_OFFSET, newForm);
>>          UNSAFE.fullFence();
>>
>
> Constant folding of MH.form is crucial for performance: without it no
> inlining through MethodHandle chains would happen.
>
> It is expected that MH.form updates aren't guaranteed to be visible (and
> it's not a theoretical possibility, but a pretty common scenario in
> JIT-compiled code (both by C1 & C2)). So, observing old MH.form values
> should be benign.
>
> Here's the relevant part of MH.updateForm() contract:
>     /**
>      * Replace the old lambda form of this method handle with a new one.
>      * The new one must be functionally equivalent to the old one.
>     ...
>     void updateForm(LambdaForm newForm) {
>
> It is used to improve performance (remove class init check, customize LF
> for a particular MH instance) and not to arbitrarily change behavior of
> existing MH instance.
>
> Actual code on writer side (in MH.updateForm) doesn't matter much (it can
> be a volatile or even a plain field write) while readers treat the field as
> a plain final.
>
> There's an option to invalidate all compiled code where old MH.form value
> is used (akin to how CallSite.target is optimized), but it doesn't worth it
> most of the time: invalidating C1-compiled code isn't needed in tiered mode
> (it'll be recompiled by C2 anyway), recompilation is expensive (waste of
> resources), and possible performance improvements are limited.
>
> So, having a full fence there (to improve how prompt update is visible to
> readers w/o affecting them) is as good as other options IMO and I'd leave
> it as is for now.
>

Thanks for the explanation.

Simple fix was submitted.

I'm still hoping we can later reduce the magicalness of j.l.invoke.

The memory model is already too hard to reason about, but here the VM can
assume that the final fields will never be mutated - yet they are!  With
the special j.l.invoke guarantees, there should be no need for final fields
to additionally have @Stable annotations, yet many do!  LambdaForm is a
mutable class, so publishing it via a plain Unsafe write is a (tiny, hard
to detect) data race.  I would feel much more comfortable replacing the
Unsafe put with a putVolatile and dropping the fence.  Whenever the form
field is read, perhaps it should be explicitly read via a volatile or
acquire read for safety.