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).

And then it worked, but stopped working half a year later when
Jason and I came to revisit this old pull request. The final twist
was pre-initializing ObjectClass, because it's a dependency of AnyRefClass,
which is a critical dependency of ScalaPackageClass (full information
can be found in comments).

8 files changed, 146 insertions, 34 deletions

diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index 99aad4f057..e8fea43758 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -1180,10 +1180,17 @@ trait Definitions extends api.StandardDefinitions {
     }
 
     lazy val AnnotationDefaultAttr: ClassSymbol = {
-      val attr = enterNewClass(RuntimePackageClass, tpnme.AnnotationDefaultATTR, List(AnnotationClass.tpe))
-      // This attribute needs a constructor so that modifiers in parsed Java code make sense
-      attr.info.decls enter attr.newClassConstructor(NoPosition)
-      attr
+      val sym = RuntimePackageClass.newClassSymbol(tpnme.AnnotationDefaultATTR, NoPosition, 0L)
+      sym setInfo ClassInfoType(List(AnnotationClass.tpe), newScope, sym)
+      RuntimePackageClass.info.decls.toList.filter(_.name == sym.name) match {
+        case existing :: _ =>
+          existing.asInstanceOf[ClassSymbol]
+        case _ =>
+          RuntimePackageClass.info.decls enter sym
+          // This attribute needs a constructor so that modifiers in parsed Java code make sense
+          sym.info.decls enter sym.newClassConstructor(NoPosition)
+          sym
+      }
     }
 
     private def fatalMissingSymbol(owner: Symbol, name: Name, what: String = "member") = {
@@ -1388,10 +1395,13 @@ trait Definitions extends api.StandardDefinitions {
       else flatNameString(etp.typeSymbol, '.')
     }
 
+    // documented in JavaUniverse.init
     def init() {
       if (isInitialized) return
-      // force initialization of every symbol that is synthesized or hijacked by the compiler
-      val _ = symbolsNotPresentInBytecode
+      ObjectClass.initialize
+      ScalaPackageClass.initialize
+      val forced1 = symbolsNotPresentInBytecode
+      val forced2 = NoSymbol
       isInitialized = true
     } //init
 
diff --git a/src/reflect/scala/reflect/internal/Mirrors.scala b/src/reflect/scala/reflect/internal/Mirrors.scala
index 4c2995b676..e122fa498b 100644
--- a/src/reflect/scala/reflect/internal/Mirrors.scala
+++ b/src/reflect/scala/reflect/internal/Mirrors.scala
@@ -250,6 +250,19 @@ trait Mirrors extends api.Mirrors {
       RootClass.info.decls enter EmptyPackage
       RootClass.info.decls enter RootPackage
 
+      if (rootOwner != NoSymbol) {
+        // synthetic core classes are only present in root mirrors
+        // because Definitions.scala, which initializes and enters them, only affects rootMirror
+        // therefore we need to enter them manually for non-root mirrors
+        definitions.syntheticCoreClasses foreach (theirSym => {
+          val theirOwner = theirSym.owner
+          assert(theirOwner.isPackageClass, s"theirSym = $theirSym, theirOwner = $theirOwner")
+          val ourOwner = staticPackage(theirOwner.fullName).moduleClass
+          val ourSym = theirSym // just copy the symbol into our branch of the symbol table
+          ourOwner.info.decls enterIfNew ourSym
+        })
+      }
+
       initialized = true
     }
   }
diff --git a/src/reflect/scala/reflect/internal/Scopes.scala b/src/reflect/scala/reflect/internal/Scopes.scala
index 1060b3a99c..b7a1681838 100644
--- a/src/reflect/scala/reflect/internal/Scopes.scala
+++ b/src/reflect/scala/reflect/internal/Scopes.scala
@@ -139,6 +139,12 @@ trait Scopes extends api.Scopes { self: SymbolTable =>
       enter(sym)
     }
 
