Don't leak the sbt metabuild classpath in run/test

Prior to this commit, it was difficult to prevent the sbt metabuild
classpath from leaking into the runtime and test classpaths. The biggest
issue is that the test-inferface jar was located in the metabuild
classpath. We tried to prevent leakage using the DualClassLoader, but
this was an ugly solution that did not seem to work reliably. The fix is
to modify the actual sbt metabuild classloader provided by the sbt
launcher.

To do this, I add a new classloader SbtMetaClassLoader that isolates the
test-interface jar from the rest of the classpath. I modify xMain to
create a new AppConfiguration that uses this new classloader and
use reflection to invoke the sbt main method using the new classloader.

Not only do I think that this is a much saner solution than DualLoaders,
I accidentally fixed #4575 with this change.

main/src/main/scala/sbt/Main.scala

@@ -8,6 +8,7 @@
 package sbt
 
 import java.io.{ File, IOException }
+import java.lang.reflect.InvocationTargetException
 import java.net.URI
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.{ Locale, Properties }
@@ -27,6 +28,7 @@ import sbt.io._
 import sbt.io.syntax._
 import sbt.util.{ Level, Logger, Show }
 import xsbti.compile.CompilerCache
+import xsbti.{ AppMain, AppProvider, ComponentProvider, ScalaProvider }
 
 import scala.annotation.tailrec
 import scala.concurrent.ExecutionContext
