Skip to content

Commit 981da8e

Browse files
committed
cleans up initialization of runtime reflection
At first I just tried to remove syntheticCoreClasses from missingHook and put them into the initializer of freshly created mirrors in order to reduce the non-determinism in mutations of the global symbol table. And then it didn't work, crashing on me claiming that AnyRef is missing. Apparently we still need AnyRefClass in missingHook, just because it's impossible to initialize (i.e. unpickle) ScalaPackageClass without it. And then it still didn't work, whining about multiple overloaded defs of some synthetic symbols. That was really tricky, but I figured it out as well by initializing ScalaPackageClass first before forcing any synthetic symbols (see the details in comments).
1 parent b2c2493 commit 981da8e

File tree

7 files changed

+59
-29
lines changed

7 files changed

+59
-29
lines changed

src/reflect/scala/reflect/internal/Definitions.scala

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1248,8 +1248,24 @@ trait Definitions extends api.StandardDefinitions {
12481248

12491249
def init() {
12501250
if (isInitialized) return
1251+
// forcing ScalaPackageClass first thing during startup is important, because syntheticCoreClasses such as AnyRefClass
1252+
// need to be entered into ScalaPackageClass, which entails calling ScalaPackageClass.info.decls.enter
1253+
// if ScalaPackageClass isn't initialized by that moment, the following will happen for runtime reflection:
1254+
// 1) initialization of ScalaPackageClass will trigger unpickling
1255+
// 2) unpickling will need to load some auxiliary types such as, for example, String
1256+
// 3) to load String, runtime reflection will call mirrorDefining(classOf[String])
1257+
// 4) this, in turn, will call runtimeMirror(classOf[String].getClassLoader)
1258+
// 5) for some classloader configurations, the resulting mirror will be different from rootMirror
1259+
// 6) in that case, initialization of the resulting mirror will try to import definitions.syntheticCoreClasses into the mirror
1260+
// 7) this will force all the lazy vals corresponding to syntheticCoreClasses
1261+
// 8) by that time, the completer of ScalaPackageClass will have already called setInfo on ScalaPackageClass, so there won't be any stack overflow
1262+
// so far so good, but there's a subtle detail. if forcing of ScalaPackageClass was called by a syntheticCoreClasses lazy val,
1263+
// then this lazy val will be entered twice: once during step 7 and once when returning from the original call
1264+
// to avoid this we need to initialize ScalaPackageClass first and only then synthesize the core classes
1265+
ScalaPackageClass.initialize
12511266
// force initialization of every symbol that is synthesized or hijacked by the compiler
1252-
val forced = symbolsNotPresentInBytecode
1267+
val forced1 = symbolsNotPresentInBytecode
1268+
val forced2 = NoSymbol
12531269
isInitialized = true
12541270
} //init
12551271

src/reflect/scala/reflect/internal/Mirrors.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,19 @@ trait Mirrors extends api.Mirrors {
239239

240240
RootClass.info.decls enter EmptyPackage
241241
RootClass.info.decls enter RootPackage
242+
243+
if (rootOwner != NoSymbol) {
244+
// synthetic core classes are only present in root mirrors
245+
// because Definitions.scala, which initializes and enters them, only affects rootMirror
246+
// therefore we need to enter them manually for non-root mirrors
247+
definitions.syntheticCoreClasses foreach (theirSym => {
248+
val theirOwner = theirSym.owner
249+
assert(theirOwner.isPackageClass, s"theirSym = $theirSym, theirOwner = $theirOwner")
250+
val ourOwner = staticPackage(theirOwner.fullName)
251+
val ourSym = theirSym // just copy the symbol into our branch of the symbol table
252+
ourOwner.info.decls enter ourSym
253+
})
254+
}
242255
}
243256
}
244257

