SI-6412 alleviates leaks in toolboxes, attempt #2

Turns importer caches into fully weak hash maps, and also applies
manual cleanup to toolboxes every time they are used.

It's not enough, because reflection-mem-typecheck test is still leaking
at a rate of ~100kb per typecheck, but it's much better than it was before.
We'll fix the rest later, after 2.10.0-final.

For more information, see https://issues.scala-lang.org/browse/SI-6412 and
http://groups.google.com/group/scala-internals/browse_thread/thread/eabcf3d406dab8b2

In comparison with scala@b403c1d,
the original commit that implemented the fix, this one doesn't crash tests.
The problem with the original commit was that it called tryFixup() before
updating the cache, leading to stack overflows.

Author: xeno-by

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
    */

src/compiler/scala/tools/reflect/ToolBoxFactory.scala

@@ -46,6 +46,20 @@ 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 = {
+        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 +351,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 +371,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)

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,12 @@ 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))
+    }
 
     // fixups and maps prevent stackoverflows in importer
     var pendingSyms = 0
@@ -44,79 +50,81 @@ 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
     def importPosition(pos: from.Position): Position =
       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) with FlagAgnosticCompleter {
-            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) with FlagAgnosticCompleter {
+                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,17 +194,18 @@ trait Importers extends api.Importers { self: SymbolTable =>
       } // end importOrRelink
 
       val sym = sym0
-      if (symMap contains sym) {
-        symMap(sym)
-      } else {
-        pendingSyms += 1
-
-        try {
-          symMap getOrElseUpdate (sym, importOrRelink)
-        } finally {
-          pendingSyms -= 1
-          tryFixup()
-        }
+      symMap.weakGet(sym) match {
+        case Some(result) => result
+        case None =>
+          pendingSyms += 1
+          try {
+            val result = importOrRelink
+            symMap.weakUpdate(sym, result)
+            result
+          } finally {
+            pendingSyms -= 1
+            tryFixup()
+          }
       }
     }
 
@@ -258,17 +267,18 @@ trait Importers extends api.Importers { self: SymbolTable =>
       def importOrRelink: Type =
         doImport(tpe)
 
-      if (tpeMap contains tpe) {
-        tpeMap(tpe)
-      } else {
-        pendingTpes += 1
-
-        try {
-          tpeMap getOrElseUpdate (tpe, importOrRelink)
-        } finally {
-          pendingTpes -= 1
-          tryFixup()
-        }
+      tpeMap.weakGet(tpe) match {
+        case Some(result) => result
+        case None =>
+          pendingTpes += 1
+          try {
+            val result = importOrRelink
+            tpeMap.weakUpdate(tpe, result)
+            result
+          } finally {
+            pendingTpes -= 1
+            tryFixup()
+          }
       }
     }
 

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))
+  }
+}

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))
+  }
+}