From 621789eeb25ab2056db700631b3b017d1aab74ed Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 29 Jul 2019 11:52:42 -0700 Subject: [PATCH] Remove resource layer for AllDependencyJars strategy Changed resources were causing the dependency layer to be invalidated on resource changes in turbo mode because the resource layer was in between the scala library layer. This commit reworks the layers for the AllDependencyJars strategy so that the top layer is able to load _all_ of the resources during a test run. The resource layer was added to address the problem that dependencies may need to be able to load resources from the project classpath but wouldn't be able to do so if the dependencies were in a separate layer from the rest of the classpath. The resource layer was a classloader that could load any resource on the full classpath but couldn't load any classes. When I added the resource layer, I was thinking that when resources changed, the resource class loader needed to be invalidated. Resources, however, are different from classes in that the same ClassLoader can find the same resources in a different place because getResource and getResourceAsStream just return locations but do not actually do any loading. Taking advantage of this, I add a proxy classloader for finding resource locations to ReverseLookupClassLoader. We can reset the classpath of the resource loader in ReverseLookupClassLoaderHolder.checkout. This allows us to see the new versions of the resources without invalidating the dependency layer. --- .../java/sbt/internal/ResourceLoader.java | 23 ---------- .../scala/sbt/internal/ClassLoaders.scala | 45 ++++--------------- .../sbt/internal/LayeredClassLoaders.scala | 21 +++++++-- 3 files changed, 25 insertions(+), 64 deletions(-) delete mode 100644 main/src/main/java/sbt/internal/ResourceLoader.java diff --git a/main/src/main/java/sbt/internal/ResourceLoader.java b/main/src/main/java/sbt/internal/ResourceLoader.java deleted file mode 100644 index 9d7993c64..000000000 --- a/main/src/main/java/sbt/internal/ResourceLoader.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * sbt - * Copyright 2011 - 2018, Lightbend, Inc. - * Copyright 2008 - 2010, Mark Harrah - * Licensed under Apache License 2.0 (see LICENSE) - */ - -package sbt.internal; - -import java.io.File; -import scala.collection.immutable.Map; -import scala.collection.Seq; - -final class ResourceLoader extends ResourceLoaderImpl { - ResourceLoader( - final Seq classpath, final ClassLoader parent, final Map resources) { - super(classpath, parent, resources); - } - - static { - ClassLoader.registerAsParallelCapable(); - } -} diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 1650efc9e..866c2964b 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -35,12 +35,10 @@ 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 resourceCP = modifiedTimes((outputFileStamps in resources).value) buildLayers( strategy = classLoaderLayeringStrategy.value, si = si, fullCP = fullCP, - resourceCP = resourceCP, allDependencies = dependencyJars(dependencyClasspath).value.filterNot(exclude), cache = extendedClassLoaderCache.value, resources = ClasspathUtilities.createClasspathResources(fullCP.map(_._1), si), @@ -55,7 +53,6 @@ private[sbt] object ClassLoaders { val s = streams.value val opts = forkOptions.value val options = javaOptions.value - val resourceCP = modifiedTimes((outputFileStamps in resources).value) if (fork.value) { s.log.debug(s"javaOptions: $options") Def.task(new ForkRun(opts)) @@ -85,7 +82,6 @@ private[sbt] object ClassLoaders { strategy = classLoaderLayeringStrategy.value: @sbtUnchecked, si = instance, fullCP = classpath.map(f => f -> IO.getModifiedTimeOrZero(f)), - resourceCP = resourceCP, allDependencies = transformedDependencies, cache = extendedClassLoaderCache.value: @sbtUnchecked, resources = ClasspathUtilities.createClasspathResources(classpath, instance), @@ -118,7 +114,6 @@ private[sbt] object ClassLoaders { strategy: ClassLoaderLayeringStrategy, si: ScalaInstance, fullCP: Seq[(File, Long)], - resourceCP: Seq[(File, Long)], allDependencies: Seq[File], cache: ClassLoaderCache, resources: Map[String, String], @@ -156,34 +151,28 @@ private[sbt] object ClassLoaders { } .getOrElse(scalaLibraryLayer) - // layer 2 (resources) - val resourceLayer = - if (layerDependencies) - getResourceLayer(cpFiles, resourceCP, scalaReflectLayer, cache, resources) - else scalaReflectLayer - - // layer 3 (optional if in the test config and the runtime layer is not shared) - val dependencyLayer = + // layer 2 (optional if in the test config and the runtime layer is not shared) + val dependencyLayer: ClassLoader = if (layerDependencies && allDependencies.nonEmpty) { cache( allDependencies.toList.map(f => f -> IO.getModifiedTimeOrZero(f)), - resourceLayer, - () => new ReverseLookupClassLoaderHolder(allDependencies, resourceLayer) + scalaReflectLayer, + () => new ReverseLookupClassLoaderHolder(allDependencies, scalaReflectLayer) ) - } else resourceLayer + } else scalaReflectLayer val scalaJarNames = (si.libraryJars ++ scalaReflectJar).map(_.getName).toSet - // layer 4 + // layer 3 val filteredSet = if (layerDependencies) allDependencies.toSet ++ si.libraryJars ++ scalaReflectJar else Set(si.libraryJars ++ scalaReflectJar: _*) val dynamicClasspath = cpFiles.filterNot(f => filteredSet(f) || scalaJarNames(f.getName)) dependencyLayer match { case dl: ReverseLookupClassLoaderHolder => - dl.checkout(dynamicClasspath, tmp) + dl.checkout(cpFiles, tmp) case cl => cl.getParent match { - case dl: ReverseLookupClassLoaderHolder => dl.checkout(dynamicClasspath, tmp) + case dl: ReverseLookupClassLoaderHolder => dl.checkout(cpFiles, tmp) case _ => new LayeredClassLoader(dynamicClasspath, cl, tmp) } } @@ -194,24 +183,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"))) - // Creates a one or two layered classloader for the provided classpaths depending on whether - // or not the classpath contains any snapshots. If it does, the snapshots are placed in a layer - // above the regular jar layer. This allows the snapshot layer to be invalidated without - // invalidating the regular jar layer. If the classpath is empty, it just returns the parent - // loader. - private def getResourceLayer( - classpath: Seq[File], - resources: Seq[(File, Long)], - parent: ClassLoader, - cache: ClassLoaderCache, - resourceMap: Map[String, String] - ): ClassLoader = { - if (resources.nonEmpty) { - val mkLoader = () => new ResourceLoader(classpath, parent, resourceMap) - cache(resources.toList, parent, mkLoader) - } else parent - } - // helper methods private def flatLoader(classpath: Seq[File], parent: ClassLoader): ClassLoader = new FlatLoader(classpath.map(_.toURI.toURL).toArray, parent) diff --git a/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala b/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala index 468615dcf..869c5e39c 100644 --- a/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala @@ -8,7 +8,7 @@ package sbt.internal import java.io.File -import java.net.URLClassLoader +import java.net.{ URL, URLClassLoader } import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.{ AtomicBoolean, AtomicReference } @@ -69,7 +69,7 @@ private[internal] final class ReverseLookupClassLoaderHolder( * * @return a ClassLoader */ - def checkout(dependencyClasspath: Seq[File], tempDir: File): ClassLoader = { + def checkout(fullClasspath: 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." @@ -79,8 +79,8 @@ private[internal] final class ReverseLookupClassLoaderHolder( case null => new ReverseLookupClassLoader case c => c } - reverseLookupClassLoader.setTempDir(tempDir) - new BottomClassLoader(dependencyClasspath, reverseLookupClassLoader, tempDir) + reverseLookupClassLoader.setup(tempDir, fullClasspath) + new BottomClassLoader(fullClasspath, reverseLookupClassLoader, tempDir) } private def checkin(reverseLookupClassLoader: ReverseLookupClassLoader): Unit = { @@ -149,6 +149,19 @@ private[internal] final class ReverseLookupClassLoaderHolder( private[this] val classLoadingLock = new ClassLoadingLock def isDirty: Boolean = dirty.get() def setDescendant(classLoader: BottomClassLoader): Unit = directDescendant.set(classLoader) + private[this] val resourceLoader = new AtomicReference[ResourceLoader](null) + private class ResourceLoader(cp: Seq[File]) + extends URLClassLoader(cp.map(_.toURI.toURL).toArray, parent) { + def lookup(name: String): URL = findResource(name) + } + private[ReverseLookupClassLoaderHolder] def setup(tmpDir: File, cp: Seq[File]): Unit = { + setTempDir(tmpDir) + resourceLoader.set(new ResourceLoader(cp)) + } + override def findResource(name: String): URL = resourceLoader.get() match { + case null => null + case l => l.lookup(name) + } def loadClass(name: String, resolve: Boolean, reverseLookup: Boolean): Class[_] = { classLoadingLock.withLock(name) { try super.loadClass(name, resolve)