JDK-8145371 ClassCastException thrown in LambdaFormEditor.getInCache

[Paul Sandoz]

Hi Martin

[Back from holiday]

Can you reproduce using the debugger? If so do you have a more up to date stack trace?

That change looks ok. The form field is final, and in the j.l.invoke package such fields are also stable so once C2 gets at it it will likely treat it as a constant. In general the updates to the field will only be visible to interpreted code or recompiled code.

I suspect the problematic field may well be that of LambdaForm.transformCache.

Paul.

> On 8 Jan 2018, at 18:41, Martin Buchholz <martinrb at google.com> wrote:
> 
> No takers? ... let's try again.  Despite uniform failure to reproduce this
> except under a debugger, let's apply the simple obviously correct fix,
> namely:
> 
>     /** Craft a LambdaForm customized for this particular MethodHandle */
>     /*non-public*/
>     void customize() {
> +        final LambdaForm form = this.form;
>         if (form.customized == null) {
>             LambdaForm newForm = form.customize(this);
>             updateForm(newForm);
> 
> 
> 
> 
> On Wed, Jan 3, 2018 at 1:51 PM, Martin Buchholz <martinrb at google.com> wrote:
> 
>> Let's try again without mail bounces ...
>> 
>> 
>> On Wed, Jan 3, 2018 at 1:40 PM, Martin Buchholz <martinrb at google.com>
>> wrote:
>> 
>>> Here at Google we tried to fix JDK-8145371 because it looked like it was
>>> causing rare problems in production.  But after looking at the code, I'm
>>> not sure...  Anyways,
>>> 
>>> http://cr.openjdk.java.net/~martin/webrevs/jdk/MethodHandle.form/
>>> https://bugs.openjdk.java.net/browse/JDK-8145371
>>> Copying to a local in customize() must be an improvement.
>>> The UNSAFE publication in updateForm looks fishy, as does that comment
>>> in MethodHandleImpl.java.  Is there something the fullFence() is supposed
>>> to do that isn't taken care of by putObjectVolatile ?
>>> 
>>> Feel free to take ownership of this bug from me.
>>> 
>>> --- a/src/java.base/share/classes/java/lang/invoke/MethodHandle.java
>>> +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandle.java
>>> @@ -1660,13 +1660,13 @@
>>>         assert(newForm.customized == null || newForm.customized == this);
>>>         if (form == newForm)  return;
>>>         newForm.prepare();  // as in MethodHandle.<init>
>>> -        UNSAFE.putObject(this, FORM_OFFSET, newForm);
>>> -        UNSAFE.fullFence();
>>> +        UNSAFE.putObjectVolatile(this, FORM_OFFSET, newForm);
>>>     }
>>> 
>>>     /** Craft a LambdaForm customized for this particular MethodHandle */
>>>     /*non-public*/
>>>     void customize() {
>>> +        final LambdaForm form = this.form;
>>>         if (form.customized == null) {
>>>             LambdaForm newForm = form.customize(this);
>>>             updateForm(newForm);
>>> diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java
>>> --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java
>>> +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java
>>> @@ -909,7 +909,7 @@
>>>             int c = count;
>>>             maybeCustomizeTarget();
>>>             if (c <= 1) {
>>> -                // Try to limit number of updates. MethodHandle.updateForm() doesn't guarantee LF update visibility.
>>> +                // Try to limit number of updates.
>>>                 if (isCounting) {
>>>                     isCounting = false;
>>>                     return true;
>>> 
>>> 
>>> 
>>