Merge pull request scala#11128 from som-snytt/test/8946-restore-memory-leak-test

Restore memory leak test for reflection

Author: lrytz

src/library/scala/collection/immutable/HashMap.scala

@@ -1345,6 +1345,8 @@ private final class BitmapIndexedMapNode[K, +V](
   override def hashCode(): Int =
     throw new UnsupportedOperationException("Trie nodes do not support hashing.")
 
+  override def toString = s"${getClass.getName}@${Integer.toHexString(System.identityHashCode(this))}"
+
   override def concat[V1 >: V](that: MapNode[K, V1], shift: Int): BitmapIndexedMapNode[K, V1] = that match {
     case bm: BitmapIndexedMapNode[K, V] @unchecked =>
       if (size == 0) return bm
@@ -2090,6 +2092,8 @@ private final class HashCollisionMapNode[K, +V ](
   override def hashCode(): Int =
     throw new UnsupportedOperationException("Trie nodes do not support hashing.")
 
+  override def toString = s"${getClass.getName}@${Integer.toHexString(System.identityHashCode(this))}"
+
   override def cachedJavaKeySetHashCode: Int = size * hash
 
 }

src/reflect/scala/reflect/runtime/ThreadLocalStorage.scala

@@ -13,18 +13,23 @@
 package scala.reflect
 package runtime
 
-import java.lang.Thread._
+import java.lang.Thread.currentThread
+import java.util.Collections.synchronizedMap
+import java.util.{WeakHashMap => jWeakHashMap}
 
 private[reflect] trait ThreadLocalStorage {
   self: SymbolTable =>
 
   // see a discussion at scala-internals for more information:
   // https://groups.google.com/group/scala-internals/browse_thread/thread/337ce68aa5e51f79
-  trait ThreadLocalStorage[T] { def get: T; def set(newValue: T): Unit }
+  trait ThreadLocalStorage[T] {
+    def get: T
+    def set(newValue: T): Unit
+  }
   private class MyThreadLocalStorage[T](initialValue: => T) extends ThreadLocalStorage[T] {
-    // TODO: how do we use org.cliffc.high_scale_lib.NonBlockingHashMap here?
-    //       (we would need a version that uses weak keys)
-    private[this] val values = java.util.Collections.synchronizedMap(new java.util.WeakHashMap[Thread, T]())
+    // consider a nonblocking map such as org.cliffc.high_scale_lib.NonBlockingHashMap
+    // but we would need a version that uses weak keys
+    private[this] val values = synchronizedMap(new jWeakHashMap[Thread, T])
     def get: T = {
       if (values containsKey currentThread) values.get(currentThread)
       else {

src/testkit/scala/tools/testkit/AssertUtil.scala

@@ -203,6 +203,11 @@ object AssertUtil {
   def assertSameElements[A, B >: A](expected: Array[A], actual: Array[B], message: String): Unit =
     assertSameElements(expected.toIndexedSeq, actual.toIndexedSeq, message)
 
+  @nowarn("msg=This catches all Throwables")
+  private def safeString(x: AnyRef): String =
+    try String.valueOf(x)
+    catch (t: Throwable) => t.getMessage
+
   /** Value is not strongly reachable from roots after body is evaluated.
    */
   def assertNotReachable[A <: AnyRef](a: => A, roots: AnyRef*)(body: => Unit): Unit = {
@@ -216,7 +221,8 @@ object AssertUtil {
         val o: AnyRef = stack.pop()
         if (o != null && !seen.containsKey(o)) {
           seen.put(o, ())
-          assertFalse(s"Root $root held reference $o", ref.hasReferent(o))
+          if (ref.hasReferent(o))
+            fail(s"Root ${safeString(root)} held reference ${safeString(o)}")
           o match {
             case a: Array[AnyRef] =>
               a.foreach(e => if (e != null && !e.isInstanceOf[Reference[_]]) stack.push(e))

test/files/run/t8946.scala

@@ -0,0 +1,35 @@
+//> using jvm 17+
+//> using javaOpt --add-opens java.base/java.lang=ALL-UNNAMED
+//> using options -Xsource:3
+
+import scala.jdk.CollectionConverters.*
+import scala.jdk.OptionConverters.*
+import scala.reflect.runtime.*, universe.*
+import scala.tools.testkit.AssertUtil.assertNotReachable
+import scala.tools.testkit.ReflectUtil.*
+
+// `t8946 reflection subtype does not hold onto Thread`
+object Test extends App {
+  CanOpener.deworm()
+  val reflector = new Thread("scala.reflector") {
+    override def run() = typeOf[List[String]] <:< typeOf[Seq[_]]
+  }
+  val joiner: Thread => Unit = { t =>
+    t.start()
+    t.join()
+  }
+  assertNotReachable(reflector, universe)(joiner(reflector))
+  // without the fix to use WeakHashMap in ThreadLocalStorage:
+  //AssertionError: Root scala.reflect.runtime.JavaUniverse@6ce86ce1 held reference Thread[#23,scala.reflector,5,]
+}
+
+// Modules are a can of worms for tools which must tread through arbitrary packages.
+object CanOpener {
+  def deworm(): Unit = {
+    val True = java.lang.Boolean.TRUE
+    val unnamed = getClass.getClassLoader.getUnnamedModule
+    val method = getMethodAccessible[Module]("implAddExportsOrOpens")
+    for (base <- ModuleLayer.boot.findModule("java.base").toScala; pkg <- base.getPackages.asScala)
+      method.invokeAs[Unit](base, pkg, unnamed, True, True)
+  }
+}