Add closeClassLoader setting

There have been a number of issues that have come up because of sbt
1.3.0 aggressively closing classloaders. While these issues have been
quite useful in helping us determine some issues related to classloader
lifecycle, we should give users the option to prevent sbt from closing
the classloaders.

I also noticed that the classloader-cache/spark test has been
occasionally segfaulting on travis so I disable classloader closing in
that test.

main/src/main/java/sbt/internal/BottomClassLoader.java

@@ -32,9 +32,10 @@ final class BottomClassLoader extends ManagedClassLoader {
       final URL[] dynamicClasspath,
       final ReverseLookupClassLoader reverseLookupClassLoader,
       final File tempDir,
+      final boolean close,
       final boolean allowZombies,
       final Logger logger) {
-    super(dynamicClasspath, reverseLookupClassLoader, allowZombies, logger);
+    super(dynamicClasspath, reverseLookupClassLoader, close, allowZombies, logger);
     setTempDir(tempDir);
     this.holder = holder;
     this.parent = reverseLookupClassLoader;

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

@@ -20,9 +20,10 @@ final class FlatLoader extends ManagedClassLoader {
       final URL[] urls,
       final ClassLoader parent,
       final File file,
+      final boolean close,
       final boolean allowZombies,
       final Logger logger) {
-    super(urls, parent, allowZombies, logger);
+    super(urls, parent, close, allowZombies, logger);
     setTempDir(file);
   }
 

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

@@ -12,9 +12,9 @@ import java.net.URL;
 import sbt.util.Logger;
 
 final class LayeredClassLoader extends ManagedClassLoader {
-  LayeredClassLoader(final URL[] classpath, final ClassLoader parent, final File tempDir, final
+  LayeredClassLoader(final URL[] classpath, final ClassLoader parent, final File tempDir, final boolean close, final
       boolean allowZombies, final Logger logger) {
-    super(classpath, parent, allowZombies, logger);
+    super(classpath, parent, close, allowZombies, logger);
     setTempDir(tempDir);
   }
 

main/src/main/java/sbt/internal/ManagedClassLoader.java

@@ -20,6 +20,7 @@ abstract class ManagedClassLoader extends URLClassLoader implements NativeLoader
   private final AtomicBoolean closed = new AtomicBoolean(false);
   private final AtomicBoolean printedWarning = new AtomicBoolean(false);
   private final AtomicReference<ZombieClassLoader> zombieLoader = new AtomicReference<>();
+  private final boolean close;
   private final boolean allowZombies;
   private final Logger logger;
   private final NativeLookup nativeLookup = new NativeLookup();
@@ -29,8 +30,9 @@ abstract class ManagedClassLoader extends URLClassLoader implements NativeLoader
   }
 
   ManagedClassLoader(
-      final URL[] urls, final ClassLoader parent, final boolean allowZombies, final Logger logger) {
+      final URL[] urls, final ClassLoader parent, final boolean close, final boolean allowZombies, final Logger logger) {
     super(urls, parent);
+    this.close = close;
     this.allowZombies = allowZombies;
     this.logger = logger;
   }
@@ -103,8 +105,8 @@ abstract class ManagedClassLoader extends URLClassLoader implements NativeLoader
   @Override
   public void close() throws IOException {
     final ZombieClassLoader zb = zombieLoader.getAndSet(null);
-    if (zb != null) zb.close();
+    if (zb != null && close) zb.close();
-    if (closed.compareAndSet(false, true)) super.close();
+    if (close && closed.compareAndSet(false, true)) super.close();
   }
 
   @Override

main/src/main/java/sbt/internal/ReverseLookupClassLoader.java

@@ -36,8 +36,12 @@ import sbt.util.Logger;
  */
 final class ReverseLookupClassLoader extends ManagedClassLoader {
   ReverseLookupClassLoader(
-      final URL[] urls, final ClassLoader parent, final boolean allowZombies, final Logger logger) {
+      final URL[] urls,
-    super(urls, parent, allowZombies, logger);
+      final ClassLoader parent,
+      final boolean close,
+      final boolean allowZombies,
+      final Logger logger) {
+    super(urls, parent, close, allowZombies, logger);
     this.parent = parent;
   }
 

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

@@ -205,7 +205,8 @@ object Defaults extends BuildCommon {
       bgStop := bgStopTask.evaluated,
       bgWaitFor := bgWaitForTask.evaluated,
       bgCopyClasspath :== true,
-      allowZombieClassLoaders :== !SysProp.closeClassLoaders,
+      closeClassLoaders :== SysProp.closeClassLoaders,
+      allowZombieClassLoaders :== true,
     )
 
   private[sbt] lazy val globalIvyCore: Seq[Setting[_]] =

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

@@ -327,6 +327,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 closeClassLoaders = settingKey[Boolean]("Close classloaders in run and test when the task completes.").withRank(DSetting)
   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)

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

@@ -44,6 +44,7 @@ private[sbt] object ClassLoaders {
       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 close = closeClassLoaders.value
     val allowZombies = allowZombieClassLoaders.value
     buildLayers(
       strategy = classLoaderLayeringStrategy.value,
@@ -55,6 +56,7 @@ private[sbt] object ClassLoaders {
       tmp = IO.createUniqueDirectory(taskTemporaryDirectory.value),
       scope = resolvedScoped.value.scope,
       logger = logger,
+      close = close,
       allowZombies = allowZombies,
     )
   }
@@ -88,6 +90,7 @@ private[sbt] object ClassLoaders {
         val allDeps = dependencyJars(dependencyClasspath).value.filterNot(exclude)
         val logger = state.value.globalLogging.full
         val allowZombies = allowZombieClassLoaders.value
+        val close = closeClassLoaders.value
         val newLoader =
           (classpath: Seq[File]) => {
             val mappings = classpath.map(f => f.getName -> f).toMap
@@ -102,6 +105,7 @@ private[sbt] object ClassLoaders {
               tmp = taskTemporaryDirectory.value: @sbtUnchecked,
               scope = resolvedScope,
               logger = logger,
+              close = close,
               allowZombies = allowZombies,
             )
           }
@@ -136,11 +140,12 @@ private[sbt] object ClassLoaders {
       tmp: File,
       scope: Scope,
       logger: Logger,
+      close: Boolean,
       allowZombies: Boolean
   ): ClassLoader = {
     val cpFiles = fullCP.map(_._1)
     strategy match {
-      case Flat => new FlatLoader(cpFiles.urls, interfaceLoader, tmp, allowZombies, logger)
+      case Flat => new FlatLoader(cpFiles.urls, interfaceLoader, tmp, close, allowZombies, logger)
       case _ =>
         val layerDependencies = strategy match {
           case _: AllLibraryJars => true
@@ -206,6 +211,7 @@ private[sbt] object ClassLoaders {
                 new ReverseLookupClassLoaderHolder(
                   otherDependencies,
                   frameworkLayer,
+                  close,
                   allowZombies,
                   logger
                 )
@@ -225,7 +231,7 @@ private[sbt] object ClassLoaders {
             cl.getParent match {
               case dl: ReverseLookupClassLoaderHolder => dl.checkout(dynamicClasspath, tmp)
               case _ =>
-                new LayeredClassLoader(dynamicClasspath.urls, cl, tmp, allowZombies, logger)
+                new LayeredClassLoader(dynamicClasspath.urls, cl, tmp, close, allowZombies, logger)
             }
         }
     }

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

@@ -42,6 +42,7 @@ import scala.collection.JavaConverters._
 private[internal] final class ReverseLookupClassLoaderHolder(
     val classpath: Seq[File],
     val parent: ClassLoader,
+    val closeThis: Boolean,
     val allowZombies: Boolean,
     val logger: Logger
 ) extends URLClassLoader(Array.empty, null) {
@@ -62,7 +63,7 @@ private[internal] final class ReverseLookupClassLoaderHolder(
       throw new IllegalStateException(msg)
     }
     val reverseLookupClassLoader = cached.getAndSet(null) match {
-      case null => new ReverseLookupClassLoader(urls, parent, allowZombies, logger)
+      case null => new ReverseLookupClassLoader(urls, parent, closeThis, allowZombies, logger)
       case c    => c
     }
     reverseLookupClassLoader.setup(tempDir)
@@ -71,6 +72,7 @@ private[internal] final class ReverseLookupClassLoaderHolder(
       dynamicClasspath.map(_.toURI.toURL).toArray,
       reverseLookupClassLoader,
       tempDir,
+      closeThis,
       allowZombies,
       logger
     )

sbt/src/sbt-test/classloader-cache/spark/build.sbt

@@ -5,3 +5,5 @@ version := "1.0"
 scalaVersion := "2.12.10"
 
 libraryDependencies += "org.apache.spark" %% "spark-sql" % "2.4.3"
+
+Compile / closeClassLoaders := false