summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/compiler/scala/tools/nsc/ast/TreeGen.scala21
-rw-r--r--src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala11
-rw-r--r--src/compiler/scala/tools/nsc/transform/Fields.scala37
-rw-r--r--test/files/run/delambdafy_t6028.check14
-rw-r--r--test/files/run/t6028.check14
5 files changed, 51 insertions, 46 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!
diff --git a/test/files/run/delambdafy_t6028.check b/test/files/run/delambdafy_t6028.check
index b188928c09..7bd68c78e9 100644
--- a/test/files/run/delambdafy_t6028.check
+++ b/test/files/run/delambdafy_t6028.check
@@ -43,11 +43,15 @@ package <empty> {
<synthetic> <stable> <artifact> def $outer(): T = MethodLocalObject$2.this.$outer
};
final <stable> private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
- T.this.synchronized({
- if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
- MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
- scala.runtime.BoxedUnit.UNIT
- });
+ if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+ {
+ T.this.synchronized({
+ if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+ MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
+ scala.runtime.BoxedUnit.UNIT
+ });
+ ()
+ };
MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
};
final <artifact> private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try {
diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check
index c2e3ca58d8..f757bc93ff 100644
--- a/test/files/run/t6028.check
+++ b/test/files/run/t6028.check
@@ -55,11 +55,15 @@ package <empty> {
<synthetic> <stable> <artifact> def $outer(): T = MethodLocalObject$2.this.$outer
};
final <stable> private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
- T.this.synchronized({
- if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
- MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
- scala.runtime.BoxedUnit.UNIT
- });
+ if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+ {
+ T.this.synchronized({
+ if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+ MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
+ scala.runtime.BoxedUnit.UNIT
+ });
+ ()
+ };
MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
};
@SerialVersionUID(value = 0) final <synthetic> class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable {
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-30 19:14:20 +0000