+    def enterIfNew[T <: Symbol](sym: T): T = {
+      val existing = lookupEntry(sym.name)
+      if (existing == null) enter(sym)
+      else existing.sym.asInstanceOf[T]
+    }
+
     private def createHash() {
       hashtable = new Array[ScopeEntry](HASHSIZE)
       enterAllInHash(elems)
diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala
index af26253802..c39efa26fa 100644
--- a/src/reflect/scala/reflect/internal/StdNames.scala
+++ b/src/reflect/scala/reflect/internal/StdNames.scala
@@ -273,6 +273,8 @@ trait StdNames {
     final val SourceFileATTR: NameType             = "SourceFile"
     final val SyntheticATTR: NameType              = "Synthetic"
 
+    final val scala_ : NameType = "scala"
+
     def dropSingletonName(name: Name): TypeName = (name dropRight SINGLETON_SUFFIX.length).toTypeName
     def singletonName(name: Name): TypeName     = (name append SINGLETON_SUFFIX).toTypeName
     def implClassName(name: Name): TypeName     = (name append IMPL_CLASS_SUFFIX).toTypeName
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 868913c82f..858d6cf618 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -3341,10 +3341,11 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def name = nme.NO_NAME
     override def name_=(n: Name) = abort("Cannot set NoSymbol's name to " + n)
 
-    synchronized {
-      setInfo(NoType)
-      privateWithin = this
-    }
+    // Syncnote: no need to synchronize this, because NoSymbol's initialization is triggered by JavaUniverse.init
+    // which is called in universe's constructor - something that's inherently single-threaded
+    setInfo(NoType)
+    privateWithin = this
+
     override def info_=(info: Type) = {
       infos = TypeHistory(1, NoType, null)
       unlock()
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index 2cdacde900..487da2eb4b 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -44,16 +44,6 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
 
   trait JavaClassCompleter extends FlagAssigningCompleter
 
-  def init() = {
-    definitions.AnyValClass // force it.
-
-    // establish root association to avoid cyclic dependency errors later
-    rootMirror.classToScala(classOf[java.lang.Object]).initialize
-
-    // println("initializing definitions")
-    definitions.init()
-  }
-
   def runtimeMirror(cl: ClassLoader): Mirror = mirrors get cl match {
     case Some(WeakReference(m)) => m
     case _ => createMirror(rootMirror.RootClass, cl)
@@ -1280,11 +1270,6 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
     case _ => abort(s"${sym}.enclosingRootClass = ${sym.enclosingRootClass}, which is not a RootSymbol")
   }
 
-  private lazy val syntheticCoreClasses: Map[(String, Name), Symbol] = {
-    def mapEntry(sym: Symbol): ((String, Name), Symbol) = (sym.owner.fullName, sym.name) -> sym
-    Map() ++ (definitions.syntheticCoreClasses map mapEntry)
-  }
-
   /** 1. If `owner` is a package class (but not the empty package) and `name` is a term name, make a new package
    *  <owner>.<name>, otherwise return NoSymbol.
    *  Exception: If owner is root and a java class with given name exists, create symbol in empty package instead
@@ -1301,13 +1286,15 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
       if (name.isTermName && !owner.isEmptyPackageClass)
         return mirror.makeScalaPackage(
           if (owner.isRootSymbol) name.toString else owner.fullName+"."+name)
-      syntheticCoreClasses get ((owner.fullName, name)) foreach { tsym =>
-        // synthetic core classes are only present in root mirrors
-        // because Definitions.scala, which initializes and enters them, only affects rootMirror
-        // therefore we need to enter them manually for non-root mirrors
-        if (mirror ne thisUniverse.rootMirror) owner.info.decls enter tsym
-        return tsym
-      }
+      if (name == tpnme.AnyRef && owner.owner.isRoot && owner.name == tpnme.scala_)
+        // when we synthesize the scala.AnyRef symbol, we need to add it to the scope of the scala package
+        // the problem is that adding to the scope implies doing something like `owner.info.decls enter anyRef`
+        // which entails running a completer for the scala package
+        // which will try to unpickle the stuff in scala/package.class
+        // which will transitively load scala.AnyRef
+        // which doesn't exist yet, because it hasn't been added to the scope yet
+        // this missing hook ties the knot without introducing synchronization problems like before
+        return definitions.AnyRefClass
     }
     info("*** missing: "+name+"/"+name.isTermName+"/"+owner+"/"+owner.hasPackageFlag+"/"+owner.info.decls.getClass)
     super.missingHook(owner, name)
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverse.scala b/src/reflect/scala/reflect/runtime/JavaUniverse.scala
index 06a7db6289..8f1c1967e0 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverse.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverse.scala
@@ -27,4 +27,65 @@ class JavaUniverse extends internal.SymbolTable with ReflectSetup with runtime.S
   } with internal.TreeInfo
 
   init()
+
+  // ======= Initialization of runtime reflection =======
+  //
+  // This doc describes the carefully laid out sequence of actions used to initialize reflective universes.
+  //
+  // Before reading the text below, read up the section Mirrors in the reflection pre-SIP
+  // https://docs.google.com/document/d/1nAwSw4TmMplsIlzh2shYLUJ5mVh3wndDa1Zm1H6an9A/edit.
+  // Take an especially good look at Figure 2, because it illustrates fundamental principles underlying runtime reflection:
+  //   1) For each universe we have one mirror per classloader
+  //   2) Package symbols are per-mirror
+  //   3) Other symbols are per-universe, which means that a symbol (e.g. Seq on the picture) might be shared between multiple owners
+  //
+  // Main challenges that runtime reflection presents wrt initialization are:
+  //   1) Extravagant completion scheme that enters package members on-demand rather than a result of scanning a directory with class files.
+  //      (That's a direct consequence of the fact that in general case we can't enumerate all classes in a classloader).
+  //   2) Presence of synthetic symbols that aren't loaded by normal means (from classfiles) but are synthesized on-the-fly,
+  //      and the necessity to propagate these synthetic symbols from rootMirror to other mirrors,
+  //      complicated by the fact that such symbols depend on normal symbols (e.g. AnyRef depends on Object).
+  //   3) Necessity to remain thread-safe, which limits our options related to lazy initialization
+  //      (E.g. we cannot use missingHook to enter synthetic symbols, because that's thread-unsafe).
+  //
+  // Directly addressing the challenge #3, we create all synthetic symbols fully in advance during init().
+  // However, it's not that simple as just calling definitions.symbolsNotPresentInBytecode.
+  // Before doing that, we need to first initialize ObjectClass, then ScalaPackageClass, and only then deal with synthetics.
+  // Below you can find a detailed explanation for that.
+  //
+  // ### Why ScalaPackageClass? ###
+  //
+  // Forcing ScalaPackageClass first thing during startup is important, because syntheticCoreClasses such as AnyRefClass
+  // need to be entered into ScalaPackageClass, which entails calling ScalaPackageClass.info.decls.enter.
+  // If ScalaPackageClass isn't initialized by that moment, the following will happen for runtime reflection:
+  //   1) Initialization of ScalaPackageClass will trigger unpickling.
+  //   2) Unpickling will need to load some auxiliary types such as, for example, String.
+  //   3) To load String, runtime reflection will call mirrorDefining(classOf[String]).
+  //   4) This, in turn, will call runtimeMirror(classOf[String].getClassLoader).
+  //   5) For some classloader configurations, the resulting mirror will be different from rootMirror.
+  //   6) In that case, initialization of the resulting mirror will try to import definitions.syntheticCoreClasses into the mirror.
+  //   7) This will force all the lazy vals corresponding to syntheticCoreClasses.
+  //   8) By that time, the completer of ScalaPackageClass will have already called setInfo on ScalaPackageClass, so there won't be any stack overflow.
+  //
+  // So far so good, no crashes, no problems, right? Not quite.
+  // If forcing of ScalaPackageClass was called by a syntheticCoreClasses lazy val,
+  // then this lazy val will be entered twice: once during step 7 and once when returning from the original call.
+  // To avoid this we need to initialize ScalaPackageClass prior to other synthetics.
+  //
+  // ### Why ObjectClass? ###
+  //
+  // 1) As explained in JavaMirrors.missingHook, initialization of ScalaPackageClass critically depends on AnyRefClass.
+  // 2) AnyRefClass is defined as "lazy val AnyRefClass = newAlias(ScalaPackageClass, tpnme.AnyRef, ObjectTpe)",
+  //    which means that initialization of AnyRefClass depends on ObjectClass.
+  // 3) ObjectClass is defined as "lazy val ObjectClass = getRequiredClass(sn.Object.toString)",
+  //    which means that under some classloader configurations (see JavaMirrors.missingHook for more details)
+  //    dereferencing ObjectClass might trigger an avalanche of initializations calling back into AnyRefClass
+  //    while another AnyRefClass initializer is still on stack.
+  // 4) That will lead to AnyRefClass being entered two times (once when the recursive call returns and once when the original one returns)
+  // 5) That will crash PackageScope.enter that helpfully detects double-enters.
+  //
+  // Therefore, before initializing ScalaPackageClass, we must pre-initialize ObjectClass
+  def init() {
+    definitions.init()
+  }
 }
diff --git a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala
index 3e149c1fe0..706572ba2b 100644
--- a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala
+++ b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala
@@ -70,6 +70,25 @@ private[reflect] trait SymbolLoaders { self: SymbolTable =>
   class PackageScope(pkgClass: Symbol) extends Scope(initFingerPrints = -1L) // disable fingerprinting as we do not know entries beforehand
       with SynchronizedScope {
     assert(pkgClass.isType)
+
+    // materializing multiple copies of the same symbol in PackageScope is a very popular bug
+    // this override does its best to guard against it
+    override def enter[T <: Symbol](sym: T): T = {
+      // workaround for SI-7728
+      if (isCompilerUniverse) super.enter(sym)
+      else {
+        val existing = super.lookupEntry(sym.name)
+        assert(existing == null || existing.sym.isMethod, s"pkgClass = $pkgClass, sym = $sym, existing = $existing")
+        super.enter(sym)
+      }
+    }
+
+    override def enterIfNew[T <: Symbol](sym: T): T = {
+      val existing = super.lookupEntry(sym.name)
+      if (existing == null) enter(sym)
+      else existing.sym.asInstanceOf[T]
+    }
+
     // disable fingerprinting as we do not know entries beforehand
     private val negatives = mutable.Set[Name]() // Syncnote: Performance only, so need not be protected.
     override def lookupEntry(name: Name): ScopeEntry = {
@@ -95,8 +114,21 @@ private[reflect] trait SymbolLoaders { self: SymbolTable =>
                 val module = origOwner.info decl name.toTermName
                 assert(clazz != NoSymbol)
                 assert(module != NoSymbol)
-                pkgClass.info.decls enter clazz
-                pkgClass.info.decls enter module
+                // currentMirror.mirrorDefining(cls) might side effect by entering symbols into pkgClass.info.decls
+                // therefore, even though in the beginning of this method, super.lookupEntry(name) returned null
+                // entering clazz/module now will result in a double-enter assertion in PackageScope.enter
+                // here's how it might happen
+                // 1) we are the rootMirror
+                // 2) cls.getClassLoader is different from our classloader
+                // 3) mirrorDefining(cls) looks up a mirror corresponding to that classloader and cannot find it
+                // 4) mirrorDefining creates a new mirror
+                // 5) that triggers Mirror.init() of the new mirror
+                // 6) that triggers definitions.syntheticCoreClasses
+                // 7) that might materialize symbols and enter them into our scope (because syntheticCoreClasses live in rootMirror)
+                // 8) now we come back here and try to enter one of the now entered symbols => BAM!
+                // therefore we use enterIfNew rather than just enter
+                enterIfNew(clazz)
+                enterIfNew(module)
                 (clazz, module)
               }
             debugInfo(s"created $module/${module.moduleClass} in $pkgClass")