8000 SI-10026 Fix endless cycle in runtime reflection · SethTisue/scala@777a0e5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 777a0e5

Browse files
retronymretronym
committed
SI-10026 Fix endless cycle in runtime reflection
56f23af introduced a call to `baseTypeSeq` of `scala.collection.mutable.ArrayOps.ofRef[?T]{}` in `findMember`. This exposed a latent bug in the synchronized wrapper of `BaseTypeSeq`, demonstrated below with an older version of Scala: ``` Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_112). Type in expressions for evaluation. Or try :help. scala> val symtab = reflect.runtime.universe.asInstanceOf[scala.reflect.internal.SymbolTable] symtab: scala.reflect.internal.SymbolTable = scala.reflect.runtime.JavaUniverse@458544e0 scala> import symtab._ import symtab._ scala> val ArrayOps_ofRef_Class = symtab.symbolOf[scala.collection.mutable.ArrayOps.ofRef[AnyRef]] ArrayOps_ofRef_Class: symtab.TypeSymbol = class ofRef scala> appliedType(symbolOf[Set[Any]], symbolOf[Set[Any]].typeParams.map(TypeVar(_))) res2: symtab.Type = Set[?A] scala> .narrow res3: symtab.Type = <none>.<refinement>.type scala> .baseTypeSeq java.lang.StackOverflowError at scala.reflect.runtime.Gil$class.gilSynchronized(Gil.scala:21) at scala.reflect.runtime.JavaUniverse.gilSynchronized(JavaUniverse.scala:16) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$class.map(SynchronizedOps.scala:27) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anon$2.map(SynchronizedOps.scala:34) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$class. 8000 lateMap(SynchronizedOps.scala:34) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anon$2.lateMap(SynchronizedOps.scala:34) at scala.reflect.internal.BaseTypeSeqs$MappedBaseTypeSeq.map(BaseTypeSeqs.scala:235) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anon$2.scala$reflect$runtime$SynchronizedOps$SynchronizedBaseTypeSeq$$super$map(SynchronizedOps.scala:34) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anonfun$map$1.apply(SynchronizedOps.scala:27) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anonfun$map$1.apply(SynchronizedOps.scala:27) at scala.reflect.runtime.Gil$class.gilSynchronized(Gil.scala:19) at scala.reflect.runtime.JavaUniverse.gilSynchronized(JavaUniverse.scala:16) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$class.map(SynchronizedOps.scala:27) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anon$2.map(SynchronizedOps.scala:34) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$class.lateMap(SynchronizedOps.scala:34) at scala.reflect.runtime.SynchronizedOps$SynchronizedBaseTypeSeq$$anon$2.lateMap(SynchronizedOps.scala:34) at scala.reflect.internal.BaseTypeSeqs$MappedBaseTypeSeq.map(BaseTypeSeqs.scala:235) ``` The infinite cycle involves: ``` class MappedBaseTypeSeq(orig: BaseTypeSeq, f: Type => Type) extends BaseTypeSeq(orig.parents map f, orig.elems) { ... override def map(g: Type => Type) = lateMap(g) override def lateMap(g: Type => Type) = orig.lateMap(x => g(f(x))) } trait SynchronizedBaseTypeSeq extends BaseTypeSeq { ... override def map(f: Type => Type): BaseTypeSeq = gilSynchronized { super.map(f) } override def lateMap(f: Type => Type): BaseTypeSeq = // only need to synchronize BaseTypeSeqs if they contain refined types if (map(f).toList.exists(_.isInstanceOf[RefinedType])) new MappedBaseTypeSeq(this, f) with SynchronizedBaseTypeSeq else new MappedBaseTypeSeq(this, f) } ``` This commit creates a new factory method for `MappedBaseTypeSeq`-s to break the cycle. As an independent change, I have also removed the attempt to conditionally synchronize them, as the condition was eagerly applying the map function (and throwing away the result!). I've appeased MiMa with new whitelist entries, but I'm confident that this is deep enough in the bowels of our runtime reflection implementation that there is no way that user code will be calling these methods directly.
1 parent 147e5dd commit 777a0e5

File tree

6 files changed

+38
-6
lines changed

6 files changed

+38
-6
lines changed

