Merge pull request #5083 from eatkins/zombie-classloaders

Allow classloaders to be used after close

build.sbt

@@ -121,7 +121,7 @@ def sbt10Plus =
     "1.2.7",
     "1.2.8",
   ) ++ sbt13Plus
-def sbt13Plus = Seq() // Add sbt 1.3+ stable versions when released
+def sbt13Plus = Seq("1.3.0")
 
 def mimaSettings = mimaSettingsSince(sbt10Plus)
 def mimaSettingsSince(versions: Seq[String]) = Def settings (
@@ -672,12 +672,14 @@ lazy val mainProj = (project in file("main"))
       exclude[ReversedMissingMethodProblem]("sbt.internal.KeyIndex.*"),
       // internal
       exclude[IncompatibleMethTypeProblem]("sbt.internal.server.LanguageServerReporter.*"),
+
       // Changed signature or removed private[sbt] methods
       exclude[DirectMissingMethodProblem]("sbt.Classpaths.unmanagedLibs0"),
       exclude[DirectMissingMethodProblem]("sbt.Defaults.allTestGroupsTask"),
       exclude[DirectMissingMethodProblem]("sbt.Plugins.topologicalSort"),
       exclude[IncompatibleMethTypeProblem]("sbt.Defaults.allTestGroupsTask"),
-      exclude[DirectMissingMethodProblem]("sbt.StandardMain.shutdownHook")
+      exclude[DirectMissingMethodProblem]("sbt.StandardMain.shutdownHook"),
+      exclude[MissingClassProblem]("sbt.internal.ResourceLoaderImpl"),
     )
   )
   .configure(

main/src/main/java/sbt/internal/FlatLoader.java

@@ -7,17 +7,23 @@
 
 package sbt.internal;
 
-import java.io.IOException;
+import java.io.File;
 import java.net.URL;
-import java.net.URLClassLoader;
+import sbt.util.Logger;
+import scala.collection.Seq;
 
-final class FlatLoader extends URLClassLoader {
+final class FlatLoader extends LayeredClassLoaderImpl {
   static {
     ClassLoader.registerAsParallelCapable();
   }
 
-  FlatLoader(final URL[] urls, final ClassLoader parent) {
-    super(urls, parent);
+  FlatLoader(
+      final Seq<File> files,
+      final ClassLoader parent,
+      final File file,
+      final boolean allowZombies,
+      final Logger logger) {
+    super(files, parent, file, allowZombies, logger);
   }
 
   @Override
@@ -30,9 +36,4 @@ final class FlatLoader extends URLClassLoader {
     }
     return "FlatLoader(\n  parent = " + getParent() + "\n  jars = " + jars.toString() + ")";
   }
-
-  @Override
-  public void close() throws IOException {
-    if (SysProp.closeClassLoaders()) super.close();
-  }
 }

main/src/main/java/sbt/internal/LayeredClassLoader.java

@@ -8,14 +8,13 @@
 package sbt.internal;
 
 import java.io.File;
+import sbt.util.Logger;
 import scala.collection.Seq;
 
 final class LayeredClassLoader extends LayeredClassLoaderImpl {
-  LayeredClassLoader(
-      final Seq<File> classpath,
-      final ClassLoader parent,
-      final File tempDir) {
-    super(classpath, parent, tempDir);
+  LayeredClassLoader(final Seq<File> classpath, final ClassLoader parent, final File tempDir, final
+      boolean allowZombies, final Logger logger) {
+    super(classpath, parent, tempDir, allowZombies, logger);
   }
 
   static {

main/src/main/java/sbt/internal/ScalaLibraryClassLoader.java

@@ -7,7 +7,6 @@
 
 package sbt.internal;
 
-import java.io.IOException;
 import java.net.URL;
 import java.net.URLClassLoader;
 
@@ -32,9 +31,4 @@ final class ScalaLibraryClassLoader extends URLClassLoader {
     }
     return "ScalaLibraryClassLoader(" + builder + " parent = " + getParent() + ")";
   }
-
-  @Override
-  public void close() throws IOException {
-    if (SysProp.closeClassLoaders()) super.close();
-  }
 }

main/src/main/java/sbt/internal/ScalaReflectClassLoader.java

@@ -7,7 +7,6 @@
 
 package sbt.internal;
 
-import java.io.IOException;
 import java.net.URL;
 import java.net.URLClassLoader;
 
@@ -27,9 +26,4 @@ final class ScalaReflectClassLoader extends URLClassLoader {
   public String toString() {
     return "ScalaReflectClassLoader(" + jar + " parent = " + getParent() + ")";
   }
-
-  @Override
-  public void close() throws IOException {
-    if (SysProp.closeClassLoaders()) super.close();
-  }
 }

main/src/main/scala/sbt/Defaults.scala

@@ -32,7 +32,7 @@ import sbt.internal.CommandStrings.ExportStream
 import sbt.internal._
 import sbt.internal.classpath.AlternativeZincUtil
 import sbt.internal.inc.JavaInterfaceUtil._
-import sbt.internal.inc.classpath.ClasspathFilter
+import sbt.internal.inc.classpath.{ ClassLoaderCache, ClasspathFilter }
 import sbt.internal.inc.{ ZincLmUtil, ZincUtil }
 import sbt.internal.io.{ Source, WatchState }
 import sbt.internal.librarymanagement.mavenint.{
@@ -47,7 +47,6 @@ import sbt.internal.server.{
   LanguageServerReporter,
   ServerHandler
 }
-import sbt.nio.FileStamp.Formats.seqPathFileStampJsonFormatter
 import sbt.internal.testing.TestLogger
 import sbt.internal.util.Attributed.data
 import sbt.internal.util.Types._
@@ -70,6 +69,7 @@ import sbt.librarymanagement.CrossVersion.{ binarySbtVersion, binaryScalaVersion
 import sbt.librarymanagement._
 import sbt.librarymanagement.ivy._
 import sbt.librarymanagement.syntax._
+import sbt.nio.FileStamp.Formats.seqPathFileStampJsonFormatter
 import sbt.nio.Keys._
 import sbt.nio.file.syntax._
 import sbt.nio.file.{ FileTreeView, Glob, RecursiveGlob }
@@ -205,6 +205,7 @@ object Defaults extends BuildCommon {
       bgStop := bgStopTask.evaluated,
       bgWaitFor := bgWaitForTask.evaluated,
       bgCopyClasspath :== true,
+      allowZombieClassLoaders :== !SysProp.closeClassLoaders,
     )
 
   private[sbt] lazy val globalIvyCore: Seq[Setting[_]] =
@@ -813,7 +814,7 @@ object Defaults extends BuildCommon {
       allJars: Seq[File],
       libraryJars: Array[File],
       compilerJar: File,
-      classLoaderCache: sbt.internal.inc.classpath.ClassLoaderCache
+      classLoaderCache: ClassLoaderCache,
   ): ScalaInstance = {
     val allJarsDistinct = allJars.distinct
     val libraryLoader = classLoaderCache(libraryJars.toList)
@@ -837,7 +838,7 @@ object Defaults extends BuildCommon {
     val dummy = ScalaInstance(dir)(state.value.classLoaderCache.apply)
     Seq(dummy.loader, dummy.loaderLibraryOnly).foreach {
       case a: AutoCloseable => a.close()
-      case cl               =>
+      case _                =>
     }
     mkScalaInstance(
       dummy.version,

main/src/main/scala/sbt/Keys.scala

@@ -325,6 +325,7 @@ object Keys {
   val dependencyClasspathAsJars = taskKey[Classpath]("The classpath consisting of internal and external, managed and unmanaged dependencies, all as JARs.")
   val fullClasspathAsJars = taskKey[Classpath]("The exported classpath, consisting of build products and unmanaged and managed, internal and external dependencies, all as JARs.")
   val internalDependencyConfigurations = settingKey[Seq[(ProjectRef, Set[String])]]("The project configurations that this configuration depends on")
+  val allowZombieClassLoaders = settingKey[Boolean]("Allow a classloader that has previously been closed by `run` or `test` to continue loading classes.")
 
   val useCoursier = settingKey[Boolean]("Use Coursier for dependency resolution.").withRank(BSetting)
   val csrCacheDirectory = settingKey[File]("Coursier cache directory. Uses -Dsbt.coursier.home or Coursier's default.").withRank(CSetting)

main/src/main/scala/sbt/internal/ClassLoaders.scala

@@ -22,6 +22,7 @@ import sbt.io.IO
 import sbt.nio.FileStamp
 import sbt.nio.FileStamp.LastModified
 import sbt.nio.Keys._
+import sbt.util.Logger
 
 private[sbt] object ClassLoaders {
   private[this] val interfaceLoader = classOf[sbt.testing.Framework].getClassLoader
@@ -38,6 +39,8 @@ private[sbt] object ClassLoaders {
       if (si.isManagedVersion) rawCP
       else si.libraryJars.map(j => j -> IO.getModifiedTimeOrZero(j)).toSeq ++ rawCP
     val exclude = dependencyJars(exportedProducts).value.toSet ++ si.libraryJars
+    val logger = state.value.globalLogging.full
+    val allowZombies = allowZombieClassLoaders.value
     buildLayers(
       strategy = classLoaderLayeringStrategy.value,
       si = si,
@@ -46,7 +49,9 @@ private[sbt] object ClassLoaders {
       cache = extendedClassLoaderCache.value,
       resources = ClasspathUtilities.createClasspathResources(fullCP.map(_._1), si),
       tmp = IO.createUniqueDirectory(taskTemporaryDirectory.value),
-      scope = resolvedScoped.value.scope
+      scope = resolvedScoped.value.scope,
+      logger = logger,
+      allowZombies = allowZombies,
     )
   }
 
@@ -77,6 +82,8 @@ private[sbt] object ClassLoaders {
         }
         val exclude = dependencyJars(exportedProducts).value.toSet ++ instance.libraryJars
         val allDeps = dependencyJars(dependencyClasspath).value.filterNot(exclude)
+        val logger = state.value.globalLogging.full
+        val allowZombies = allowZombieClassLoaders.value
         val newLoader =
           (classpath: Seq[File]) => {
             val mappings = classpath.map(f => f.getName -> f).toMap
@@ -89,7 +96,9 @@ private[sbt] object ClassLoaders {
               cache = extendedClassLoaderCache.value: @sbtUnchecked,
               resources = ClasspathUtilities.createClasspathResources(classpath, instance),
               tmp = taskTemporaryDirectory.value: @sbtUnchecked,
-              scope = resolvedScope
+              scope = resolvedScope,
+              logger = logger,
+              allowZombies = allowZombies,
             )
           }
         new Run(newLoader, trapExit.value)
@@ -121,11 +130,13 @@ private[sbt] object ClassLoaders {
       cache: ClassLoaderCache,
       resources: Map[String, String],
       tmp: File,
-      scope: Scope
+      scope: Scope,
+      logger: Logger,
+      allowZombies: Boolean
   ): ClassLoader = {
     val cpFiles = fullCP.map(_._1)
     strategy match {
-      case Flat => flatLoader(cpFiles, interfaceLoader)
+      case Flat => new FlatLoader(cpFiles, interfaceLoader, tmp, allowZombies, logger)
       case _ =>
         val layerDependencies = strategy match {
           case _: AllLibraryJars => true
@@ -161,7 +172,13 @@ private[sbt] object ClassLoaders {
             cache(
               allDependencies.toList.map(f => f -> IO.getModifiedTimeOrZero(f)),
               scalaReflectLayer,
-              () => new ReverseLookupClassLoaderHolder(allDependencies, scalaReflectLayer)
+              () =>
+                new ReverseLookupClassLoaderHolder(
+                  allDependencies,
+                  scalaReflectLayer,
+                  allowZombies,
+                  logger
+                )
             )
           } else scalaReflectLayer
 
@@ -177,7 +194,7 @@ private[sbt] object ClassLoaders {
           case cl =>
             cl.getParent match {
               case dl: ReverseLookupClassLoaderHolder => dl.checkout(cpFiles, tmp)
-              case _                                  => new LayeredClassLoader(dynamicClasspath, cl, tmp)
+              case _                                  => new LayeredClassLoader(dynamicClasspath, cl, tmp, allowZombies, logger)
             }
         }
     }
@@ -187,9 +204,6 @@ private[sbt] object ClassLoaders {
       key: sbt.TaskKey[Seq[Attributed[File]]]
   ): Def.Initialize[Task[Seq[File]]] = Def.task(data(key.value).filter(_.getName.endsWith(".jar")))
 
-  // helper methods
-  private def flatLoader(classpath: Seq[File], parent: ClassLoader): ClassLoader =
-    new FlatLoader(classpath.map(_.toURI.toURL).toArray, parent)
   private[this] def modifiedTimes(stamps: Seq[(Path, FileStamp)]): Seq[(File, Long)] = stamps.map {
     case (p, LastModified(lm)) => p.toFile -> lm
     case (p, _) =>

main/src/main/scala/sbt/internal/LayeredClassLoaders.scala

@@ -12,8 +12,8 @@ import java.net.{ URL, URLClassLoader }
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.{ AtomicBoolean, AtomicReference }
 
-import sbt.internal.inc.classpath._
 import sbt.io.IO
+import sbt.util.Logger
 
 import scala.collection.JavaConverters._
 
@@ -27,11 +27,11 @@ import scala.collection.JavaConverters._
 private[internal] class LayeredClassLoaderImpl(
     classpath: Seq[File],
     parent: ClassLoader,
-    tempDir: File
-) extends URLClassLoader(classpath.map(_.toURI.toURL).toArray, parent)
-    with NativeLoader {
+    tempDir: File,
+    allowZombies: Boolean,
+    logger: Logger
+) extends ManagedClassLoader(classpath.toArray.map(_.toURI.toURL), parent, allowZombies, logger) {
   setTempDir(tempDir)
-  override def close(): Unit = if (SysProp.closeClassLoaders) super.close()
 }
 
 /**
@@ -58,10 +58,13 @@ private[internal] class LayeredClassLoaderImpl(
  */
 private[internal] final class ReverseLookupClassLoaderHolder(
     val classpath: Seq[File],
-    val parent: ClassLoader
+    val parent: ClassLoader,
+    val allowZombies: Boolean,
+    val logger: Logger
 ) extends URLClassLoader(Array.empty, null) {
   private[this] val cached: AtomicReference[ReverseLookupClassLoader] = new AtomicReference
   private[this] val closed = new AtomicBoolean(false)
+  private[this] val urls = classpath.map(_.toURI.toURL).toArray
 
   /**
    * Get a classloader. If there is a loader available in the cache, it will use that loader,
@@ -141,8 +144,9 @@ private[internal] final class ReverseLookupClassLoaderHolder(
    *
    */
   private class ReverseLookupClassLoader
-      extends URLClassLoader(classpath.map(_.toURI.toURL).toArray, parent)
+      extends ManagedClassLoader(urls, parent, allowZombies, logger)
       with NativeLoader {
+    override def getURLs: Array[URL] = urls
     private[this] val directDescendant: AtomicReference[BottomClassLoader] =
       new AtomicReference
     private[this] val dirty = new AtomicBoolean(false)
@@ -179,7 +183,6 @@ private[internal] final class ReverseLookupClassLoaderHolder(
     }
     override def loadClass(name: String, resolve: Boolean): Class[_] =
       loadClass(name, resolve, reverseLookup = true)
-    override def close(): Unit = if (SysProp.closeClassLoaders) super.close()
   }
 
   /**
@@ -202,7 +205,12 @@ private[internal] final class ReverseLookupClassLoaderHolder(
       dynamicClasspath: Seq[File],
       parent: ReverseLookupClassLoader,
       tempDir: File
-  ) extends URLClassLoader(dynamicClasspath.map(_.toURI.toURL).toArray, parent)
+  ) extends ManagedClassLoader(
+        dynamicClasspath.map(_.toURI.toURL).toArray,
+        parent,
+        allowZombies,
+        logger
+      )
       with NativeLoader {
     parent.setDescendant(this)
     setTempDir(tempDir)
@@ -236,7 +244,7 @@ private[internal] final class ReverseLookupClassLoaderHolder(
     }
     override def close(): Unit = {
       checkin(parent)
-      if (SysProp.closeClassLoaders) super.close()
+      super.close()
     }
   }
 }
@@ -294,21 +302,6 @@ private trait NativeLoader extends ClassLoader with AutoCloseable {
   }
 }
 
-private[internal] class ResourceLoaderImpl(
-    classpath: Seq[File],
-    parent: ClassLoader,
-    override val resources: Map[String, String]
-) extends URLClassLoader(classpath.map(_.toURI.toURL).toArray, parent)
-    with RawResources {
-  override def findClass(name: String): Class[_] = throw new ClassNotFoundException(name)
-  override def loadClass(name: String, resolve: Boolean): Class[_] = {
-    val clazz = parent.loadClass(name)
-    if (resolve) resolveClass(clazz)
-    clazz
-  }
-  override def toString: String = "ResourceLoader"
-}
-
 private[internal] object NativeLibs {
   private[this] val nativeLibs = new java.util.HashSet[File].asScala
   ShutdownHooks.add(() => {
@@ -327,3 +320,64 @@ private[internal] object NativeLibs {
     ()
   }
 }
+
+private sealed abstract class ManagedClassLoader(
+    urls: Array[URL],
+    parent: ClassLoader,
+    allowZombies: Boolean,
+    logger: Logger
+) extends URLClassLoader(urls, parent)
+    with NativeLoader {
+  private[this] val closed = new AtomicBoolean(false)
+  private[this] val printedWarning = new AtomicBoolean(false)
+  private[this] val zombieLoader = new AtomicReference[ZombieClassLoader]
+  private class ZombieClassLoader extends URLClassLoader(urls, this) {
+    def lookupClass(name: String): Class[_] =
+      try findClass(name)
+      catch {
+        case e: ClassNotFoundException =>
+          val deleted = urls.flatMap { u =>
+            val f = new File(u.getPath)
+            if (f.exists) None else Some(f)
+          }
+          if (deleted.toSeq.nonEmpty) {
+            // TODO - add doc link
+            val msg = s"Couldn't load class $name. " +
+              s"The following urls on the classpath do not exist:\n${deleted mkString "\n"}\n" +
+              "This may be due to shutdown hooks added during an invocation of `run`."
+            // logging may be shutdown at this point so we need to print directly to System.err.
+            System.err.println(msg)
+          }
+          throw e
+      }
+  }
+  private def getZombieLoader(name: String): ZombieClassLoader = {
+    if (printedWarning.compareAndSet(false, true) && !allowZombies) {
+      // TODO - Need to add link to documentation in website
+      val thread = Thread.currentThread
+      val msg =
+        s"$thread loading $name after test or run has completed. This is a likely resource leak."
+      logger.warn(msg)
+    }
+    zombieLoader.get match {
+      case null =>
+        val zb = new ZombieClassLoader
+        zombieLoader.set(zb)
+        zb
+      case zb => zb
+    }
+  }
+  override def findResource(name: String): URL = {
+    if (closed.get) getZombieLoader(name).findResource(name)
+    else super.findResource(name)
+  }
+  override def findClass(name: String): Class[_] = {
+    if (closed.get) getZombieLoader(name).lookupClass(name)
+    else super.findClass(name)
+  }
+  override def close(): Unit = {
+    closed.set(true)
+    Option(zombieLoader.getAndSet(null)).foreach(_.close())
+    super.close()
+  }
+}