@@ -35,6 +37,47 @@ import scala.util.control.NonFatal
 /** This class is the entry point for sbt. */
 final class xMain extends xsbti.AppMain {
   def run(configuration: xsbti.AppConfiguration): xsbti.MainResult = {
+    val modifiedConfiguration = new ModifiedConfiguration(configuration)
+    val loader = modifiedConfiguration.provider.loader
+    // No need to memoize the old class loader. It is reset by the launcher anyway.
+    Thread.currentThread.setContextClassLoader(loader)
+    val clazz = loader.loadClass("sbt.xMainImpl$")
+    val instance = clazz.getField("MODULE$").get(null)
+    val runMethod = clazz.getMethod("run", classOf[xsbti.AppConfiguration])
+    try {
+      runMethod.invoke(instance, modifiedConfiguration).asInstanceOf[xsbti.MainResult]
+    } catch {
+      case e: InvocationTargetException =>
+        // This propogates xsbti.FullReload to the launcher
+        throw e.getCause
+    }
+  }
+  /*
+   * Replaces the AppProvider.loader method with a new loader that puts the sbt test interface
+   * jar ahead of the rest of the sbt classpath in the classloading hierarchy.
+   */
+  private class ModifiedConfiguration(val configuration: xsbti.AppConfiguration)
+      extends xsbti.AppConfiguration {
+    private[this] val initLoader = configuration.provider.loader
+    private[this] val scalaLoader = configuration.provider.scalaProvider.loader
+    private[this] val metaLoader: ClassLoader = SbtMetaBuildClassLoader(scalaLoader, initLoader)
+    private class ModifiedAppProvider(val appProvider: AppProvider) extends AppProvider {
+      override def scalaProvider(): ScalaProvider = appProvider.scalaProvider
+      override def id(): xsbti.ApplicationID = appProvider.id()
+      override def loader(): ClassLoader = metaLoader
+      override def mainClass(): Class[_ <: AppMain] = appProvider.mainClass()
+      override def entryPoint(): Class[_] = appProvider.entryPoint()
+      override def newMain(): AppMain = appProvider.newMain()
+      override def mainClasspath(): Array[File] = appProvider.mainClasspath()
+      override def components(): ComponentProvider = appProvider.components()
+    }
+    override def arguments(): Array[String] = configuration.arguments
+    override def baseDirectory(): File = configuration.baseDirectory
+    override def provider(): AppProvider = new ModifiedAppProvider(configuration.provider)
+  }
+}
+private[sbt] object xMainImpl {
+  private[sbt] def run(configuration: xsbti.AppConfiguration): xsbti.MainResult = {
     import BasicCommandStrings.{ DashClient, DashDashClient, runEarly }
     import BasicCommands.early
     import BuiltinCommands.defaults

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

@@ -9,36 +9,20 @@ package sbt
 package internal
 
 import java.io.File
-import java.net.URLClassLoader
+import java.net.{ URL, URLClassLoader }
 
+import sbt.ClassLoaderLayeringStrategy.{ ScalaInstance => ScalaInstanceLayer, _ }
 import sbt.Keys._
 import sbt.SlashSyntax0._
 import sbt.internal.inc.ScalaInstance
-import sbt.internal.inc.classpath.{ ClasspathUtilities, DualLoader, NullLoader }
+import sbt.internal.inc.classpath.ClasspathUtilities
 import sbt.internal.util.Attributed
 import sbt.internal.util.Attributed.data
 import sbt.io.IO
 import sbt.librarymanagement.Configurations.{ Runtime, Test }
-import PrettyPrint.indent
-
-import scala.annotation.tailrec
-import ClassLoaderLayeringStrategy.{ ScalaInstance => ScalaInstanceLayer, _ }
 
 private[sbt] object ClassLoaders {
-  private[this] lazy val interfaceLoader =
+  private[this] val interfaceLoader = classOf[sbt.testing.Framework].getClassLoader
-    combine(
-      classOf[sbt.testing.Framework].getClassLoader,
-      new NullLoader,
-      toString = "sbt.testing.Framework interface ClassLoader"
-    )
-  private[this] lazy val baseLoader = {
-    @tailrec
-    def getBase(classLoader: ClassLoader): ClassLoader = classLoader.getParent match {
-      case null   => classLoader
-      case loader => getBase(loader)
-    }
-    getBase(ClassLoaders.getClass.getClassLoader)
-  }
   /*
    * Get the class loader for a test task. The configuration could be IntegrationTest or Test.
    */
@@ -54,7 +38,6 @@ private[sbt] object ClassLoaders {
       rawRuntimeDependencies =
         dependencyJars(Runtime / dependencyClasspath).value.filterNot(exclude),
       allDependencies = dependencyJars(dependencyClasspath).value.filterNot(exclude),
-      base = interfaceLoader,
       runtimeCache = (Runtime / classLoaderCache).value,
       testCache = (Test / classLoaderCache).value,
       resources = ClasspathUtilities.createClasspathResources(fullCP, si),
@@ -101,7 +84,6 @@ private[sbt] object ClassLoaders {
               fullCP = classpath,
               rawRuntimeDependencies = runtimeDeps,
               allDependencies = allDeps,
-              base = baseLoader,
               runtimeCache = runtimeCache,
               testCache = testCache,
               resources = ClasspathUtilities.createClasspathResources(classpath, instance),
@@ -130,7 +112,6 @@ private[sbt] object ClassLoaders {
       fullCP: Seq[File],
       rawRuntimeDependencies: Seq[File],
       allDependencies: Seq[File],
-      base: ClassLoader,
       runtimeCache: ClassLoaderCache,
       testCache: ClassLoaderCache,
       resources: Map[String, String],
@@ -139,7 +120,7 @@ private[sbt] object ClassLoaders {
   ): ClassLoader = {
     val isTest = scope.config.toOption.map(_.name) == Option("test")
     val raw = strategy match {
-      case Flat => flatLoader(fullCP, base)
+      case Flat => flatLoader(fullCP, interfaceLoader)
       case _ =>
         val (layerDependencies, layerTestDependencies) = strategy match {
           case ShareRuntimeDependenciesLayerWithTestDependencies if isTest => (true, true)
@@ -160,7 +141,7 @@ private[sbt] object ClassLoaders {
         val allTestDependencies = if (layerTestDependencies) allDependenciesSet else Set.empty[File]
         val allRuntimeDependencies = (if (layerDependencies) rawRuntimeDependencies else Nil).toSet
 
-        val scalaInstanceLayer = combine(base, loader(si))
+        val scalaInstanceLayer = new ScalaInstanceLoader(si)
         // layer 2
         val runtimeDependencySet = allDependenciesSet intersect allRuntimeDependencies
         val runtimeDependencies = rawRuntimeDependencies.filter(runtimeDependencySet)
@@ -204,49 +185,42 @@ private[sbt] object ClassLoaders {
     if (snapshots.isEmpty) jarLoader else cache.get((snapshots, jarLoader, resources, tmp))
   }
 
-  // Code related to combining two classloaders that primarily exists so the test loader correctly
+  private class ScalaInstanceLoader(val instance: ScalaInstance)
-  // loads the testing framework using the same classloader as sbt itself.
+      extends URLClassLoader(instance.allJars.map(_.toURI.toURL), interfaceLoader) {
-  private val interfaceFilter = (name: String) =>
-    name.startsWith("org.scalatools.testing.") || name.startsWith("sbt.testing.") || name
-      .startsWith("java.") || name.startsWith("sun.")
-  private val notInterfaceFilter = (name: String) => !interfaceFilter(name)
-  private class WrappedDualLoader(
-      val parent: ClassLoader,
-      val child: ClassLoader,
-      string: => String
-  ) extends ClassLoader(
-        new DualLoader(parent, interfaceFilter, _ => false, child, notInterfaceFilter, _ => true)
-      ) {
     override def equals(o: Any): Boolean = o match {
-      case that: WrappedDualLoader => this.parent == that.parent && this.child == that.child
+      case that: ScalaInstanceLoader => this.instance.allJars.sameElements(that.instance.allJars)
-      case _                       => false
+      case _                         => false
     }
-    override def hashCode: Int = (parent.hashCode * 31) ^ child.hashCode
+    override def hashCode: Int = instance.hashCode
-    override lazy val toString: String = string
+    override lazy val toString: String =
+      s"ScalaInstanceLoader($interfaceLoader, jars = {${instance.allJars.mkString(", ")}})"
   }
-  private def combine(parent: ClassLoader, child: ClassLoader, toString: String): ClassLoader =
-    new WrappedDualLoader(parent, child, toString)
-  private def combine(parent: ClassLoader, child: ClassLoader): ClassLoader =
-    new WrappedDualLoader(
-      parent,
-      child,
-      s"WrappedDualLoader(\n  parent =\n${indent(parent, 4)}"
-        + s"\n  child =\n${indent(child, 4)}\n)"
-    )
 
   // helper methods
   private def flatLoader(classpath: Seq[File], parent: ClassLoader): ClassLoader =
     new URLClassLoader(classpath.map(_.toURI.toURL).toArray, parent)
 
-  // This makes the toString method of the ScalaInstance classloader much more readable, but
+}
-  // it is not strictly necessary.
+
-  private def loader(si: ScalaInstance): ClassLoader = new ClassLoader(si.loader) {
+private[sbt] object SbtMetaBuildClassLoader {
-    override lazy val toString: String =
+  private[this] implicit class Ops(val c: ClassLoader) {
-      "ScalaInstanceClassLoader(\n  instance = " +
+    def urls: Array[URL] = c match {
-        s"${indent(si.toString.split(",").mkString("\n  ", ",\n  ", "\n"), 4)}\n)"
+      case u: URLClassLoader => u.getURLs
-    // Delegate equals to that.equals in case that is itself some kind of wrapped classloader that
+      case cl =>
-    // needs to delegate its equals method to the delegated ClassLoader.
+        throw new IllegalStateException(s"sbt was launched with a non URLClassLoader: $cl")
-    override def equals(that: Any): Boolean = if (that != null) that.equals(si.loader) else false
+    }
-    override def hashCode: Int = si.loader.hashCode
+  }
+  def apply(libraryLoader: ClassLoader, fullLoader: ClassLoader): ClassLoader = {
+    val interfaceFilter: URL => Boolean = _.getFile.endsWith("test-interface-1.0.jar")
+    val (interfaceURL, rest) = fullLoader.urls.partition(interfaceFilter)
+    val interfaceLoader = new URLClassLoader(interfaceURL, libraryLoader.getParent) {
+      override def toString: String = s"SbtTestInterfaceClassLoader(${getURLs.head})"
+    }
+    val updatedLibraryLoader = new URLClassLoader(libraryLoader.urls, interfaceLoader) {
+      override def toString: String = s"ScalaClassLoader(jars = {${getURLs.mkString(", ")}}"
+    }
+    new URLClassLoader(rest, updatedLibraryLoader) {
+      override def toString: String = s"SbtMetaBuildClassLoader"
+    }
   }
 }

sbt/src/test/scala/sbt/RunFromSourceMain.scala

@@ -56,7 +56,7 @@ object RunFromSourceMain {
     }
 
   @tailrec private def launch(conf: AppConfiguration): Option[Int] =
-    new xMain().run(conf) match {
+    xMainImpl.run(conf) match {
       case e: xsbti.Exit     => Some(e.code)
       case _: xsbti.Continue => None
       case r: xsbti.Reboot   => launch(getConf(conf.baseDirectory(), r.arguments()))