SI-9824 SI-9814 proper locking scope for lazy vals in lambdas

[adriaanm]

Consolidating comments from #161 and #177:

• SI-9824 deadlock using parallel collections with delambdafy:method
• SI-9814 Function0 doesn't synchronize properly

solution

Let users opt-in to old encoding + feature in release notes.

a look at the byte code

lazy val in closure has different locking scope due to delambdafy:method. It used to be the anonymous function class:

Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_71).
Type in expressions for evaluation. Or try :help.

scala> :paste -raw
// Entering paste mode (ctrl-D to finish)

package p; class C { def foo = () => { lazy val x = 42; x } }

// Exiting paste mode, now interpreting.


scala> new p.C().foo
res0: () => Int = <function0>

scala> new p.C().foo.getClass
res1: Class[_ <: () => Int] = class p.C$$anonfun$foo$1

scala> :javap -private -c p.C$$anonfun$foo$1
Compiled from "<pastie>"
public final class p.C$$anonfun$foo$1 extends scala.runtime.AbstractFunction0$mcI$sp implements scala.Serializable {
  public static final long serialVersionUID;

  public final int apply();
    Code:
       0: aload_0
       1: invokevirtual #21                 // Method apply$mcI$sp:()I
       4: ireturn

  public int apply$mcI$sp();
    Code:
       0: invokestatic  #29                 // Method scala/runtime/IntRef.zero:()Lscala/runtime/IntRef;
       3: astore_1
       4: iconst_0
       5: invokestatic  #35                 // Method scala/runtime/VolatileByteRef.create:(B)Lscala/runtime/VolatileByteRef;
       8: astore_2
       9: aload_0
      10: aload_1
      11: aload_2
      12: invokespecial #39                 // Method x$1:(Lscala/runtime/IntRef;Lscala/runtime/VolatileByteRef;)I
      15: ireturn

  public final java.lang.Object apply();
    Code:
       0: aload_0
       1: invokevirtual #46                 // Method apply:()I
       4: invokestatic  #52                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
       7: areturn

  private final int x$lzycompute$1(scala.runtime.IntRef, scala.runtime.VolatileByteRef);
    Code:
       0: aload_0
       1: dup
       2: astore_3
       3: monitorenter
       4: aload_2
       5: getfield      #57                 // Field scala/runtime/VolatileByteRef.elem:B
       8: iconst_1
       9: iand
      10: i2b
      11: iconst_0
      12: if_icmpne     32
      15: aload_1
      16: bipush        42
      18: putfield      #60                 // Field scala/runtime/IntRef.elem:I
      21: aload_2
      22: aload_2
      23: getfield      #57                 // Field scala/runtime/VolatileByteRef.elem:B
      26: iconst_1
      27: ior
      28: i2b
      29: putfield      #57                 // Field scala/runtime/VolatileByteRef.elem:B
      32: getstatic     #66                 // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
      35: pop
      36: aload_3
      37: monitorexit
      38: aload_1
      39: getfield      #60                 // Field scala/runtime/IntRef.elem:I
      42: ireturn
      43: aload_3
      44: monitorexit
      45: athrow
    Exception table:
       from    to  target type
           4    38    43   any

But is now the enclosing class of the lambda:

⚡ ~/scala/2.12/bin/scala
Welcome to Scala 2.12.0-M4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_71).
Type in expressions for evaluation. Or try :help.

scala> :paste -raw
// Entering paste mode (ctrl-D to finish)

package p; class C { def foo = () => { lazy val x = 42; x } }

// Exiting paste mode, now interpreting.


scala> :javap -private -c p.C
Compiled from "<pastie>"
public class p.C {
  public scala.Function0<java.lang.Object> foo();
    Code:
       0: aload_0
       1: invokedynamic #37,  0             // InvokeDynamic #0:apply$mcI$sp:(Lp/C;)Lscala/runtime/java8/JFunction0$mcI$sp;
       6: areturn

  private final int x$lzycompute$1(scala.runtime.IntRef, scala.runtime.VolatileByteRef);
    Code:
       0: aload_0
       1: dup
       2: astore_3
       3: monitorenter
       4: aload_2
       5: getfield      #51                 // Field scala/runtime/VolatileByteRef.elem:B
       8: iconst_1
       9: iand
      10: i2b
      11: iconst_0
      12: if_icmpne     32
      15: aload_1
      16: bipush        42
      18: putfield      #56                 // Field scala/runtime/IntRef.elem:I
      21: aload_2
      22: aload_2
      23: getfield      #51                 // Field scala/runtime/VolatileByteRef.elem:B
      26: iconst_1
      27: ior
      28: i2b
      29: putfield      #51                 // Field scala/runtime/VolatileByteRef.elem:B
      32: getstatic     #62                 // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
      35: pop
      36: aload_3
      37: monitorexit
      38: goto          44
      41: aload_3
      42: monitorexit
      43: athrow
      44: aload_1
      45: getfield      #56                 // Field scala/runtime/IntRef.elem:I
      48: ireturn
    Exception table:
       from    to  target type
           4    36    41   Class java/lang/Throwable

[adriaanm]

probably all we can do is document the workaround + expose option to use old encoding

[DarkDimius]

@adriaanm @retronym note that issue with a lazy val inside a lambda is fixed in Dotty.

[DarkDimius]

It is both quite easy to fix and removes one more indirection present in code.
Instead of having a pair of (IntRef, VolatileByteRef) dotty has a single dotty.runtime.LazyInt class that it synchronizes on. In your case, you can synchronize over VolatileByteRef.

import dotty.runtime.LazyInt;
import scala.Function0;
import scala.runtime.BoxesRunTime;

// version in dotty
public class C {
    public Function0 foo() {
        return () -> this.$anonfun$foo$1();
    }

    private int x$lzyINIT1$1(LazyInt x$lzy1$1) {
        Integer n;
        LazyInt lazyInt = x$lzy1$1;
        synchronized (lazyInt) {
            int n2;
            if (x$lzy1$1.initialized()) {
                n2 = x$lzy1$1.value();
            } else {
                x$lzy1$1.initialized_$eq(true);
                x$lzy1$1.value_$eq(42);
                n2 = x$lzy1$1.value();
            }
            n = BoxesRunTime.boxToInteger((int)n2);
        }
        return BoxesRunTime.unboxToInt((Object)n);
    }

    private int x$1(LazyInt x$lzy1$2) {
        return x$lzy1$2.initialized() ? x$lzy1$2.value() : this.x$lzyINIT1$1(x$lzy1$2);
    }

    private int $anonfun$foo$1() {
        LazyInt lazyInt = new LazyInt();
        return this.x$1(lazyInt);
    }
}

[lrytz]

Implementing it is not trivial: when generating the lzy$compute method (which synchronizes on C.this) in lazyvals the VolatileByteRef does not exist yet. It is only added later when lambdalift moves the method out of the def $anonfun$f nest.

How is this handled in dotty? Does the lazy val translation know if a lazy val is captured and needs boxing?

[DarkDimius]

@lrytz,
Dotty has multiple lazy vals schemes.
For local lazy vals, such as one in example - it uses holder classes https://github.com/lampepfl/dotty/blob/master/src/dotty/runtime/LazyHolders.scala. This saves one object allocation and allows to have finer-grained synchronization.

[lrytz]

How does the translation decide whether a holder is needed or not?

Scalac always generates a local variable for the bitmap. In the example, the bitmap is transformed into a VolatileByteRef in lambdalift, which comes later in the pipeline.

[DarkDimius]

@lrytz, for local vals, it's generated unconditionally.

[lrytz]

So you rely on the optimizer / linker / hotspot to inline the box?