Make local lazy values thread-safe.

is now guaranteed to be initialized at most once, even when accessed
from different threads. Closes #3007, review by odersky.

5 files changed, 59 insertions, 32 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index 1327781d31..5044105684 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -13,8 +13,7 @@ import symtab.SymbolTable
 /** XXX to resolve: TreeGen only assumes global is a SymbolTable, but
  *  TreeDSL at the moment expects a Global.  Can we get by with SymbolTable?
  */
-abstract class TreeGen
-{
+abstract class TreeGen {
   val global: SymbolTable
 
   import global._
@@ -397,4 +396,21 @@ abstract class TreeGen
     if (prefix.isEmpty) containing
     else Block(prefix, containing) setPos (prefix.head.pos union containing.pos)
   }
+
+  /** Return a double-checked locking idiom around the syncBody tree. It guards with 'cond' and
+   *  synchronizez on 'clazz.this'. Additional statements can be included after initialization,
+   *  (outside the synchronized block).
+   *
+   *  The idiom works only if the condition is using a volatile field.
+   *  @see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
+   */
+  def mkDoubleCheckedLocking(clazz: Symbol, cond: Tree, syncBody: List[Tree], stats: List[Tree]): Tree = {
+    If(cond,
+       Block(
+         mkSynchronized(
+           mkAttributedThis(clazz),
+           If(cond, Block(syncBody: _*), EmptyTree)) ::
+         stats: _*),
+       EmptyTree)
+  }
 }
diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala
index 8db6958d07..35db3c0984 100644
--- a/src/compiler/scala/tools/nsc/ast/Trees.scala
+++ b/src/compiler/scala/tools/nsc/ast/Trees.scala
@@ -291,6 +291,14 @@ trait Trees extends reflect.generic.Trees { self: SymbolTable =>
   def Ident(sym: Symbol): Ident =
     Ident(sym.name) setSymbol sym
 
+  /** Block factory that flattens directly nested blocks.
+   */
+  def Block(stats: Tree*): Block = stats match {
+    case Seq(b @ Block(_, _)) => b
+    case Seq(stat) => Block(stats.toList, Literal(Constant(())))
+    case Seq(_, rest @ _*) => Block(stats.init.toList, stats.last)
+  }
+
   /** A synthetic term holding an arbitrary type.  Not to be confused with
     * with TypTree, the trait for trees that are only used for type trees.
     * TypeTree's are inserted in several places, but most notably in
diff --git a/src/compiler/scala/tools/nsc/transform/LazyVals.scala b/src/compiler/scala/tools/nsc/transform/LazyVals.scala
index 4cadc66af1..ec283f67fd 100644
--- a/src/compiler/scala/tools/nsc/transform/LazyVals.scala
+++ b/src/compiler/scala/tools/nsc/transform/LazyVals.scala
@@ -4,7 +4,7 @@ package transform;
 import scala.tools.nsc._
 import scala.collection.mutable.HashMap
 
-abstract class LazyVals extends Transform with ast.TreeDSL {
+abstract class LazyVals extends Transform with TypingTransformers with ast.TreeDSL {
   // inherits abstract value `global' and class `Phase' from Transform
 
   import global._                  // the global environment
@@ -17,18 +17,10 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
   def newTransformer(unit: CompilationUnit): Transformer =
     new LazyValues(unit)
 
-  /** Create a new phase which applies transformer */
-  override def newPhase(prev: scala.tools.nsc.Phase): StdPhase = new Phase(prev)
-
-  /** The phase defined by this transform */
-  class Phase(prev: scala.tools.nsc.Phase) extends StdPhase(prev) {
-    def apply(unit: global.CompilationUnit): Unit = newTransformer(unit) transformUnit unit
-  }
-
   /**
    * Transform local lazy accessors to check for the initialized bit.
    */
-  class LazyValues(unit: CompilationUnit) extends Transformer {
+  class LazyValues(unit: CompilationUnit) extends TypingTransformer(unit) {
     /** map from method symbols to the number of lazy values it defines. */
     private val lazyVals = new HashMap[Symbol, Int] {
       override def default(meth: Symbol) = 0
@@ -47,8 +39,10 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
      */
     override def transform(tree: Tree): Tree = {
       val sym = tree.symbol
+      curTree = tree
+
       tree match {
-        case DefDef(mods, name, tparams, vparams, tpt, rhs) =>
+        case DefDef(mods, name, tparams, vparams, tpt, rhs) => atOwner(tree.symbol) {
           val res = if (!sym.owner.isClass && sym.hasFlag(LAZY)) {
             val enclosingDummyOrMethod =
               if (sym.enclMethod == NoSymbol) sym.owner else sym.enclMethod
@@ -61,8 +55,9 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
             super.transform(rhs)
           treeCopy.DefDef(tree, mods, name, tparams, vparams, tpt,
                           typed(addBitmapDefs(sym, res)))
+        }
 
-        case Template(parents, self, body) =>
+        case Template(parents, self, body) => atOwner(currentOwner) {
           val body1 = super.transformTrees(body)
           var added = false
           val stats =
@@ -76,6 +71,7 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
                 stat
             }
           treeCopy.Template(tree, parents, self, stats)
+        }
 
         case _ => super.transform(tree)
       }
@@ -95,7 +91,7 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
 
       val bmps = bitmaps(methSym) map (ValDef(_, ZERO))
 
-      def isMatch(params: List[Ident]) = (params.tail corresponds methSym.tpe.params)(_.tpe == _.tpe) // @PP: corresponds
+      def isMatch(params: List[Ident]) = (params.tail corresponds methSym.tpe.params)(_.tpe == _.tpe)
 
       if (bmps.isEmpty) rhs else rhs match {
         case Block(assign, l @ LabelDef(name, params, rhs1))
@@ -145,8 +141,10 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
       }
       assert(res != UNIT || meth.tpe.finalResultType.typeSymbol == UnitClass)
 
-      atPos(tree.pos)(typed {
-        def body = { IF ((Ident(bitmapSym) INT_& mask) INT_== ZERO) THEN block ENDIF }
+      val cond = (Ident(bitmapSym) INT_& mask) INT_== ZERO
+
+      atPos(tree.pos)(localTyper.typed {
+        def body = gen.mkDoubleCheckedLocking(meth.enclClass, cond, List(block), Nil)
         BLOCK(body, res)
       })
     }
@@ -169,6 +167,10 @@ abstract class LazyVals extends Transform with ast.TreeDSL {
         bmps(n)
       else {
         val sym = meth.newVariable(meth.pos, nme.bitmapName(n)).setInfo(IntClass.tpe)
+        atPhase(currentRun.typerPhase) {
+          sym addAnnotation AnnotationInfo(VolatileAttr.tpe, Nil, Nil)
+        }
+
         bitmaps(meth) = (sym :: bmps).reverse
         sym
       }
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index a9810b2217..c228ee0e46 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -636,7 +636,10 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         else              lhs INT_!= ZERO
       }
 
-      /** return a 'lazified' version of rhs.
+      /** return a 'lazified' version of rhs. It uses double-checked locking to ensure
+       *  initialization is performed at most once. Private fields used only in this
+       *  initializer are subsequently set to null.
+       *
        *  @param clazz The class symbol
        *  @param init The tree which initializes the field ( f = <rhs> )
        *  @param fieldSym The symbol of this lazy field
@@ -647,11 +650,13 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
        *    if ((bitmap$n & MASK) == 0) {
        *       synchronized(this) {
        *         if ((bitmap$n & MASK) == 0) {
-       *           synchronized(this) {
-       *             init // l$ = <rhs>
-       *           }
+       *           init // l$ = <rhs>
        *           bitmap$n = bimap$n | MASK
-       *         }}}
+       *         }
+       *       }
+       *       this.f1 = null
+       *       ... this.fn = null
+       *    }
        *    l$
        *  }
        *  where bitmap$n is an int value acting as a bitmap of initialized values. It is
@@ -671,14 +676,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         def syncBody  = init ::: List(mkSetFlag(clazz, offset), UNIT)
 
         log("nulling fields inside " + lzyVal + ": " + nulls)
-        val result    =
-          IF (cond) THEN BLOCK(
-            (gen.mkSynchronized(
-              gen mkAttributedThis clazz,
-              IF (cond) THEN BLOCK(syncBody: _*) ENDIF
-            )
-            :: nulls): _*) ENDIF
-
+        val result    = gen.mkDoubleCheckedLocking(clazz, cond, syncBody, nulls)
         typedPos(init.head.pos)(BLOCK(result, retVal))
       }
 
diff --git a/src/compiler/scala/tools/nsc/transform/TypingTransformers.scala b/src/compiler/scala/tools/nsc/transform/TypingTransformers.scala
index 8efbb356e8..6656b79d26 100644
--- a/src/compiler/scala/tools/nsc/transform/TypingTransformers.scala
+++ b/src/compiler/scala/tools/nsc/transform/TypingTransformers.scala
@@ -17,8 +17,11 @@ trait TypingTransformers {
   import global._
 
   abstract class TypingTransformer(unit: CompilationUnit) extends Transformer {
-    var localTyper: analyzer.Typer = analyzer.newTyper(
-      analyzer.rootContext(unit, EmptyTree, true))
+    var localTyper: analyzer.Typer =
+      if (phase.erasedTypes)
+        erasure.newTyper(erasure.rootContext(unit, EmptyTree, true)).asInstanceOf[analyzer.Typer]
+      else
+        analyzer.newTyper(analyzer.rootContext(unit, EmptyTree, true))
     protected var curTree: Tree = _
     protected def typedPos(pos: Position)(tree: Tree) = localTyper typed { atPos(pos)(tree) }