diff options
author | Adriaan Moors <adriaan.moors@typesafe.com> | 2016-08-22 11:43:49 -0700 |
---|---|---|
committer | Adriaan Moors <adriaan@lightbend.com> | 2016-08-29 09:54:09 +0200 |
commit | d9974be376e23f2a41373cd85977732aee6761a1 (patch) | |
tree | 9710c040412319d5217408bbef35ec797a6b3ceb /src | |
parent | 6f7bd990ae69d6796c68894133c1975bef354e12 (diff) | |
download | scala-d9974be376e23f2a41373cd85977732aee6761a1.tar.gz scala-d9974be376e23f2a41373cd85977732aee6761a1.tar.bz2 scala-d9974be376e23f2a41373cd85977732aee6761a1.zip |
Double-checked locking for modules.
Inline `mkSynchronizedCheck`, whose abstraction obscured rather than clarified.
Diffstat (limited to 'src')
-rw-r--r-- | src/compiler/scala/tools/nsc/ast/TreeGen.scala | 21 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala | 11 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/Fields.scala | 37 |
3 files changed, 33 insertions, 36 deletions
diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala index ac47b3c464..762456c9c9 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala @@ -91,7 +91,7 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL { ) /** Make a synchronized block on 'monitor'. */ - def mkSynchronized(monitor: Tree, body: Tree): Tree = + def mkSynchronized(monitor: Tree)(body: Tree): Tree = Apply(Select(monitor, Object_synchronized), List(body)) def mkAppliedTypeForCase(clazz: Symbol): Tree = { @@ -233,25 +233,6 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL { else Block(prefix, containing) setPos (prefix.head.pos union containing.pos) } - /** Return the synchronized part of the double-checked locking idiom around the syncBody tree. It guards with `cond` and - * synchronizes on `attrThis`. 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 - * - * TODO: update to state of the art on java 8 (https://shipilev.net/blog/2014/safe-public-construction/) - */ - def mkSynchronizedCheck(attrThis: Tree, cond: Tree, syncBody: List[Tree], stats: List[Tree]): Tree = { - def blockOrStat(stats: List[Tree]): Tree = stats match { - case head :: Nil => head - case _ => Block(stats : _*) - } - val sync = mkSynchronized(attrThis, If(cond, blockOrStat(syncBody), EmptyTree)) - blockOrStat(sync :: stats) - } - /** Creates a tree representing new Object { stats }. * To make sure an anonymous subclass of Object is created, * if there are no stats, a () is added. diff --git a/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala b/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala index c3bcf69205..120ee5c26e 100644 --- a/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala +++ b/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala @@ -329,7 +329,7 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL { def expandLazyClassMember(lazyVar: Symbol, lazyAccessor: Symbol, transformedRhs: Tree, nullables: Map[Symbol, List[Symbol]]): Tree = { // use cast so that specialization can turn null.asInstanceOf[T] into null.asInstanceOf[Long] def nullify(sym: Symbol) = - Select(thisRef, sym.accessedOrSelf) === gen.mkAsInstanceOf(NULL, sym.info.resultType) + Select(thisRef, sym.accessedOrSelf) === gen.mkAsInstanceOf(NULL, sym.info.resultType) val nulls = nullables.getOrElse(lazyAccessor, Nil) map nullify @@ -343,13 +343,14 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL { val selectVar = if (isUnit) UNIT else Select(thisRef, lazyVar) val storeRes = if (isUnit) rhsAtSlowDef else Assign(selectVar, rhsAtSlowDef) - val synchedStats = storeRes :: mkSetFlag(lazyAccessor) :: Nil - val slowPathRhs = - Block(List(gen.mkSynchronizedCheck(thisRef, mkTest(lazyAccessor), synchedStats, nulls)), selectVar) + def needsInit = mkTest(lazyAccessor) + val doInit = Block(List(storeRes), mkSetFlag(lazyAccessor)) + // the slow part of double-checked locking (TODO: is this the most efficient pattern? https://github.come/scala/scala-dev/issues/204) + val slowPathRhs = Block(gen.mkSynchronized(thisRef)(If(needsInit, doInit, EmptyTree)) :: nulls, selectVar) // The lazy accessor delegates to the compute method if needed, otherwise just accesses the var (it was initialized previously) // `if ((bitmap&n & MASK) == 0) this.l$compute() else l$` - val accessorRhs = If(mkTest(lazyAccessor), Apply(Select(thisRef, slowPathSym), Nil), selectVar) + val accessorRhs = If(needsInit, Apply(Select(thisRef, slowPathSym), Nil), selectVar) afterOwnPhase { // so that we can assign to vals Thicket(List((DefDef(slowPathSym, slowPathRhs)), DefDef(lazyAccessor, accessorRhs)) map typedPos(lazyAccessor.pos.focus)) diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index cf8c25afb6..217c4cb5e3 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -189,10 +189,29 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor // for local modules, we synchronize on the owner of the method that owns the module val monitorHolder = This(moduleVar.owner.enclClass) - val cond = Apply(Select(moduleVarRef, Object_eq), List(CODE.NULL)) + def needsInit = Apply(Select(moduleVarRef, Object_eq), List(CODE.NULL)) val init = Assign(moduleVarRef, gen.newModule(module, moduleVar.info)) - Block(List(gen.mkSynchronizedCheck(monitorHolder, cond, List(init), Nil)), moduleVarRef) + /** double-checked locking following https://shipilev.net/blog/2014/safe-public-construction/#_safe_publication + * + * public class SafeDCLFactory { + * private volatile Singleton instance; + * + * public Singleton get() { + * if (instance == null) { // check 1 + * synchronized(this) { + * if (instance == null) { // check 2 + * instance = new Singleton(); + * } + * } + * } + * return instance; + * } + * } + * + * TODO: optimize using local variable? + */ + Block(If(needsInit, gen.mkSynchronized(monitorHolder)(If(needsInit, init, EmptyTree)), EmptyTree) :: Nil, moduleVarRef) } // NoSymbol for lazy accessor sym with unit result type @@ -523,16 +542,12 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val valueSetter = if (isUnit) NoSymbol else valueGetter.setterIn(refClass) val setValue = if (isUnit) rhs else Apply(Select(Ident(holderSym), valueSetter), rhs :: Nil) val getValue = if (isUnit) UNIT else Apply(Select(Ident(holderSym), valueGetter), Nil) - gen.mkSynchronized(Ident(holderSym), - Block(List( - If(NOT(Ident(holderSym) DOT initializedGetter), - Block(List( - setInitialized), - setValue), - EmptyTree)), - getValue)) // must read the value within the synchronized block since it's not volatile + + // must read the value within the synchronized block since it's not volatile // (there's no happens-before relation with the read of the volatile initialized field) - // TODO: double-checked locking + // TODO: double-checked locking (https://github.come/scala/scala-dev/issues/204) + gen.mkSynchronized(Ident(holderSym))( + If(Ident(holderSym) DOT initializedGetter, getValue, Block(List(setValue, setInitialized), getValue))) } // do last! |