Skip to content

Commit 61701c2

Browse files
authored
Merge pull request scala#8396 from retronym/topic/reflection-thread-safety-tycon
A pair of thread safety fixes for runtime reflection.
2 parents 3797ff9 + 67e4d95 commit 61701c2

File tree

5 files changed

+110
-5
lines changed

5 files changed

+110
-5
lines changed

src/reflect/mima-filters/2.12.0.backwards.excludes

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ ProblemFilters.exclude[MissingTypesProblem]("scala.reflect.runtime.JavaUniverse"
1818
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.reflect.io.ZipArchive.close")
1919

2020
ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.reflect.io.ZipArchive.getDir")
21-
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.reflect.io.FileZipArchive.allDirs")
21+
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.reflect.io.FileZipArchive.allDirs")
22+
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.reflect.runtime.SynchronizedSymbols#SynchronizedSymbol.scala$reflect$runtime$SynchronizedSymbols$SynchronizedSymbol$$super$typeConstructor")

src/reflect/mima-filters/2.12.0.forwards.excludes

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,5 @@ ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.api.Universe.T
4141
ProblemFilters.exclude[MissingClassProblem]("scala.reflect.macros.Attachments$")
4242

4343
ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.reflect.io.ZipArchive.getDir")
44-
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.reflect.io.FileZipArchive.allDirs")
44+
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.reflect.io.FileZipArchive.allDirs")
45+
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.runtime.SynchronizedSymbols#SynchronizedSymbol.typeConstructor")

src/reflect/scala/reflect/internal/Types.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,11 @@ trait Types
111111
private object substTypeMapCache {
112112
private[this] var cached: SubstTypeMap = new SubstTypeMap(Nil, Nil)
113113

114-
def apply(from: List[Symbol], to: List[Type]): SubstTypeMap = {
114+
def apply(from: List[Symbol], to: List[Type]): SubstTypeMap = if (isCompilerUniverse) {
115115
if ((cached.from ne from) || (cached.to ne to))
116116
cached = new SubstTypeMap(from, to)
117-
118117
cached
119-
}
118+
} else new SubstTypeMap(from, to)
120119
}
121120

122121
/** The current skolemization level, needed for the algorithms

src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ private[reflect] trait SynchronizedSymbols extends internal.Symbols { self: Symb
160160
override def exists: Boolean = gilSynchronizedIfNotThreadsafe(super.exists)
161161
override def typeSignature: Type = gilSynchronizedIfNotThreadsafe { super.typeSignature }
162162
override def typeSignatureIn(site: Type): Type = gilSynchronizedIfNotThreadsafe { super.typeSignatureIn(site) }
163+
override def typeConstructor: Type = gilSynchronizedIfNotThreadsafe { super.typeConstructor }
163164

164165
override def typeParams: List[Symbol] = gilSynchronizedIfNotThreadsafe {
165166
if (isCompilerUniverse) super.typeParams
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Scala (https://www.scala-lang.org)
3+
*
4+
* Copyright EPFL and Lightbend, Inc.
5+
*
6+
* Licensed under Apache License 2.0
7+
* (http://www.apache.org/licenses/LICENSE-2.0).
8+
*
9+
* See the NOTICE file distributed with this work for
10+
* additional information regarding copyright ownership.
11+
*/
12+
13+
package scala.reflect.runtime
14+
15+
import java.util.concurrent.{Callable, Executors}
16+
17+
import org.junit.Test
18+
19+
class ThreadSafetyTest {
20+
import scala.reflect.runtime.universe._
21+
sealed abstract class Instance(tp: Type)
22+
final case class ListInstance(tp: Type, elemInstance: Instance) extends Instance(appliedType(symbolOf[List[_]], tp :: Nil))
23+
24+
final case class DoubleInstance() extends Instance(typeOf[Double])
25+
26+
final case class StringInstance() extends Instance(typeOf[String])
27+
28+
final case class LowPriorityInstance(tpe: Type) extends Instance(tpe)
29+
30+
def classKeyOf[T: TypeTag]: String = {
31+
classKeyOf(typeOf[T])
32+
}
33+
def classKeyOf(tpe: Type): String = {
34+
tpe.typeSymbol.fullName
35+
}
36+
37+
class Lazy[T](thunk: () => T) {
38+
lazy val force: T = thunk()
39+
}
40+
41+
class Registry {
42+
private val cache = new java.util.concurrent.ConcurrentHashMap[Type, Lazy[Instance]]()
43+
def instance[T: TypeTag]: Lazy[Instance] = {
44+
instance(typeOf[T])
45+
}
46+
def instance(tpe: Type): Lazy[Instance] = {
47+
val value = keyOf(tpe)
48+
assert(value =:= tpe, (value, tpe))
49+
cache.computeIfAbsent(value, create(_))
50+
}
51+
52+
private def create(tpe: Type): Lazy[Instance] = {
53+
val key = classKeyOf(tpe)
54+
if (key == "scala.collection.immutable.List") {
55+
new Lazy(() => {val elemTpe = tpe.dealias.typeArgs.head; ListInstance(tpe.dealias, instance(elemTpe).force)})
56+
} else if (key == "scala.Double") {
57+
new Lazy(() => DoubleInstance())
58+
} else if (key == "java.lang.String") {
59+
new Lazy(() => StringInstance())
60+
} else {
61+
new Lazy(() => LowPriorityInstance(tpe))
62+
}
63+
}
64+
private def keyOf(tp: Type): universe.Type = {
65+
tp.map(_.dealias)
66+
}
67+
}
68+
69+
@Test
70+
def test(): Unit = {
71+
val executor = Executors.newFixedThreadPool(16)
72+
for (i <- (0 to 128)) {
73+
val registry = new Registry
74+
val is = List(
75+
(() => typeOf[List[Double]], "ListInstance(List[Double],DoubleInstance())"),
76+
(() => typeOf[List[String]], "ListInstance(List[String],StringInstance())"),
77+
(() => typeOf[List[List[Double]]], "ListInstance(List[List[Double]],ListInstance(List[Double],DoubleInstance()))"),
78+
(() => typeOf[List[List[List[Double]]]], "ListInstance(List[List[List[Double]]],ListInstance(List[List[Double]],ListInstance(List[Double],DoubleInstance())))"),
79+
(() => typeOf[List[List[List[Object]]]], "ListInstance(List[List[List[Object]]],ListInstance(List[List[Object]],ListInstance(List[Object],LowPriorityInstance(Object))))")
80+
)
81+
sealed abstract class Result
82+
case object Okay extends Result
83+
case class Failed(i: Int, tp: Type, expected: String, instance: String) extends Result
84+
def check(i: Int): Result = {
85+
val (f, expected) = is(i % is.size)
86+
val tp = f()
87+
val instance = registry.instance(tp).force
88+
if (instance.toString == expected) Okay else Failed(i, tp, expected, "[" + instance + "]")
89+
}
90+
val par = true
91+
val fs: List[Result] = if (par) Array.tabulate(32)(i =>
92+
executor.submit(new Callable[Result] {
93+
override def call(): Result = {
94+
check(i % is.size)
95+
}
96+
})).map(_.get).toList
97+
else is.indices.map(check).toList
98+
val fails = fs.filter(_ != Okay)
99+
assert(fails.isEmpty, "iteration " + i + ": " + fails.mkString("\n"))
100+
}
101+
executor.shutdownNow()
102+
}
103+
}

0 commit comments

Comments
 (0)