diff --git a/main/src/main/java/sbt/internal/ReverseLookupClassLoader.java b/main/src/main/java/sbt/internal/ReverseLookupClassLoader.java index 5a155005c..2bce9f755 100644 --- a/main/src/main/java/sbt/internal/ReverseLookupClassLoader.java +++ b/main/src/main/java/sbt/internal/ReverseLookupClassLoader.java @@ -8,9 +8,7 @@ package sbt.internal; import java.io.File; -import java.io.IOException; import java.net.URL; -import java.net.URLClassLoader; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import sbt.util.Logger; @@ -46,7 +44,6 @@ final class ReverseLookupClassLoader extends ManagedClassLoader { private final AtomicReference directDescendant = new AtomicReference<>(); private final AtomicBoolean dirty = new AtomicBoolean(false); private final ClassLoadingLock classLoadingLock = new ClassLoadingLock(); - private final AtomicReference resourceLoader = new AtomicReference<>(); private final ClassLoader parent; boolean isDirty() { @@ -57,26 +54,14 @@ final class ReverseLookupClassLoader extends ManagedClassLoader { directDescendant.set(bottomClassLoader); } - private class ResourceLoader extends URLClassLoader { - ResourceLoader(final URL[] urls) { - super(urls, parent); - } - - final URL lookup(final String name) { - return findResource(name); - } - } - @Override public URL findResource(String name) { - final ResourceLoader loader = resourceLoader.get(); - return loader == null ? null : loader.lookup(name); + final URL url = super.findResource(name); + return url != null ? url : directDescendant.get().findResource(name); } - void setup(final File tmpDir, final URL[] urls) throws IOException { + void setup(final File tmpDir) { setTempDir(tmpDir); - final ResourceLoader previous = resourceLoader.getAndSet(new ResourceLoader(urls)); - if (previous != null) previous.close(); } @Override diff --git a/main/src/main/java/sbt/internal/ScalaTestFrameworkClassLoader.java b/main/src/main/java/sbt/internal/ScalaTestFrameworkClassLoader.java new file mode 100644 index 000000000..33d420bce --- /dev/null +++ b/main/src/main/java/sbt/internal/ScalaTestFrameworkClassLoader.java @@ -0,0 +1,34 @@ +/* + * sbt + * Copyright 2011 - 2018, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt.internal; + + import java.net.URL; + import java.net.URLClassLoader; + +final class ScalaTestFrameworkClassLoader extends URLClassLoader { + static { + ClassLoader.registerAsParallelCapable(); + } + + private final URL[] jars; + + ScalaTestFrameworkClassLoader(final URL[] jars, final ClassLoader parent) { + super(jars, parent); + this.jars = jars; + } + + @Override + public String toString() { + final StringBuilder builder = new StringBuilder(); + for (int i = 0; i < jars.length; ++ i) { + builder.append(jars[i].toString()); + if (i < jars.length - 2) builder.append(", "); + } + return "ScalaTestFrameworkClassLoader(" + builder + " parent = " + getParent() + ")"; + } +} diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index bdf25ebe6..930124037 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -156,10 +156,25 @@ private[sbt] object ClassLoaders { val cpFiles = fullCP.map(_._1) val allDependencies = cpFiles.filter(allDependenciesSet) - val scalaReflectJar = allDependencies.collectFirst { - case f if f.getName == "scala-reflect.jar" => - si.allJars.find(_.getName == "scala-reflect.jar") - }.flatten + /* + * We need to put scalatest and its dependencies (scala-xml) in its own layer so that the + * classloader for scalatest is generally not ever closed. This is because one project + * closing its scalatest loader can cause a crash in another project if that project + * is reading the scalatest bundle properties because the underlying JarFile instance is + * shared across URLClassLoaders if it points to the same jar. + * + * TODO: Make this configurable and/or add other frameworks in this layer. + */ + val (frameworkDependencies, otherDependencies) = allDependencies.partition { d => + val name = d.getName + name.startsWith("scalatest_") || name.startsWith("scalactic_") || + name.startsWith("scala-xml_") + } + def isReflectJar(f: File) = + f.getName == "scala-reflect.jar" || f.getName.startsWith("scala-reflect-") + val scalaReflectJar = otherDependencies.collectFirst { + case f if isReflectJar(f) => si.allJars.find(isReflectJar).getOrElse(f) + } val scalaReflectLayer = scalaReflectJar .map { file => cache.apply( @@ -170,34 +185,45 @@ private[sbt] object ClassLoaders { } .getOrElse(scalaLibraryLayer) - // layer 2 (optional if in the test config and the runtime layer is not shared) + // layer 2 framework jars + val frameworkLayer = + if (frameworkDependencies.isEmpty) scalaReflectLayer + else { + cache.apply( + frameworkDependencies.map(file => file -> IO.getModifiedTimeOrZero(file)).toList, + scalaLibraryLayer, + () => new ScalaTestFrameworkClassLoader(frameworkDependencies.urls, scalaReflectLayer) + ) + } + + // layer 3 (optional if in the test config and the runtime layer is not shared) val dependencyLayer: ClassLoader = - if (layerDependencies && allDependencies.nonEmpty) { + if (layerDependencies && otherDependencies.nonEmpty) { cache( - allDependencies.toList.map(f => f -> IO.getModifiedTimeOrZero(f)), - scalaReflectLayer, + otherDependencies.toList.map(f => f -> IO.getModifiedTimeOrZero(f)), + frameworkLayer, () => new ReverseLookupClassLoaderHolder( - allDependencies, - scalaReflectLayer, + otherDependencies, + frameworkLayer, allowZombies, logger ) ) - } else scalaReflectLayer + } else frameworkLayer val scalaJarNames = (si.libraryJars ++ scalaReflectJar).map(_.getName).toSet // layer 3 val filteredSet = if (layerDependencies) allDependencies.toSet ++ si.libraryJars ++ scalaReflectJar - else Set(si.libraryJars ++ scalaReflectJar: _*) + else Set(frameworkDependencies ++ si.libraryJars ++ scalaReflectJar: _*) val dynamicClasspath = cpFiles.filterNot(f => filteredSet(f) || scalaJarNames(f.getName)) dependencyLayer match { case dl: ReverseLookupClassLoaderHolder => - dl.checkout(cpFiles, tmp) + dl.checkout(dynamicClasspath, tmp) case cl => cl.getParent match { - case dl: ReverseLookupClassLoaderHolder => dl.checkout(cpFiles, tmp) + case dl: ReverseLookupClassLoaderHolder => dl.checkout(dynamicClasspath, tmp) case _ => new LayeredClassLoader(dynamicClasspath.urls, cl, tmp, allowZombies, logger) } diff --git a/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala b/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala index 3f50f8c2a..ae56b2723 100644 --- a/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala @@ -55,7 +55,7 @@ private[internal] final class ReverseLookupClassLoaderHolder( * * @return a ClassLoader */ - def checkout(fullClasspath: Seq[File], tempDir: File): ClassLoader = { + def checkout(dynamicClasspath: Seq[File], tempDir: File): ClassLoader = { if (closed.get()) { val msg = "Tried to extract class loader from closed ReverseLookupClassLoaderHolder. " + "Try running the `clearCaches` command and re-trying." @@ -65,10 +65,10 @@ private[internal] final class ReverseLookupClassLoaderHolder( case null => new ReverseLookupClassLoader(urls, parent, allowZombies, logger) case c => c } - reverseLookupClassLoader.setup(tempDir, fullClasspath.map(_.toURI.toURL).toArray) + reverseLookupClassLoader.setup(tempDir) new BottomClassLoader( ReverseLookupClassLoaderHolder.this, - fullClasspath.map(_.toURI.toURL).toArray, + dynamicClasspath.map(_.toURI.toURL).toArray, reverseLookupClassLoader, tempDir, allowZombies,