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 https://github.com/scala/scala/commit/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.

1 files changed, 96 insertions, 86 deletions

diff --git a/src/reflect/scala/reflect/internal/Importers.scala b/src/reflect/scala/reflect/internal/Importers.scala
index 893d039c3e..43902c1930 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,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,8 +50,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 +61,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) 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()
+          }
       }
     }