Merge pull request #1385 from scalamacros/ticket/6412

SI-6412 some fixes for reflection leaks

11 files changed, 262 insertions, 96 deletions

diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index 58fcee4b30..0fbd930ad7 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -1079,12 +1079,12 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
    *  of what file was being compiled when it broke.  Since I really
    *  really want to know, this hack.
    */
-  private var lastSeenSourceFile: SourceFile = NoSourceFile
+  protected var lastSeenSourceFile: SourceFile = NoSourceFile
 
   /** Let's share a lot more about why we crash all over the place.
    *  People will be very grateful.
    */
-  private var lastSeenContext: analyzer.Context = null
+  protected var lastSeenContext: analyzer.Context = null
 
   /** The currently active run
    */
diff --git a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala
index f985eedf99..790c78cbf5 100644
--- a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala
+++ b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala
@@ -46,6 +46,23 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
         newTermName("__wrapper$" + wrapCount + "$" + java.util.UUID.randomUUID.toString.replace("-", ""))
       }
 
+      // should be called after every use of ToolBoxGlobal in order to prevent leaks
+      // there's the `withCleanupCaches` method defined below, which provides a convenient interface for that
+      def cleanupCaches(): Unit = {
+        lub(List(IntTpe, IntTpe)) // indirectly clears lubResults and glbResults
+                                  // I didn't want to turn those caches from private to protected
+                                  // in order not to screw up the performance
+        perRunCaches.clearAll()
+        undoLog.clear()
+        analyzer.lastTreeToTyper = EmptyTree
+        lastSeenSourceFile = NoSourceFile
+        lastSeenContext = null
+      }
+
+      def withCleanupCaches[T](body: => T): T =
+        try body
+        finally cleanupCaches()
+
       def verify(expr: Tree): Unit = {
         // Previously toolboxes used to typecheck their inputs before compiling.
         // Actually, the initial demo by Martin first typechecked the reified tree,
@@ -337,7 +354,7 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
     lazy val importer = compiler.mkImporter(u)
     lazy val exporter = importer.reverse
 
-    def typeCheck(tree: u.Tree, expectedType: u.Type, silent: Boolean = false, withImplicitViewsDisabled: Boolean = false, withMacrosDisabled: Boolean = false): u.Tree = {
+    def typeCheck(tree: u.Tree, expectedType: u.Type, silent: Boolean = false, withImplicitViewsDisabled: Boolean = false, withMacrosDisabled: Boolean = false): u.Tree = compiler.withCleanupCaches {
       if (compiler.settings.verbose.value) println("importing "+tree+", expectedType = "+expectedType)
       var ctree: compiler.Tree = importer.importTree(tree)
       var cexpectedType: compiler.Type = importer.importType(expectedType)
@@ -357,7 +374,7 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
       inferImplicit(tree, viewTpe, isView = true, silent = silent, withMacrosDisabled = withMacrosDisabled, pos = pos)
     }
 
-    private def inferImplicit(tree: u.Tree, pt: u.Type, isView: Boolean, silent: Boolean, withMacrosDisabled: Boolean, pos: u.Position): u.Tree = {
+    private def inferImplicit(tree: u.Tree, pt: u.Type, isView: Boolean, silent: Boolean, withMacrosDisabled: Boolean, pos: u.Position): u.Tree = compiler.withCleanupCaches {
       if (compiler.settings.verbose.value) println("importing "+pt, ", tree = "+tree+", pos = "+pos)
       var ctree: compiler.Tree = importer.importTree(tree)
       var cpt: compiler.Type = importer.importType(pt)
diff --git a/src/partest/scala/tools/partest/MemoryTest.scala b/src/partest/scala/tools/partest/MemoryTest.scala
new file mode 100644
index 0000000000..58d25d2f01
--- /dev/null
+++ b/src/partest/scala/tools/partest/MemoryTest.scala
@@ -0,0 +1,38 @@
+package scala.tools.partest
+
+abstract class MemoryTest {
+  def maxDelta: Double
+  def calcsPerIter: Int
+  def calc(): Unit
+
+  def main(args: Array[String]) {
+    val rt = Runtime.getRuntime()
+    def memUsage() = {
+      import java.lang.management._
+      import scala.collection.JavaConverters._
+      val pools = ManagementFactory.getMemoryPoolMXBeans.asScala
+      pools.map(_.getUsage.getUsed).sum / 1000000d
+    }
+
+    val history = scala.collection.mutable.ListBuffer[Double]()
+    def stressTestIter() = {
+      var i = 0
+      while (i < calcsPerIter) { calc(); i += 1 }
+      1 to 5 foreach (_ => rt.gc())
+      history += memUsage
+    }
+
+    1 to 5 foreach (_ => stressTestIter())
+    val reference = memUsage()
+    1 to 5 foreach (_ => stressTestIter())
+    1 to 5 foreach (_ => rt.gc())
+    val result = memUsage()
+    history += result
+
+    val delta = result - reference
+    if (delta > maxDelta) {
+      println("FAILED")
+      history foreach (mb => println(mb + " Mb"))
+    }
+  }
+}
diff --git a/src/reflect/scala/reflect/internal/Importers.scala b/src/reflect/scala/reflect/internal/Importers.scala
index c116928d37..02209d0712 100644
--- a/src/reflect/scala/reflect/internal/Importers.scala
+++ b/src/reflect/scala/reflect/internal/Importers.scala
@@ -1,6 +1,8 @@
 package scala.reflect
 package internal
+
 import scala.collection.mutable.WeakHashMap
+import scala.ref.WeakReference
 
 // SI-6241: move importers to a mirror
 trait Importers extends api.Importers { self: SymbolTable =>
@@ -26,8 +28,20 @@ trait Importers extends api.Importers { self: SymbolTable =>
 
     val from: SymbolTable
 
-    lazy val symMap: WeakHashMap[from.Symbol, Symbol] = new WeakHashMap
-    lazy val tpeMap: WeakHashMap[from.Type, Type] = new WeakHashMap
+    protected lazy val symMap = new Cache[from.Symbol, Symbol]()
+    protected lazy val tpeMap = new Cache[from.Type, Type]()
+    protected class Cache[K <: AnyRef, V <: AnyRef] extends WeakHashMap[K, WeakReference[V]] {
+      def weakGet(key: K): Option[V] = this get key flatMap WeakReference.unapply
+      def weakUpdate(key: K, value: V) = this.update(key, WeakReference(value))
+      def weakGetOrElseUpdate(key: K)(value: => V): V =
+        weakGet(key) match {
+          case Some(result) => result
+          case None =>
+            val result = value
+            this(key) = WeakReference(result)
+            result
+        }
+    }
 
     // fixups and maps prevent stackoverflows in importer
     var pendingSyms = 0
@@ -44,8 +58,10 @@ trait Importers extends api.Importers { self: SymbolTable =>
 
     object reverse extends from.StandardImporter {
       val from: self.type = self
-      for ((fromsym, mysym) <- StandardImporter.this.symMap) symMap += ((mysym, fromsym))
-      for ((fromtpe, mytpe) <- StandardImporter.this.tpeMap) tpeMap += ((mytpe, fromtpe))
+      // FIXME this and reverse should be constantly kept in sync
+      // not just synced once upon the first usage of reverse
+      for ((fromsym, WeakReference(mysym)) <- StandardImporter.this.symMap) symMap += ((mysym, WeakReference(fromsym)))
+      for ((fromtpe, WeakReference(mytpe)) <- StandardImporter.this.tpeMap) tpeMap += ((mytpe, WeakReference(fromtpe)))
     }
 
     // todo. careful import of positions
@@ -53,70 +69,70 @@ trait Importers extends api.Importers { self: SymbolTable =>
       pos.asInstanceOf[Position]
 
     def importSymbol(sym0: from.Symbol): Symbol = {
-      def doImport(sym: from.Symbol): Symbol = {
-        if (symMap.contains(sym))
-          return symMap(sym)
-
-        val myowner = importSymbol(sym.owner)
-        val mypos   = importPosition(sym.pos)
-        val myname  = importName(sym.name).toTermName
-        val myflags = sym.flags
-        def linkReferenced(mysym: TermSymbol, x: from.TermSymbol, op: from.Symbol => Symbol): Symbol = {
-          symMap(x) = mysym
-          mysym.referenced = op(x.referenced)
-          mysym
-        }
-        val mysym = sym match {
-          case x: from.MethodSymbol =>
-            linkReferenced(myowner.newMethod(myname, mypos, myflags), x, importSymbol)
-          case x: from.ModuleSymbol =>
-            linkReferenced(myowner.newModuleSymbol(myname, mypos, myflags), x, importSymbol)
-          case x: from.FreeTermSymbol =>
-            newFreeTermSymbol(importName(x.name).toTermName, x.value, x.flags, x.origin) setInfo importType(x.info)
-          case x: from.FreeTypeSymbol =>
-            newFreeTypeSymbol(importName(x.name).toTypeName, x.flags, x.origin)
-          case x: from.TermSymbol =>
-            linkReferenced(myowner.newValue(myname, mypos, myflags), x, importSymbol)
-          case x: from.TypeSkolem =>
-            val origin = x.unpackLocation match {
-              case null           => null
-              case y: from.Tree   => importTree(y)
-              case y: from.Symbol => importSymbol(y)
+      def doImport(sym: from.Symbol): Symbol =
+        symMap weakGet sym match {
+          case Some(result) => result
+          case _ =>
+            val myowner = importSymbol(sym.owner)
+            val mypos   = importPosition(sym.pos)
+            val myname  = importName(sym.name).toTermName
+            val myflags = sym.flags
+            def linkReferenced(mysym: TermSymbol, x: from.TermSymbol, op: from.Symbol => Symbol): Symbol = {
+              symMap.weakUpdate(x, mysym)
+              mysym.referenced = op(x.referenced)
+              mysym
             }
-            myowner.newTypeSkolemSymbol(myname.toTypeName, origin, mypos, myflags)
-          case x: from.ModuleClassSymbol =>
-            val mysym = myowner.newModuleClass(myname.toTypeName, mypos, myflags)
-            symMap(x) = mysym
-            mysym.sourceModule = importSymbol(x.sourceModule)
-            mysym
-          case x: from.ClassSymbol =>
-            val mysym = myowner.newClassSymbol(myname.toTypeName, mypos, myflags)
-            symMap(x) = mysym
-            if (sym.thisSym != sym) {
-              mysym.typeOfThis = importType(sym.typeOfThis)
-              mysym.thisSym setName importName(sym.thisSym.name)
+            val mysym = sym match {
+              case x: from.MethodSymbol =>
+                linkReferenced(myowner.newMethod(myname, mypos, myflags), x, importSymbol)
+              case x: from.ModuleSymbol =>
+                linkReferenced(myowner.newModuleSymbol(myname, mypos, myflags), x, importSymbol)
+              case x: from.FreeTermSymbol =>
+                newFreeTermSymbol(importName(x.name).toTermName, x.value, x.flags, x.origin) setInfo importType(x.info)
+              case x: from.FreeTypeSymbol =>
+                newFreeTypeSymbol(importName(x.name).toTypeName, x.flags, x.origin)
+              case x: from.TermSymbol =>
+                linkReferenced(myowner.newValue(myname, mypos, myflags), x, importSymbol)
+              case x: from.TypeSkolem =>
+                val origin = x.unpackLocation match {
+                  case null           => null
+                  case y: from.Tree   => importTree(y)
+                  case y: from.Symbol => importSymbol(y)
+                }
+                myowner.newTypeSkolemSymbol(myname.toTypeName, origin, mypos, myflags)
+              case x: from.ModuleClassSymbol =>
+                val mysym = myowner.newModuleClass(myname.toTypeName, mypos, myflags)
+                symMap.weakUpdate(x, mysym)
+                mysym.sourceModule = importSymbol(x.sourceModule)
+                mysym
+              case x: from.ClassSymbol =>
+                val mysym = myowner.newClassSymbol(myname.toTypeName, mypos, myflags)
+                symMap.weakUpdate(x, mysym)
+                if (sym.thisSym != sym) {
+                  mysym.typeOfThis = importType(sym.typeOfThis)
+                  mysym.thisSym setName importName(sym.thisSym.name)
+                }
+                mysym
+              case x: from.TypeSymbol =>
+                myowner.newTypeSymbol(myname.toTypeName, mypos, myflags)
             }
-            mysym
-          case x: from.TypeSymbol =>
-            myowner.newTypeSymbol(myname.toTypeName, mypos, myflags)
-        }
-        symMap(sym) = mysym
-        mysym setFlag Flags.LOCKED
-        mysym setInfo {
-          val mytypeParams = sym.typeParams map importSymbol
-          new LazyPolyType(mytypeParams) {
-            override def complete(s: Symbol) {
-              val result = sym.info match {
-                case from.PolyType(_, res) => res
-                case result => result
+            symMap.weakUpdate(sym, mysym)
+            mysym setFlag Flags.LOCKED
+            mysym setInfo {
+              val mytypeParams = sym.typeParams map importSymbol
+              new LazyPolyType(mytypeParams) {
+                override def complete(s: Symbol) {
+                  val result = sym.info match {
+                    case from.PolyType(_, res) => res
+                    case result => result
+                  }
+                  s setInfo GenPolyType(mytypeParams, importType(result))
+                  s setAnnotations (sym.annotations map importAnnotationInfo)
+                }
               }
-              s setInfo GenPolyType(mytypeParams, importType(result))
-              s setAnnotations (sym.annotations map importAnnotationInfo)
             }
-          }
-        }
-        mysym resetFlag Flags.LOCKED
-      } // end doImport
+            mysym resetFlag Flags.LOCKED
+        } // end doImport
 
       def importOrRelink: Symbol = {
         val sym = sym0 // makes sym visible in the debugger
@@ -186,14 +202,10 @@ trait Importers extends api.Importers { self: SymbolTable =>
       } // end importOrRelink
 
       val sym = sym0
-      if (symMap contains sym) {
-        symMap(sym)
-      } else {
+      symMap.weakGetOrElseUpdate(sym) {
         pendingSyms += 1
-
-        try {
-          symMap getOrElseUpdate (sym, importOrRelink)
-        } finally {
+        try importOrRelink
+        finally {
           pendingSyms -= 1
           tryFixup()
         }
@@ -258,14 +270,10 @@ trait Importers extends api.Importers { self: SymbolTable =>
       def importOrRelink: Type =
         doImport(tpe)
 
-      if (tpeMap contains tpe) {
-        tpeMap(tpe)
-      } else {
+      tpeMap.weakGetOrElseUpdate(tpe) {
         pendingTpes += 1
-
-        try {
-          tpeMap getOrElseUpdate (tpe, importOrRelink)
-        } finally {
+        try importOrRelink
+        finally {
           pendingTpes -= 1
           tryFixup()
         }
diff --git a/src/reflect/scala/reflect/internal/Mirrors.scala b/src/reflect/scala/reflect/internal/Mirrors.scala
index bde7f7ac51..4836db5db0 100644
--- a/src/reflect/scala/reflect/internal/Mirrors.scala
+++ b/src/reflect/scala/reflect/internal/Mirrors.scala
@@ -10,10 +10,15 @@ package internal
 import Flags._
 
 trait Mirrors extends api.Mirrors {
-  self: SymbolTable =>
+  thisUniverse: SymbolTable =>
 
   override type Mirror >: Null <: RootsBase
 
+  // root symbols hold a strong reference to the enclosing mirror
+  // this prevents the mirror from being collected
+  // if there are any symbols created by that mirror
+  trait RootSymbol extends Symbol { def mirror: Mirror }
+
   abstract class RootsBase(rootOwner: Symbol) extends MirrorOf[Mirrors.this.type] { thisMirror =>
 
     protected[scala] def rootLoader: LazyType
@@ -70,7 +75,7 @@ trait Mirrors extends api.Mirrors {
 
     protected def mirrorMissingHook(owner: Symbol, name: Name): Symbol = NoSymbol
 
-    protected def universeMissingHook(owner: Symbol, name: Name): Symbol = self.missingHook(owner, name)
+    protected def universeMissingHook(owner: Symbol, name: Name): Symbol = thisUniverse.missingHook(owner, name)
 
     private[scala] def missingHook(owner: Symbol, name: Name): Symbol = mirrorMissingHook(owner, name) orElse universeMissingHook(owner, name)
 
@@ -251,10 +256,11 @@ trait Mirrors extends api.Mirrors {
     }
     // Features common to RootClass and RootPackage, the roots of all
     // type and term symbols respectively.
-    sealed trait RootSymbol extends WellKnownSymbol {
+    sealed trait RootSymbol extends WellKnownSymbol with thisUniverse.RootSymbol {
       final override def isRootSymbol = true
       override def owner              = rootOwner
       override def typeOfThis         = thisSym.tpe
+      def mirror                      = thisMirror.asInstanceOf[Mirror]
     }
 
     // This is the package _root_.  The actual root cannot be referenced at
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index 0d9e90d3a6..2f12ba59a2 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -1236,16 +1236,9 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { thisUnive
   override def scopeTransform(owner: Symbol)(op: => Scope): Scope =
     if (owner.isPackageClass) owner.info.decls else op
 
-  private lazy val rootToLoader = new WeakHashMap[Symbol, ClassLoader]
-
-  override def mirrorThatLoaded(sym: Symbol): Mirror = {
-    val root = sym.enclosingRootClass
-    def findLoader = {
-      val loaders = (mirrors collect { case (cl, ref) if ref.get.get.RootClass == root => cl })
-      assert(loaders.nonEmpty, sym)
-      loaders.head
-    }
-    mirrors(rootToLoader getOrElseUpdate(root, findLoader)).get.get
+  override def mirrorThatLoaded(sym: Symbol): Mirror = sym.enclosingRootClass match {
+    case root: RootSymbol => root.mirror
+    case _ => abort(s"${sym}.enclosingRootClass = ${sym.enclosingRootClass}, which is not a RootSymbol")
   }
 
   private lazy val syntheticCoreClasses: Map[(String, Name), Symbol] = {
diff --git a/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala b/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala
index de029ca658..b9b140a2fd 100644
--- a/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala
+++ b/src/reflect/scala/reflect/runtime/SynchronizedTypes.scala
@@ -1,6 +1,9 @@
 package scala.reflect
 package runtime
 
+import scala.collection.mutable.WeakHashMap
+import java.lang.ref.WeakReference
+
 /** This trait overrides methods in reflect.internal, bracketing
  *  them in synchronized { ... } to make them thread-safe
  */
@@ -11,7 +14,26 @@ trait SynchronizedTypes extends internal.Types { self: SymbolTable =>
 
   private object uniqueLock
 
-  override def unique[T <: Type](tp: T): T = uniqueLock.synchronized { super.unique(tp) }
+  private val uniques = WeakHashMap[Type, WeakReference[Type]]()
+  override def unique[T <: Type](tp: T): T = uniqueLock.synchronized {
+    // we need to have weak uniques for runtime reflection
+    // because unlike the normal compiler universe, reflective universe isn't organized in runs
+    // therefore perRunCaches can grow infinitely large
+    //
+    // despite that toolbox universes are decorated, toolboxes are compilers,
+    // i.e. they have their caches cleaned up automatically on per-run basis,
+    // therefore they should use vanilla uniques, which are faster
+    if (!isCompilerUniverse) {
+      val result = if (uniques contains tp) uniques(tp).get else null
+      if (result ne null) result.asInstanceOf[T]
+      else {
+        uniques(tp) = new WeakReference(tp)
+        tp
+      }
+    } else {
+      super.unique(tp)
+    }
+  }
 
   class SynchronizedUndoLog extends UndoLog {
     private val actualLock = new java.util.concurrent.locks.ReentrantLock
diff --git a/test/files/run/reflection-mem-glbs.scala b/test/files/run/reflection-mem-glbs.scala
new file mode 100644
index 0000000000..3f29a914bc
--- /dev/null
+++ b/test/files/run/reflection-mem-glbs.scala
@@ -0,0 +1,13 @@
+import scala.tools.partest.MemoryTest
+
+trait A { type T <: A }
+trait B { type T <: B }
+
+object Test extends MemoryTest {
+  override def maxDelta = 10
+  override def calcsPerIter = 50000
+  override def calc() {
+    import scala.reflect.runtime.universe._
+    glb(List(typeOf[A], typeOf[B]))
+  }
+}
\ No newline at end of file
diff --git a/test/files/run/reflection-mem-tags.scala b/test/files/run/reflection-mem-tags.scala
new file mode 100644
index 0000000000..8815e7dcd8
--- /dev/null
+++ b/test/files/run/reflection-mem-tags.scala
@@ -0,0 +1,17 @@
+import scala.tools.partest.MemoryTest
+
+trait A { type T <: A }
+trait B { type T <: B }
+
+object Test extends MemoryTest {
+  override def maxDelta = 10
+  override def calcsPerIter = 100000
+  override def calc() {
+    import scala.reflect.runtime.universe._
+    def foo = {
+      class A { def x = 2; def y: A = new A }
+      weakTypeOf[A { def z: Int }]
+    }
+    foo
+  }
+}
\ No newline at end of file
diff --git a/test/files/run/reflection-mem-typecheck.scala b/test/files/run/reflection-mem-typecheck.scala
new file mode 100644
index 0000000000..a312c2c893
--- /dev/null
+++ b/test/files/run/reflection-mem-typecheck.scala
@@ -0,0 +1,26 @@
+import scala.tools.partest.MemoryTest
+
+trait A { type T <: A }
+trait B { type T <: B }
+
+object Test extends MemoryTest {
+  lazy val tb = {
+    import scala.reflect.runtime.universe._
+    import scala.reflect.runtime.{currentMirror => cm}
+    import scala.tools.reflect.ToolBox
+    cm.mkToolBox()
+  }
+
+  override def maxDelta = 10
+  override def calcsPerIter = 8
+  override def calc() {
+    var snippet = """
+      trait A { type T <: A }
+      trait B { type T <: B }
+      def foo[T](x: List[T]) = x
+      foo(List(new A {}, new B {}))
+    """.trim
+    snippet = snippet + "\n" + (List.fill(50)(snippet.split("\n").last) mkString "\n")
+    tb.typeCheck(tb.parse(snippet))
+  }
+}
\ No newline at end of file
diff --git a/test/pending/run/reflection-mem-eval.scala b/test/pending/run/reflection-mem-eval.scala
new file mode 100644
index 0000000000..9045c44cd6
--- /dev/null
+++ b/test/pending/run/reflection-mem-eval.scala
@@ -0,0 +1,26 @@
+import scala.tools.partest.MemoryTest
+
+trait A { type T <: A }
+trait B { type T <: B }
+
+object Test extends MemoryTest {
+  lazy val tb = {
+    import scala.reflect.runtime.universe._
+    import scala.reflect.runtime.{currentMirror => cm}
+    import scala.tools.reflect.ToolBox
+    cm.mkToolBox()
+  }
+
+  override def maxDelta = 10
+  override def calcsPerIter = 3
+  override def calc() {
+    var snippet = """
+      trait A { type T <: A }
+      trait B { type T <: B }
+      def foo[T](x: List[T]) = x
+      foo(List(new A {}, new B {}))
+    """.trim
+    snippet = snippet + "\n" + (List.fill(50)(snippet.split("\n").last) mkString "\n")
+    tb.eval(tb.parse(snippet))
+  }
+}
\ No newline at end of file