‎bincompat-backward.whitelist.conf

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ filter {
3333
matchName="scala.sys.process.ProcessImpl#CompoundProcess.getExitValue"
3434
problemName=DirectMissingMethodProblem
3535
},
36+
{
37+
matchName="scala.reflect.runtime.SynchronizedOps.scala$reflect$runtime$SynchronizedOps$$super$newMappedBaseTypeSeq"
38+
problemName=ReversedMissingMethodProblem
39+
},
40+
{
41+
matchName="scala.reflect.runtime.SynchronizedOps#SynchronizedBaseTypeSeq.lateMap"
42+
problemName=DirectMissingMethodProblem
43+
},
3644
{
3745
matchName="scala.collection.immutable.HashMap.contains0"
3846
problemName=DirectMissingMethodProblem

‎bincompat-forward.whitelist.conf

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ filter {
4343
matchName="scala.util.hashing.MurmurHash3.wrappedArrayHash"
4444
problemName=DirectMissingMethodProblem
4545
},
46+
{
47+
matchName="scala.reflect.runtime.SynchronizedOps.newMappedBaseTypeSeq"
48+
problemName=DirectMissingMethodProblem
49+
},
50+
{
51+
matchName="scala.reflect.runtime.JavaUniverse.newMappedBaseTypeSeq"
52+
problemName=DirectMissingMethodProblem
53+
},
4654
{
4755
matchName="scala.collection.immutable.HashMap.contains0"
4856
problemName=DirectMissingMethodProblem

‎src/reflect/scala/reflect/internal/BaseTypeSeqs.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ trait BaseTypeSeqs {
3333
protected def newBaseTypeSeq(parents: List[Type], elems: Array[Type]) =
3434
new BaseTypeSeq(parents, elems)
3535

36+
protected def newMappedBaseTypeSeq(orig: BaseTypeSeq, f: Type => Type) =
37+
new MappedBaseTypeSeq(orig, f)
38+
3639
/** Note: constructor is protected to force everyone to use the factory method newBaseTypeSeq instead.
3740
* This is necessary because when run from reflection every base type sequence needs to have a
3841
* SynchronizedBaseTypeSeq as mixin.
@@ -125,7 +128,7 @@ trait BaseTypeSeqs {
125128
newBaseTypeSeq(parents, arr)
126129
}
127130

128-
def lateMap(f: Type => Type): BaseTypeSeq = new MappedBaseTypeSeq(this, f)
131+
def lateMap(f: Type => Type): BaseTypeSeq = newMappedBaseTypeSeq(this, f)
129132

130133
def exists(p: Type => Boolean): Boolean = elems exists p
131134

‎src/reflect/scala/reflect/runtime/SynchronizedOps.scala

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ private[reflect] trait SynchronizedOps extends internal.SymbolTable
1818
if (elems.exists(_.isInstanceOf[RefinedType])) new BaseTypeSeq(parents, elems) with SynchronizedBaseTypeSeq
1919
else new BaseTypeSeq(parents, elems)
2020

21+
override protected def newMappedBaseTypeSeq(orig: BaseTypeSeq, f: Type => Type) =
22+
// MappedBaseTypeSeq's are used rarely enough that we unconditionally mixin the synchronized
23+
// wrapper, rather than doing this conditionally. A previous attempt to do that broke the "late"
24+
// part of the "lateMap" contract in inspecting the mapped elements.
25+
new MappedBaseTypeSeq(orig, f) with SynchronizedBaseTypeSeq
26+
2127
trait SynchronizedBaseTypeSeq extends BaseTypeSeq {
2228
override def apply(i: Int): Type = gilSynchronized { super.apply(i) }
2329
override def rawElem(i: Int) = gilSynchronized { super.rawElem(i) }
@@ -28,11 +34,6 @@ private[reflect] trait SynchronizedOps extends internal.SymbolTable
2834
override def exists(p: Type => Boolean): Boolean = gilSynchronized { super.exists(p) }
2935
override lazy val maxDepth = gilSynchronized { maxDepthOfElems }
3036
override def toString = gilSynchronized { super.toString }
31-
32-
override def lateMap(f: Type => Type): BaseTypeSeq =
33-
// only need to synchronize BaseTypeSeqs if they contain refined types
34-
if (map(f).toList.exists(_.isInstanceOf[RefinedType])) new MappedBaseTypeSeq(this, f) with SynchronizedBaseTypeSeq
35-
else new MappedBaseTypeSeq(this, f)
3637
}
3738

3839
// Scopes

‎test/files/run/t10026.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
List(1, 2, 3)

‎test/files/run/t10026.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import scala.reflect.runtime.universe
2+
import scala.tools.reflect.ToolBox
3+
4+
object Test {
5+
def main(args: Array[String]): Unit = {
6+
val classloader = getClass.getClassLoader
7+
val toolbox = universe.runtimeMirror(classloader).mkToolBox()
8+
println(toolbox.compile(toolbox.parse("Array(1, 2, 3).toList")).apply())
9+
}
10+
}
11+

0 commit comments

Comments
 (0)
0