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();
On Mon, Jan 8, 2018 at 10:50 PM, Paul Sandoz <paul.san...@oracle.com> wrote:
> 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 <marti...@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 <marti...@google.com>
> wrote:
> >
> >> 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;
> >>>
> >>>
> >>>
> >>
>
>