src/reflect/scala/reflect/internal/StdNames.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ trait StdNames {
265265
final val SourceFileATTR: NameType = "SourceFile"
266266
final val SyntheticATTR: NameType = "Synthetic"
267267

268+
final val scala_ : NameType = "scala"
269+
268270
def dropSingletonName(name: Name): TypeName = (name dropRight SINGLETON_SUFFIX.length).toTypeName
269271
def singletonName(name: Name): TypeName = (name append SINGLETON_SUFFIX).toTypeName
270272
def implClassName(name: Name): TypeName = (name append IMPL_CLASS_SUFFIX).toTypeName

src/reflect/scala/reflect/internal/Symbols.scala

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3208,10 +3208,11 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
32083208
def name = nme.NO_NAME
32093209
override def name_=(n: Name) = abort("Cannot set NoSymbol's name to " + n)
32103210

3211-
synchronized {
3212-
setInfo(NoType)
3213-
privateWithin = this
3214-
}
3211+
// Syncnote: no need to synchronize this, because NoSymbol's initialization is triggered by JavaUniverse.init
3212+
// which is called in universe's constructor - something that's inherently single-threaded
3213+
setInfo(NoType)
3214+
privateWithin = this
3215+
32153216
override def info_=(info: Type) = {
32163217
infos = TypeHistory(1, NoType, null)
32173218
unlock()

src/reflect/scala/reflect/runtime/JavaMirrors.scala

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,6 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
4646

4747
trait JavaClassCompleter extends FlagAssigningCompleter
4848

49-
def init() = {
50-
definitions.AnyValClass // force it.
51-
52-
// establish root association to avoid cyclic dependency errors later
53-
rootMirror.classToScala(classOf[java.lang.Object]).initialize
54-
55-
// println("initializing definitions")
56-
definitions.init()
57-
}
58-
5949
def runtimeMirror(cl: ClassLoader): Mirror = mirrors get cl match {
6050
case Some(WeakReference(m)) => m
6151
case _ => createMirror(rootMirror.RootClass, cl)
@@ -1251,11 +1241,6 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
12511241
case _ => abort(s"${sym}.enclosingRootClass = ${sym.enclosingRootClass}, which is not a RootSymbol")
12521242
}
12531243

1254-
private lazy val syntheticCoreClasses: Map[(String, Name), Symbol] = {
1255-
def mapEntry(sym: Symbol): ((String, Name), Symbol) = (sym.owner.fullName, sym.name) -> sym
1256-
Map() ++ (definitions.syntheticCoreClasses map mapEntry)
1257-
}
1258-
12591244
/** 1. If `owner` is a package class (but not the empty package) and `name` is a term name, make a new package
12601245
* <owner>.<name>, otherwise return NoSymbol.
12611246
* Exception: If owner is root and a java class with given name exists, create symbol in empty package instead
@@ -1272,15 +1257,15 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
12721257
if (name.isTermName && !owner.isEmptyPackageClass)
12731258
return mirror.makeScalaPackage(
12741259
if (owner.isRootSymbol) name.toString else owner.fullName+"."+name)
1275-
syntheticCoreClasses get (owner.fullName, name) match {
1276-
case Some(tsym) =>
1277-
// synthetic core classes are only present in root mirrors
1278-
// because Definitions.scala, which initializes and enters them, only affects rootMirror
1279-
// therefore we need to enter them manually for non-root mirrors
1280-
if (mirror ne thisUniverse.rootMirror) owner.info.decls enter tsym
1281-
return tsym
1282-
case None =>
1283-
}
1260+
if (name == tpnme.AnyRef && owner.owner.isRoot && owner.name == tpnme.scala_)
1261+
// when we synthesize the scala.AnyRef symbol, we need to add it to the scope of the scala package
1262+
// the problem is that adding to the scope implies doing something like `owner.info.decls enter anyRef`
1263+
// which entails running a completer for the scala package
1264+
// which will try to unpickle the stuff in scala/package.class
1265+
// which will transitively load scala.AnyRef
1266+
// which doesn't exist yet, because it hasn't been added to the scope yet
1267+
// this missing hook ties the knot without introducing synchronization problems like before
1268+
return definitions.AnyRefClass
12841269
}
12851270
info("*** missing: "+name+"/"+name.isTermName+"/"+owner+"/"+owner.hasPackageFlag+"/"+owner.info.decls.getClass)
12861271
super.missingHook(owner, name)

src/reflect/scala/reflect/runtime/JavaUniverse.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,9 @@ class JavaUniverse extends internal.SymbolTable with ReflectSetup with runtime.S
2424
def newLazyTreeCopier: TreeCopier = new LazyTreeCopier
2525

2626
init()
27+
28+
def init() {
29+
definitions.init()
30+
}
2731
}
2832

src/reflect/scala/reflect/runtime/SymbolLoaders.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ private[reflect] trait SymbolLoaders { self: SymbolTable =>
7676
class PackageScope(pkgClass: Symbol) extends Scope(initFingerPrints = -1L) // disable fingerprinting as we do not know entries beforehand
7777
with SynchronizedScope {
7878
assert(pkgClass.isType)
79+
80+
// materializing multiple copies of the same symbol in PackageScope is a very popular bug
81+
// this override does its best to guard against it
82+
override def enter[T <: Symbol](sym: T): T = {
83+
val existing = super.lookupEntry(sym.name)
84+
assert(existing == null || existing.sym.isMethod, s"pkgClass = $pkgClass, sym = $sym, existing = $existing")
85+
super.enter(sym)
86+
}
87+
7988
// disable fingerprinting as we do not know entries beforehand
8089
private val negatives = mutable.Set[Name]() // Syncnote: Performance only, so need not be protected.
8190
override def lookupEntry(name: Name): ScopeEntry = {

0 commit comments

Comments
 (0)