Let's try again without mail bounces ... On Wed, Jan 3, 2018 at 1:40 PM, Martin Buchholz <marti...@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; > > >
