[2.0.x] fix: Fixes three test-classloader / jar-handle leaks in the sbt server JVM (#9485)

2 independent, pre-existing retention bugs kept the classloader of a
finished in-process test run -- and the open jar handles it holds -- alive
for the rest of the server session.

- JUnitXmlTestsListener: testSuite is an InheritableThreadLocal, so
  threads spawned during a run (e.g. async-framework pool workers) inherit
  a copy of the suite reference that remove() cannot reach.

- ClassLoaderCache: loaders evicted from delegate by clearExpiredLoaders
  are unreachable from the map and never enqueued on the ReferenceQueue, so
  neither clear()/close() nor the cleanup thread could ever close them.

SuiteResult now documents the retention hazard on throwables. Adds
deterministic, cross-platform tests for all three severed chains.

main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala

@@ -77,12 +77,34 @@ private[sbt] class ClassLoaderCache(
     new java.util.concurrent.ConcurrentHashMap[Key, Reference[ClassLoader]]()
   private val referenceQueue = new ReferenceQueue[ClassLoader]
 
+  /*
+   * Loaders evicted from `delegate` by clearExpiredLoaders are no longer reachable from the
+   * map, so clear()/close() alone can never close them. Nor can the cleanup thread: once the
+   * entry is removed, the Reference object itself becomes unreachable, and an unreachable
+   * Reference is never enqueued on the ReferenceQueue. They would linger with open jar handles
+   * for the life of the JVM. On Windows those handles make the underlying jars undeletable
+   * (e.g. clearCaches cannot delete cas blobs the loaders still reference). Track evicted
+   * loaders weakly so clear() can close them deterministically; weak keys preserve the
+   * metaspace-pressure design above by adding no strong retention of their own.
+   */
+  private val retired =
+    java.util.Collections.synchronizedMap(new java.util.WeakHashMap[ClassLoader, java.lang.Boolean])
+
   private def clearExpiredLoaders(): Unit = lock.synchronized {
     val clear = (k: Key, ref: Reference[ClassLoader]) => {
       ref.get() match {
         case w: WrappedLoader => w.invalidate()
         case _                =>
       }
+      ref match {
+        case ClassLoaderReference(_, underlying) =>
+          retired.put(underlying, java.lang.Boolean.TRUE)
+        case r =>
+          r.get() match {
+            case null   =>
+            case loader => retired.put(loader, java.lang.Boolean.TRUE)
+          }
+      }
       delegate.remove(k)
       ()
     }
@@ -109,6 +131,7 @@ private[sbt] class ClassLoaderCache(
           referenceQueue.remove(1000) match {
             case ClassLoaderReference(key, classLoader) =>
               close(classLoader)
+              retired.remove(classLoader)
               delegate.remove(key)
               ()
             case _ =>
@@ -148,6 +171,10 @@ private[sbt] class ClassLoaderCache(
    * handle from being modified. On linux and mac, we probably leak some file descriptors but it's
    * fairly uncommon for sbt to run out of file descriptors.
    *
+   * Loaders evicted by clearExpiredLoaders (as opposed to by garbage collection) are a separate
+   * case: they are removed from `delegate`, so neither clear()/close() nor the reference-queue
+   * cleanup thread can reach them. Those are tracked weakly in `retired` and closed by
+   * clear()/close(), so their handles are released deterministically rather than never.
    */
   private val metaspaceIsLimited =
     ManagementFactory.getMemoryPoolMXBeans.asScala
@@ -230,6 +257,12 @@ private[sbt] class ClassLoaderCache(
         }
     }
     delegate.clear()
+    /* Also close loaders that were evicted from the delegate map but never closed (see
+     * `retired`); WeakHashMap iteration requires holding its monitor, so snapshot the keys
+     * first and close outside the lock. */
+    val evicted = retired.synchronized(new java.util.ArrayList(retired.keySet()).asScala.toList)
+    evicted.foreach(close)
+    retired.clear()
   }
 
   /**

main-command/src/test/scala/sbt/internal/ClassLoaderCacheTest.scala

@@ -52,4 +52,31 @@ object ClassLoaderCacheTest extends BasicTestSuite:
         Predef.assert(cache.get(jarClassPath) == secondLoader)
         Predef.assert(cache.get(jarClassPath) != initLoader)
 
+  test("Loaders evicted by a newer timestamp should be closed by clear()"):
+    IO.withTemporaryDirectory: dir =>
+      val entry = "leak-test-resource.txt"
+      val jar = dir.toPath.resolve("evicted.jar").toFile
+      Using.resource(new java.util.jar.JarOutputStream(new java.io.FileOutputStream(jar))): out =>
+        out.putNextEntry(new java.util.zip.ZipEntry(entry))
+        out.write("hello".getBytes("UTF-8"))
+        out.closeEntry()
+
+      withCache: cache =>
+        val classPath = jar :: Nil
+        val first = cache.get(classPath)
+        Predef.assert(first.getResource(entry) != null, "jar resource should load before eviction")
+
+        // Bumping the timestamp makes a new Key, and addLoader calls clearExpiredLoaders after
+        // inserting it, so `first`'s entry is evicted from the delegate map here. Once evicted
+        // it is unreachable from the map, so only the `retired` set can still close it.
+        IO.setModifiedTimeOrFalse(jar, System.currentTimeMillis + 5000L)
+        val second = cache.get(classPath)
+        Predef.assert(first != second, "a newer timestamp should produce a new loader")
+
+        cache.clear()
+        Predef.assert(
+          first.getResource(entry) == null,
+          "clear() should have closed the evicted loader, releasing its jar handle"
+        )
+
 end ClassLoaderCacheTest

testing/src/main/scala/sbt/JUnitXmlTestsListener.scala

@@ -15,6 +15,7 @@ import java.time.format.DateTimeFormatter
 import java.time.temporal.ChronoUnit
 import java.util.Hashtable
 import java.util.concurrent.TimeUnit.NANOSECONDS
+import java.util.concurrent.atomic.AtomicReference
 
 import scala.collection.mutable.ListBuffer
 import scala.util.Properties
@@ -196,13 +197,31 @@ class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logg
     }
   }
 
+  /**
+   * A mutable cell holding the suite that is currently running on a thread.
+   *
+   * The cell exists purely so the suite can be released deterministically. `testSuite` below
+   * is an [[InheritableThreadLocal]], so every thread created while a suite is running -- for
+   * example a pooled worker spawned by an async test framework -- receives a copy of the
+   * *reference* to this cell at construction time. `ThreadLocal.remove()` only clears the
+   * calling thread's entry, so those inherited copies would otherwise pin the `TestSuite`, its
+   * buffered events, and through them the test class loader (and its open jar handles) for the
+   * remaining life of the JVM. Clearing the cell severs the reference for the owning thread and
+   * every thread that inherited it at once.
+   */
+  private final class SuiteRef(initial: Option[TestSuite]) {
+    private val ref = new AtomicReference(initial)
+    def current: Option[TestSuite] = ref.get()
+    def clear(): Unit = ref.set(None)
+  }
+
   /** The currently running test suite */
-  private val testSuite = new InheritableThreadLocal[Option[TestSuite]] {
-    override def initialValue(): Option[TestSuite] = None
+  private val testSuite = new InheritableThreadLocal[SuiteRef] {
+    override def initialValue(): SuiteRef = new SuiteRef(None)
   }
 
   private def withTestSuite[T](f: TestSuite => T): T =
-    testSuite.get().map(f).getOrElse(sys.error("no test suite"))
+    testSuite.get().current.map(f).getOrElse(sys.error("no test suite"))
 
   /** Creates the output Dir */
   override def doInit(): Unit = {
@@ -212,14 +231,29 @@ class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logg
   /**
    * Starts a new, initially empty Suite with the given name.
    */
-  override def startGroup(name: String): Unit = testSuite.set(Some(new TestSuite(name)))
+  override def startGroup(name: String): Unit =
+    testSuite.set(new SuiteRef(Some(new TestSuite(name))))
 
   /**
    * Adds all details for the given even to the current suite.
+   *
+   * Events that arrive after the suite has been written are dropped. Test frameworks may call
+   * the event handler from threads they spawned during the run (see `TestFramework.TestRunner`),
+   * and such a thread can report after `writeSuite` has already emitted the XML. Before the
+   * suite was released those late events were appended to an already-written suite, so they
+   * were discarded in practice; dropping them here keeps that outcome without turning every
+   * late event into an error line via `TestFramework.safeForeach`.
    */
-  override def testEvent(event: TestEvent): Unit = for (e <- event.detail) {
-    withTestSuite(_.addEvent(e))
-  }
+  override def testEvent(event: TestEvent): Unit =
+    testSuite.get().current match {
+      case Some(suite) => for (e <- event.detail) suite.addEvent(e)
+      case None        =>
+        if (logger != null) {
+          logger.debug(
+            s"ignoring ${event.detail.size} test event(s) reported after the suite was written"
+          )
+        } else ()
+    }
 
   /**
    * called for each class or equivalent grouping We map one group to one Testsuite, so for each
@@ -283,6 +317,13 @@ class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logg
     }
     val testSuiteResult = withTestSuite(_.stop())
     XML.save(file, testSuiteResult, "UTF-8", xmlDecl = true, null)
+    /* Order matters: `clear()` releases the suite for this thread *and* for every thread that
+     * inherited the cell, which `remove()` cannot reach. `remove()` then drops this thread's
+     * own entry. Without the `clear()` the suite -- and through its buffered events the test
+     * class loader with its open jar handles -- would stay reachable from pooled worker threads
+     * for the life of the JVM.
+     */
+    testSuite.get().clear()
     testSuite.remove()
   }
 

testing/src/main/scala/sbt/TestReportListener.scala

@@ -44,6 +44,20 @@ trait TestsListener extends TestReportListener {
 
 /**
  * Provides the overall `result` of a group of tests (a suite) and test counts for each result type.
+ *
+ * @param throwables
+ *   The exceptions thrown by the tests in this suite, as live objects. They are needed as objects
+ *   rather than as rendered text because the ClassLoaderLayeringStrategy diagnostic in `Defaults`
+ *   inspects them with `isInstanceOf` and walks `getCause` to recognise `NoClassDefFoundError` and
+ *   friends.
+ *
+ *   RETENTION HAZARD: a `Throwable`'s backtrace references the `Class` objects of every frame, and
+ *   a `Class` strongly references its defining class loader. Holding a `SuiteResult` with a
+ *   non-empty `throwables` therefore keeps the test class loader -- and every jar handle it has
+ *   open -- alive. That is fine for the duration of the test task, which is where these are
+ *   consumed, but anything that outlives the task must drop them first. `TestRecap.collect` does
+ *   exactly that before stashing a copy on `State.attributes`; see the note on `TestRecap.recapKey`.
+ *   On Windows a leaked handle makes the underlying jar undeletable (e.g. by `clearCaches`).
  */
 private[sbt] final class SuiteResult(
     val result: TestResult,

testing/src/test/scala/sbt/JUnitXmlTestsListenerSpec.scala

@@ -102,4 +102,54 @@ object JUnitXmlTestsListenerSpec extends BasicTestSuite:
         tempDir.listFiles().foreach(_.delete())
         tempDir.delete()
 
+  test("JUnitXmlTestsListener should release the suite from threads that inherited it"):
+    IO.withTemporaryDirectory: tempDir =>
+      val listener = new JUnitXmlTestsListener(tempDir, false, null)
+      listener.doInit()
+
+      def event(name: String) = new TEvent:
+        def fullyQualifiedName = s"InheritSuite.$name"
+        def duration() = 1L
+        def status = TStatus.Success
+        def fingerprint = null
+        def selector = new TestSelector(name)
+        def throwable = new OptionalThrowable()
+
+      listener.startGroup("InheritSuite")
+
+      // A thread created *while* the suite is set inherits the suite cell, standing in for a
+      // pooled worker spawned by an async test framework during the run.
+      val suiteWritten = new java.util.concurrent.CountDownLatch(1)
+      val childDone = new java.util.concurrent.CountDownLatch(1)
+      val endGroupOutcome = new AtomicReference[Option[Throwable]](None)
+      val testEventOutcome = new AtomicReference[Option[Throwable]](None)
+      val child = new Thread(() =>
+        suiteWritten.await()
+        // The inherited cell must no longer reach a TestSuite, so the strict path fails...
+        endGroupOutcome.set(
+          scala.util.Try(listener.endGroup("InheritSuite", TestResult.Passed)).failed.toOption
+        )
+        // ...while a late event is dropped rather than raised.
+        testEventOutcome.set(
+          scala.util.Try(listener.testEvent(sbt.TestEvent(Seq(event("late"))))).failed.toOption
+        )
+        childDone.countDown()
+      )
+      child.setDaemon(true)
+      child.start()
+
+      listener.testEvent(sbt.TestEvent(Seq(event("testMethod"))))
+      listener.endGroup("InheritSuite", TestResult.Passed)
+      suiteWritten.countDown()
+      assert(childDone.await(30, java.util.concurrent.TimeUnit.SECONDS), "child thread timed out")
+
+      assert(
+        endGroupOutcome.get().isDefined,
+        "a thread that inherited the suite could still reach it after writeSuite"
+      )
+      assert(
+        testEventOutcome.get().isEmpty,
+        s"a late test event should be dropped, but threw: ${testEventOutcome.get()}"
+      )
+
 end JUnitXmlTestsListenerSpec