From a3cde88db44b0cefd900c43733fc54df8a36a555 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 14:00:48 -0700 Subject: [PATCH 1/7] Fix runtime scala-reflect layer For best caching performance, we want to use the scala-reflect.jar that is found in the scala instance. Also, in the runtime configuration, caching didn't work correctly because we filtered the scala reflect library from the dependency jars. We really only wanted to filter out the library jars. It also was problematic to use a LayeredClassLoader for the scala reflect layer because in a subsequent commit I add the capability for a layered classloader to load classes from its descendant loaders. This caused problems when the scala-reflect layer was a LayeredClassLoader. Instead, I add the ScalaReflectClassLoader class for better reporting. --- .../scala/sbt/internal/ClassLoaders.scala | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 773ec4813..937516a0c 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -76,7 +76,7 @@ private[sbt] object ClassLoaders { ) s.log.warn(s"$showJavaOptions will be ignored, $showFork is set to false") } - val exclude = dependencyJars(exportedProducts).value.toSet ++ instance.allJars + val exclude = dependencyJars(exportedProducts).value.toSet ++ instance.libraryJars val allDeps = dependencyJars(dependencyClasspath).value.filterNot(exclude) val newLoader = (classpath: Seq[File]) => { @@ -135,10 +135,22 @@ private[sbt] object ClassLoaders { val scalaLibraryLayer = layer(si.libraryJars, interfaceLoader, cache, resources, tmp) val cpFiles = fullCP.map(_._1) - val scalaReflectJar = allDependencies.find(_.getName == "scala-reflect.jar") + val scalaReflectJar = allDependencies.collectFirst { + case f if f.getName == "scala-reflect.jar" => + si.allJars.find(_.getName == "scala-reflect.jar") + }.flatten + class ScalaReflectClassLoader(jar: File) + extends URLClassLoader(Array(jar.toURI.toURL), scalaLibraryLayer) { + override def toString: String = + s"ScalaReflectClassLoader($jar, parent = $scalaLibraryLayer)" + } val scalaReflectLayer = scalaReflectJar .map { file => - layer(file :: Nil, scalaLibraryLayer, cache, resources, tmp) + cache.apply( + file -> IO.getModifiedTimeOrZero(file) :: Nil, + scalaLibraryLayer, + () => new ScalaReflectClassLoader(file) + ) } .getOrElse(scalaLibraryLayer) @@ -153,11 +165,12 @@ private[sbt] object ClassLoaders { if (layerDependencies) layer(allDependencies, resourceLayer, cache, resources, tmp) else resourceLayer + val scalaJarNames = (si.libraryJars ++ scalaReflectJar).map(_.getName).toSet // layer 4 val filteredSet = if (layerDependencies) allDependencies.toSet ++ si.libraryJars ++ scalaReflectJar else Set(si.libraryJars ++ scalaReflectJar: _*) - val dynamicClasspath = cpFiles.filterNot(filteredSet) + val dynamicClasspath = cpFiles.filterNot(f => filteredSet(f) || scalaJarNames(f.getName)) new LayeredClassLoader(dynamicClasspath, dependencyLayer, resources, tmp) } ClasspathUtilities.filterByClasspath(cpFiles, raw) From cc8c66c66d2b67d4491f07cabf862b06f8381b7d Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 09:42:33 -0700 Subject: [PATCH 2/7] Support java reflection with layered classloaders Jave reflection did not work with layered classloaders if a dependency attempted to load a class that was below the dependency layer in the layered classloader hierarchy. The underlying problem was (in general) a call to Class.forName somewhere. If the classloader parameter is not specified, then Class.forName locates the ClassLoader for the caller using reflection. It ultimately delegates to that ClassLoader's loadClass method. With the previous LayeredClassLoader class, there was no way for that classloader to access a URL that was below it in the class loading hierarchy. I reworked LayeredClassLoader so that if it fails to load the class, it will check the Thread's context classloader and see if there are other LayeredClassLoader instances below it. If so, it will then check if any of those classloaders would be able to load the class by using findResource. If the descendant loader can load the class, then we manually load it with findClass. --- main/src/main/scala/sbt/Defaults.scala | 12 ++++- .../scala/sbt/internal/ClassLoaders.scala | 1 + .../sbt/internal/LayeredClassLoader.scala | 54 +++++++++++++++++-- .../java-serialization/build.sbt | 4 ++ .../main/scala/reflection/Reflection.scala | 17 ++++++ .../descendant/src/test/scala/test/Foo.scala | 12 +++++ .../src/test/scala/test/ReflectionTest.scala | 12 +++++ .../classloader-cache/java-serialization/test | 12 +++++ 8 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 sbt/src/sbt-test/classloader-cache/java-serialization/build.sbt create mode 100644 sbt/src/sbt-test/classloader-cache/java-serialization/dependency/src/main/scala/reflection/Reflection.scala create mode 100644 sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/Foo.scala create mode 100644 sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/ReflectionTest.scala create mode 100644 sbt/src/sbt-test/classloader-cache/java-serialization/test diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 4b6e11db9..79ad043c7 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -855,9 +855,12 @@ object Defaults extends BuildCommon { } val trl = (testResultLogger in (Test, test)).value val taskName = Project.showContextKey(state.value).show(resolvedScoped.value) + val currentLoader = Thread.currentThread.getContextClassLoader try { + Thread.currentThread.setContextClassLoader(testLoader.value) trl.run(streams.value.log, executeTests.value, taskName) } finally { + Thread.currentThread.setContextClassLoader(currentLoader) close.foreach(_.apply()) } }, @@ -1022,8 +1025,13 @@ object Defaults extends BuildCommon { ) val taskName = display.show(resolvedScoped.value) val trl = testResultLogger.value - val processed = output.map(out => trl.run(s.log, out, taskName)) - processed + val currentLoader = Thread.currentThread.getContextClassLoader + try { + Thread.currentThread.setContextClassLoader(testLoader.value) + output.map(out => trl.run(s.log, out, taskName)) + } finally { + Thread.currentThread.setContextClassLoader(currentLoader) + } } } diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 937516a0c..54e3895ad 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -206,6 +206,7 @@ private[sbt] object ClassLoaders { parent: ClassLoader, resources: Map[String, String] ) extends LayeredClassLoader(classpath, parent, resources, new File("/dev/null")) { + 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) diff --git a/main/src/main/scala/sbt/internal/LayeredClassLoader.scala b/main/src/main/scala/sbt/internal/LayeredClassLoader.scala index 6ca12c6a7..095fb42f1 100644 --- a/main/src/main/scala/sbt/internal/LayeredClassLoader.scala +++ b/main/src/main/scala/sbt/internal/LayeredClassLoader.scala @@ -9,12 +9,14 @@ package sbt.internal import java.io.File import java.net.URLClassLoader -import java.{ util => jutil } -import scala.collection.JavaConverters._ +import java.util.concurrent.ConcurrentHashMap import sbt.internal.inc.classpath._ import sbt.io.IO +import scala.collection.JavaConverters._ +import scala.collection.mutable.ListBuffer + private[sbt] class LayeredClassLoader( classpath: Seq[File], parent: ClassLoader, @@ -24,7 +26,7 @@ private[sbt] class LayeredClassLoader( with RawResources with NativeCopyLoader with AutoCloseable { - private[this] val nativeLibs = new jutil.HashSet[File]().asScala + private[this] val nativeLibs = new java.util.HashSet[File]().asScala override protected val config = new NativeCopyConfig( tempDir, classpath, @@ -38,6 +40,50 @@ private[sbt] class LayeredClassLoader( l } } + + private[this] val loaded = new ConcurrentHashMap[String, Class[_]] + /* + * Override findClass to memoize its result. We need to do this because in loadClass we will + * delegate to findClass if the current LayeredClassLoader cannot load a class but it is a + * descendant of the thread's context class loader and a class loader below it in the layering + * hierarchy is able to load the required class. Unlike loadClass, findClass does not cache + * the result which would make it possible to return multiple versions of the same class. + */ + override def findClass(name: String): Class[_] = loaded.get(name) match { + case null => + val res = super.findClass(name) + loaded.putIfAbsent(name, res) match { + case null => res + case clazz => clazz + } + case c => c + } + override def loadClass(name: String, resolve: Boolean): Class[_] = { + try super.loadClass(name, resolve) + catch { + case e: ClassNotFoundException => + val loaders = new ListBuffer[LayeredClassLoader] + var currentLoader: ClassLoader = Thread.currentThread.getContextClassLoader + do { + currentLoader match { + case cl: LayeredClassLoader if cl != this => loaders.prepend(cl) + case _ => + } + currentLoader = currentLoader.getParent + } while (currentLoader != null && currentLoader != this) + if (currentLoader == this) { + val resourceName = name.replace('.', '/').concat(".class") + loaders + .collectFirst { + case l if l.findResource(resourceName) != null => + val res = l.findClass(name) + if (resolve) l.resolveClass(res) + res + } + .getOrElse(throw e) + } else throw e + } + } override def close(): Unit = nativeLibs.foreach(NativeLibs.delete) override def toString: String = s"""LayeredClassLoader( | classpath = @@ -48,7 +94,7 @@ private[sbt] class LayeredClassLoader( } private[internal] object NativeLibs { - private[this] val nativeLibs = new jutil.HashSet[File].asScala + private[this] val nativeLibs = new java.util.HashSet[File].asScala ShutdownHooks.add(() => { nativeLibs.foreach(IO.delete) IO.deleteIfEmpty(nativeLibs.map(_.getParentFile).toSet) diff --git a/sbt/src/sbt-test/classloader-cache/java-serialization/build.sbt b/sbt/src/sbt-test/classloader-cache/java-serialization/build.sbt new file mode 100644 index 000000000..ecc5e2814 --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/java-serialization/build.sbt @@ -0,0 +1,4 @@ +val dependency = project.settings(exportJars := true) +val descendant = project.dependsOn(dependency).settings( + libraryDependencies += "org.scalatest" %% "scalatest" % "3.0.5" % "test" +) diff --git a/sbt/src/sbt-test/classloader-cache/java-serialization/dependency/src/main/scala/reflection/Reflection.scala b/sbt/src/sbt-test/classloader-cache/java-serialization/dependency/src/main/scala/reflection/Reflection.scala new file mode 100644 index 000000000..09853ea44 --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/java-serialization/dependency/src/main/scala/reflection/Reflection.scala @@ -0,0 +1,17 @@ +package reflection + +import java.io._ +import scala.util.control.NonFatal + +object Reflection { + def roundTrip[A](a: A): A = { + val baos = new ByteArrayOutputStream() + val oos = new ObjectOutputStream(baos) + oos.writeObject(a) + oos.close() + val bais = new ByteArrayInputStream(baos.toByteArray()) + val ois = new ObjectInputStream(bais) + try ois.readObject().asInstanceOf[A] + finally ois.close() + } +} diff --git a/sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/Foo.scala b/sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/Foo.scala new file mode 100644 index 000000000..ea880687e --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/Foo.scala @@ -0,0 +1,12 @@ +package test + +class Foo extends Serializable { + private[this] var value: Int = 0 + def getValue(): Int = value + def setValue(newValue: Int): Unit = value = newValue + override def equals(o: Any): Boolean = o match { + case that: Foo => this.getValue() == that.getValue() + case _ => false + } + override def hashCode: Int = value +} \ No newline at end of file diff --git a/sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/ReflectionTest.scala b/sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/ReflectionTest.scala new file mode 100644 index 000000000..a1988034d --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/ReflectionTest.scala @@ -0,0 +1,12 @@ +package test + +import org.scalatest._ + +class ReflectionTest extends FlatSpec { + val foo = new Foo + foo.setValue(3) + val newFoo = reflection.Reflection.roundTrip(foo) + assert(newFoo == foo) + assert(System.identityHashCode(newFoo) != System.identityHashCode(foo)) +} + diff --git a/sbt/src/sbt-test/classloader-cache/java-serialization/test b/sbt/src/sbt-test/classloader-cache/java-serialization/test new file mode 100644 index 000000000..78ba31f24 --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/java-serialization/test @@ -0,0 +1,12 @@ +> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars + +> test + +> testOnly + +> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ScalaLibrary + +> test + +> testOnly + From 625470cdd5e219ff0e1436c6d157c03e41c935cb Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 11:58:48 -0700 Subject: [PATCH 3/7] Make LayeredClassLoaders parallel capable The docs for ClassLoader, https://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html say that all non-hierarchical custom classloaders should be registered as parallel capable. The docs also suggest that custom classloaders should try to only override findClass so I reworked LayerdClassLoader to only override findClass. I also added locking to the class loading to make it safe for concurrent loading. All of the custom classloaders besides LayeredClassLoader either subclass URLClassLoader or LayeredClassLoader but don't override loadClass. Because those two classloaders are parallel capable, the subclasses should be as well. It isn't possible to make classloaders that are implemented in scala parallel capable because scala 2 doesn't support jvm static blocks (dotty does support this with an annotation). To work around this, I re-worked some of the classloaders so that they are either directly implemented in java or I subclassed a scala implementation class in java. --- .../sbt/internal/classpath/WrappedLoader.java | 34 ++++ .../internal/classpath/ClassLoaderCache.scala | 10 +- .../main/java/sbt/internal/FlatLoader.java | 32 ++++ .../java/sbt/internal/LayeredClassLoader.java | 26 +++ .../java/sbt/internal/ResourceLoader.java | 23 +++ .../sbt/internal/ScalaReflectClassLoader.java | 29 ++++ .../scala/sbt/internal/ClassLoaders.scala | 28 +--- .../sbt/internal/LayeredClassLoader.scala | 112 ------------- .../sbt/internal/LayeredClassLoaders.scala | 151 ++++++++++++++++++ 9 files changed, 298 insertions(+), 147 deletions(-) create mode 100644 main-command/src/main/java/sbt/internal/classpath/WrappedLoader.java create mode 100644 main/src/main/java/sbt/internal/FlatLoader.java create mode 100644 main/src/main/java/sbt/internal/LayeredClassLoader.java create mode 100644 main/src/main/java/sbt/internal/ResourceLoader.java create mode 100644 main/src/main/java/sbt/internal/ScalaReflectClassLoader.java delete mode 100644 main/src/main/scala/sbt/internal/LayeredClassLoader.scala create mode 100644 main/src/main/scala/sbt/internal/LayeredClassLoaders.scala diff --git a/main-command/src/main/java/sbt/internal/classpath/WrappedLoader.java b/main-command/src/main/java/sbt/internal/classpath/WrappedLoader.java new file mode 100644 index 000000000..8bf7b7edc --- /dev/null +++ b/main-command/src/main/java/sbt/internal/classpath/WrappedLoader.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.classpath; + +import java.net.URL; +import java.net.URLClassLoader; + +public class WrappedLoader extends URLClassLoader { + static { + ClassLoader.registerAsParallelCapable(); + } + + WrappedLoader(final ClassLoader parent) { + super(new URL[] {}, parent); + } + + @Override + public URL[] getURLs() { + final ClassLoader parent = getParent(); + return (parent instanceof URLClassLoader) + ? ((URLClassLoader) parent).getURLs() + : super.getURLs(); + } + + @Override + public String toString() { + return "WrappedClassLoader(" + getParent() + ")"; + } +} diff --git a/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala b/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala index dee8e88c4..c0683d040 100644 --- a/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala +++ b/main-command/src/main/scala/sbt/internal/classpath/ClassLoaderCache.scala @@ -10,7 +10,7 @@ package sbt.internal.classpath import java.io.File import java.lang.management.ManagementFactory import java.lang.ref.{ Reference, ReferenceQueue, SoftReference } -import java.net.{ URL, URLClassLoader } +import java.net.URLClassLoader import java.util.concurrent.atomic.AtomicInteger import sbt.internal.inc.classpath.{ @@ -142,14 +142,6 @@ private[sbt] class ClassLoaderCache( private[this] val cleanupThread = new CleanupThread(ClassLoaderCache.threadID.getAndIncrement()) private[this] val lock = new Object - private class WrappedLoader(parent: ClassLoader) extends URLClassLoader(Array.empty, parent) { - // This is to make dotty work which extracts the URLs from the loader - override def getURLs: Array[URL] = parent match { - case u: URLClassLoader => u.getURLs - case _ => Array.empty - } - override def toString: String = s"WrappedLoader($parent)" - } private def close(classLoader: ClassLoader): Unit = classLoader match { case a: AutoCloseable => a.close() case _ => diff --git a/main/src/main/java/sbt/internal/FlatLoader.java b/main/src/main/java/sbt/internal/FlatLoader.java new file mode 100644 index 000000000..e1586d8be --- /dev/null +++ b/main/src/main/java/sbt/internal/FlatLoader.java @@ -0,0 +1,32 @@ +/* + * 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; + +class FlatLoader extends URLClassLoader { + static { + ClassLoader.registerAsParallelCapable(); + } + + FlatLoader(final URL[] urls, final ClassLoader parent) { + super(urls, parent); + } + + @Override + public String toString() { + final StringBuilder jars = new StringBuilder(); + for (final URL u : getURLs()) { + jars.append(" "); + jars.append(u); + jars.append("\n"); + } + return "FlatLoader(\n parent = " + getParent() + "\n jars = " + jars.toString() + ")"; + } +} diff --git a/main/src/main/java/sbt/internal/LayeredClassLoader.java b/main/src/main/java/sbt/internal/LayeredClassLoader.java new file mode 100644 index 000000000..241f94a05 --- /dev/null +++ b/main/src/main/java/sbt/internal/LayeredClassLoader.java @@ -0,0 +1,26 @@ +/* + * 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; + +class LayeredClassLoader extends LayeredClassLoaderImpl { + LayeredClassLoader( + final Seq classpath, + final ClassLoader parent, + final Map resources, + final File tempDir) { + super(classpath, parent, resources, tempDir); + } + + static { + ClassLoader.registerAsParallelCapable(); + } +} diff --git a/main/src/main/java/sbt/internal/ResourceLoader.java b/main/src/main/java/sbt/internal/ResourceLoader.java new file mode 100644 index 000000000..b408252d7 --- /dev/null +++ b/main/src/main/java/sbt/internal/ResourceLoader.java @@ -0,0 +1,23 @@ +/* + * 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; + +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/java/sbt/internal/ScalaReflectClassLoader.java b/main/src/main/java/sbt/internal/ScalaReflectClassLoader.java new file mode 100644 index 000000000..b8a3fff17 --- /dev/null +++ b/main/src/main/java/sbt/internal/ScalaReflectClassLoader.java @@ -0,0 +1,29 @@ +/* + * 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 ScalaReflectClassLoader extends URLClassLoader { + static { + ClassLoader.registerAsParallelCapable(); + } + + private final URL jar; + + ScalaReflectClassLoader(final URL jar, final ClassLoader parent) { + super(new URL[] {jar}, parent); + this.jar = jar; + } + + @Override + public String toString() { + return "ScalaReflectClassLoader(" + jar + " parent = " + getParent() + ")"; + } +} diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 54e3895ad..df02a7f07 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -9,7 +9,6 @@ package sbt package internal import java.io.File -import java.net.URLClassLoader import java.nio.file.Path import sbt.ClassLoaderLayeringStrategy._ @@ -139,17 +138,12 @@ private[sbt] object ClassLoaders { case f if f.getName == "scala-reflect.jar" => si.allJars.find(_.getName == "scala-reflect.jar") }.flatten - class ScalaReflectClassLoader(jar: File) - extends URLClassLoader(Array(jar.toURI.toURL), scalaLibraryLayer) { - override def toString: String = - s"ScalaReflectClassLoader($jar, parent = $scalaLibraryLayer)" - } val scalaReflectLayer = scalaReflectJar .map { file => cache.apply( file -> IO.getModifiedTimeOrZero(file) :: Nil, scalaLibraryLayer, - () => new ScalaReflectClassLoader(file) + () => new ScalaReflectClassLoader(file.toURI.toURL, scalaLibraryLayer) ) } .getOrElse(scalaLibraryLayer) @@ -201,19 +195,6 @@ private[sbt] object ClassLoaders { } else parent } - private class ResourceLoader( - classpath: Seq[File], - parent: ClassLoader, - resources: Map[String, String] - ) extends LayeredClassLoader(classpath, parent, resources, new File("/dev/null")) { - 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" - } // 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 @@ -235,14 +216,9 @@ private[sbt] object ClassLoaders { } else parent } - private[this] class FlatLoader(classpath: Seq[File], parent: ClassLoader) - extends URLClassLoader(classpath.map(_.toURI.toURL).toArray, parent) { - override def toString: String = - s"FlatClassLoader(parent = $interfaceLoader, jars =\n${classpath.mkString("\n")}\n)" - } // helper methods private def flatLoader(classpath: Seq[File], parent: ClassLoader): ClassLoader = - new FlatLoader(classpath, parent) + 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, _) => diff --git a/main/src/main/scala/sbt/internal/LayeredClassLoader.scala b/main/src/main/scala/sbt/internal/LayeredClassLoader.scala deleted file mode 100644 index 095fb42f1..000000000 --- a/main/src/main/scala/sbt/internal/LayeredClassLoader.scala +++ /dev/null @@ -1,112 +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 java.net.URLClassLoader -import java.util.concurrent.ConcurrentHashMap - -import sbt.internal.inc.classpath._ -import sbt.io.IO - -import scala.collection.JavaConverters._ -import scala.collection.mutable.ListBuffer - -private[sbt] class LayeredClassLoader( - classpath: Seq[File], - parent: ClassLoader, - override protected val resources: Map[String, String], - tempDir: File, -) extends URLClassLoader(classpath.toArray.map(_.toURI.toURL), parent) - with RawResources - with NativeCopyLoader - with AutoCloseable { - private[this] val nativeLibs = new java.util.HashSet[File]().asScala - override protected val config = new NativeCopyConfig( - tempDir, - classpath, - IO.parseClasspath(System.getProperty("java.library.path", "")) - ) - override def findLibrary(name: String): String = { - super.findLibrary(name) match { - case null => null - case l => - nativeLibs += new File(l) - l - } - } - - private[this] val loaded = new ConcurrentHashMap[String, Class[_]] - /* - * Override findClass to memoize its result. We need to do this because in loadClass we will - * delegate to findClass if the current LayeredClassLoader cannot load a class but it is a - * descendant of the thread's context class loader and a class loader below it in the layering - * hierarchy is able to load the required class. Unlike loadClass, findClass does not cache - * the result which would make it possible to return multiple versions of the same class. - */ - override def findClass(name: String): Class[_] = loaded.get(name) match { - case null => - val res = super.findClass(name) - loaded.putIfAbsent(name, res) match { - case null => res - case clazz => clazz - } - case c => c - } - override def loadClass(name: String, resolve: Boolean): Class[_] = { - try super.loadClass(name, resolve) - catch { - case e: ClassNotFoundException => - val loaders = new ListBuffer[LayeredClassLoader] - var currentLoader: ClassLoader = Thread.currentThread.getContextClassLoader - do { - currentLoader match { - case cl: LayeredClassLoader if cl != this => loaders.prepend(cl) - case _ => - } - currentLoader = currentLoader.getParent - } while (currentLoader != null && currentLoader != this) - if (currentLoader == this) { - val resourceName = name.replace('.', '/').concat(".class") - loaders - .collectFirst { - case l if l.findResource(resourceName) != null => - val res = l.findClass(name) - if (resolve) l.resolveClass(res) - res - } - .getOrElse(throw e) - } else throw e - } - } - override def close(): Unit = nativeLibs.foreach(NativeLibs.delete) - override def toString: String = s"""LayeredClassLoader( - | classpath = - | ${classpath mkString "\n "} - | parent = - | ${parent.toString.linesIterator.mkString("\n ")} - |)""".stripMargin -} - -private[internal] object NativeLibs { - private[this] val nativeLibs = new java.util.HashSet[File].asScala - ShutdownHooks.add(() => { - nativeLibs.foreach(IO.delete) - IO.deleteIfEmpty(nativeLibs.map(_.getParentFile).toSet) - nativeLibs.clear() - }) - def addNativeLib(lib: String): Unit = { - nativeLibs.add(new File(lib)) - () - } - def delete(file: File): Unit = { - nativeLibs.remove(file) - file.delete() - () - } -} diff --git a/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala b/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala new file mode 100644 index 000000000..d366dc875 --- /dev/null +++ b/main/src/main/scala/sbt/internal/LayeredClassLoaders.scala @@ -0,0 +1,151 @@ +/* + * 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 java.net.URLClassLoader +import java.util.concurrent.ConcurrentHashMap + +import sbt.internal.inc.classpath._ +import sbt.io.IO + +import scala.collection.JavaConverters._ +import scala.collection.mutable.ListBuffer + +private[sbt] class LayeredClassLoaderImpl( + classpath: Seq[File], + parent: ClassLoader, + override protected val resources: Map[String, String], + tempDir: File, +) extends URLClassLoader(classpath.toArray.map(_.toURI.toURL), parent) + with RawResources + with NativeCopyLoader + with AutoCloseable { + private[this] val nativeLibs = new java.util.HashSet[File]().asScala + override protected val config = new NativeCopyConfig( + tempDir, + classpath, + IO.parseClasspath(System.getProperty("java.library.path", "")) + ) + override def findLibrary(name: String): String = { + super.findLibrary(name) match { + case null => null + case l => + nativeLibs += new File(l) + l + } + } + + private[this] val loaded = new ConcurrentHashMap[String, Class[_]] + private[this] val classLocks = new ConcurrentHashMap[String, AnyRef]() + /* + * Override findClass to both memoize its result and look down the class hierarchy to attempt to + * load a missing class from a descendant loader. If we didn't cache the loaded classes, + * then it would be possible for this class loader to load a different version of the class than + * the descendant, which would likely cause a crash. The search for the class in the descendants + * allows java reflection to work in cases where the class to load via reflection is not directly + * visible to the class that is attempting to load it. + */ + override def findClass(name: String): Class[_] = loaded.get(name) match { + case null => + val newLock = new AnyRef + val lock = classLocks.putIfAbsent(name, newLock) match { + case null => newLock + case l => l + } + lock.synchronized { + try { + val clazz = super.findClass(name) + loaded.putIfAbsent(name, clazz) match { + case null => clazz + case c => c + } + } catch { + case e: ClassNotFoundException => + /* + * If new threads are spawned, they inherit the context class loader of the parent + * This means that if a test or run task spawns background threads to do work, then the + * same context class loader is available on all of the background threads. In the test + * and run tasks, we temporarily set the context class loader of the main sbt thread to + * be the classloader generated by ClassLoaders.getLayers. This creates an environment + * that looks somewhat like a forked jvm with the app classloader set to be the + * generated class loader. If the test or run main changes the thread context class + * loader, this search might fail even if it would have passed on the initial entry + * into the method. Applications generally only modify the context classloader if they + * are manually loading classes. It's likely that if an application generated + * ClassLoader needs access to the classes in the sbt classpath, then it would be using + * the original context class loader as the parent of the new context class loader + * anyway. + * + * If we wanted to make this change permanent so that the user could not + * override the global context classloader, we would possibly need to intercept the + * classloading of java.lang.Thread itself to return a custom Thread class that mirrors + * the java.lang.Thread api, but stores the context class loader in a custom field. + * + */ + var currentLoader: ClassLoader = Thread.currentThread.getContextClassLoader + val loaders = new ListBuffer[LayeredClassLoader] + do { + currentLoader match { + case cl: LayeredClassLoader if cl != this => loaders.prepend(cl) + case _ => + } + currentLoader = currentLoader.getParent + } while (currentLoader != null && currentLoader != this) + if (currentLoader == this && loaders.nonEmpty) { + val resourceName = name.replace('.', '/').concat(".class") + loaders + .collectFirst { + case l if l.findResource(resourceName) != null => l.findClass(name) + } + .getOrElse(throw e) + } else throw e + } + } + case c => c + } + override def close(): Unit = nativeLibs.foreach(NativeLibs.delete) + override def toString: String = s"""LayeredClassLoader( + | classpath = + | ${classpath mkString "\n "} + | parent = + | ${parent.toString.linesIterator.mkString("\n ")} + |)""".stripMargin +} + +private[internal] class ResourceLoaderImpl( + classpath: Seq[File], + parent: ClassLoader, + resources: Map[String, String] +) extends LayeredClassLoader(classpath, parent, resources, new File("/dev/null")) { + 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(() => { + nativeLibs.foreach(IO.delete) + IO.deleteIfEmpty(nativeLibs.map(_.getParentFile).toSet) + nativeLibs.clear() + }) + def addNativeLib(lib: String): Unit = { + nativeLibs.add(new File(lib)) + () + } + def delete(file: File): Unit = { + nativeLibs.remove(file) + file.delete() + () + } +} From 233307b696923f9013c0d94d4eb37582cfa6df67 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 13:38:40 -0700 Subject: [PATCH 4/7] Fix classpath filter in run We were incorrectly building the dependency layer in the run task using the raw jars from dependencyClasspath rather than the actual classpath jars (which may be different if bgCopyClasspath is true -- which it is by default). This was preventing spark from working with AllLibraryJars because it would load its classes and resources from the coursier cache but the classpath filter would reject the resources because they came from the coursier cache instead of the classpath. --- main/src/main/scala/sbt/internal/ClassLoaders.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index df02a7f07..16c46e5a9 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -79,12 +79,14 @@ private[sbt] object ClassLoaders { val allDeps = dependencyJars(dependencyClasspath).value.filterNot(exclude) val newLoader = (classpath: Seq[File]) => { + val mappings = classpath.map(f => f.getName -> f).toMap + val transformedDependencies = allDeps.map(f => mappings.get(f.getName).getOrElse(f)) buildLayers( strategy = classLoaderLayeringStrategy.value: @sbtUnchecked, si = instance, fullCP = classpath.map(f => f -> IO.getModifiedTimeOrZero(f)), resourceCP = resourceCP, - allDependencies = allDeps, + allDependencies = transformedDependencies, cache = extendedClassLoaderCache.value: @sbtUnchecked, resources = ClasspathUtilities.createClasspathResources(classpath, instance), tmp = taskTemporaryDirectory.value: @sbtUnchecked, From 014190a048283ef74096c47a169440e817d0e597 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 13:48:48 -0700 Subject: [PATCH 5/7] Add spark scripted test This test ensures that a simple spark app will work with all of the classloader layering strategies. Spark is an important part of the scala ecosystem and heavily taxes classloading. The test just checks that the app runs. I verified manually that the first time that run is evaluated takes about 8 seconds regardless of the layering strategy. It drops to 6 seconds with the ScalaLibrary strategy. It drops to 1 seconds with AllLibraryJars. --- sbt/src/sbt-test/classloader-cache/spark/build.sbt | 7 +++++++ sbt/src/sbt-test/classloader-cache/spark/log.txt | 7 +++++++ .../spark/src/main/scala/spark/SimpleApp.scala | 14 ++++++++++++++ sbt/src/sbt-test/classloader-cache/spark/test | 11 +++++++++++ 4 files changed, 39 insertions(+) create mode 100644 sbt/src/sbt-test/classloader-cache/spark/build.sbt create mode 100644 sbt/src/sbt-test/classloader-cache/spark/log.txt create mode 100644 sbt/src/sbt-test/classloader-cache/spark/src/main/scala/spark/SimpleApp.scala create mode 100644 sbt/src/sbt-test/classloader-cache/spark/test diff --git a/sbt/src/sbt-test/classloader-cache/spark/build.sbt b/sbt/src/sbt-test/classloader-cache/spark/build.sbt new file mode 100644 index 000000000..33fdcaa77 --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/spark/build.sbt @@ -0,0 +1,7 @@ +name := "Simple Project" + +version := "1.0" + +scalaVersion := "2.12.8" + +libraryDependencies += "org.apache.spark" %% "spark-sql" % "2.4.3" diff --git a/sbt/src/sbt-test/classloader-cache/spark/log.txt b/sbt/src/sbt-test/classloader-cache/spark/log.txt new file mode 100644 index 000000000..65b63b6d6 --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/spark/log.txt @@ -0,0 +1,7 @@ +a +b +c +d +e +f +g \ No newline at end of file diff --git a/sbt/src/sbt-test/classloader-cache/spark/src/main/scala/spark/SimpleApp.scala b/sbt/src/sbt-test/classloader-cache/spark/src/main/scala/spark/SimpleApp.scala new file mode 100644 index 000000000..4f7dcce5b --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/spark/src/main/scala/spark/SimpleApp.scala @@ -0,0 +1,14 @@ +import org.apache.spark.sql.SparkSession + +object SimpleApp { + def main(args: Array[String]) { + val logFile = "log.txt" + val spark = SparkSession.builder.appName("Simple Application").config("spark.master", "local").getOrCreate() + try { + val logData = spark.read.textFile(logFile).cache() + val numAs = logData.filter(line => line.contains("a")).count() + val numBs = logData.filter(line => line.contains("b")).count() + println(s"Lines with a: $numAs, Lines with b: $numBs") + } finally spark.stop() + } +} diff --git a/sbt/src/sbt-test/classloader-cache/spark/test b/sbt/src/sbt-test/classloader-cache/spark/test new file mode 100644 index 000000000..9b368a1b1 --- /dev/null +++ b/sbt/src/sbt-test/classloader-cache/spark/test @@ -0,0 +1,11 @@ +> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars + +> run + +> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ScalaLibrary + +> run + +> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat + +> run From ce6484f4b48c817e05a853de5aec8e325568fd34 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 15:03:22 -0700 Subject: [PATCH 6/7] Set java source and target levels Since sbt must run on jdk8, we should be setting the javac source and target levels. --- build.sbt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build.sbt b/build.sbt index 58c54d7ec..1b75bd567 100644 --- a/build.sbt +++ b/build.sbt @@ -28,6 +28,8 @@ def buildLevelSettings: Seq[Setting[_]] = bintrayPackage := "sbt", bintrayReleaseOnPublish := false, licenses := List("Apache-2.0" -> url("https://github.com/sbt/sbt/blob/0.13/LICENSE")), + javacOptions ++= Seq("-source", "1.8", "-target", "1.8"), + Compile / doc / javacOptions := Nil, developers := List( Developer("harrah", "Mark Harrah", "@harrah", url("https://github.com/harrah")), Developer("eed3si9n", "Eugene Yokota", "@eed3si9n", url("https://github.com/eed3si9n")), From 1b8f0ed20f1d322162b3a0f616ab9d7d45cf013a Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Mon, 3 Jun 2019 15:07:51 -0700 Subject: [PATCH 7/7] Don't use filtered classpath The classpath filter for test and run was added in #661 to ensure that the classloaders were correctly isolated. That is no longer necessary with the new layering strategies that are more precise about which jars are exposed at each level. Using the filtered classloader was causing the reflection used by spark to fail when using java 11. --- main/src/main/scala/sbt/internal/ClassLoaders.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 16c46e5a9..f4f57275c 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -126,7 +126,7 @@ private[sbt] object ClassLoaders { scope: Scope ): ClassLoader = { val cpFiles = fullCP.map(_._1) - val raw = strategy match { + strategy match { case Flat => flatLoader(cpFiles, interfaceLoader) case _ => val layerDependencies = strategy match { @@ -169,7 +169,6 @@ private[sbt] object ClassLoaders { val dynamicClasspath = cpFiles.filterNot(f => filteredSet(f) || scalaJarNames(f.getName)) new LayeredClassLoader(dynamicClasspath, dependencyLayer, resources, tmp) } - ClasspathUtilities.filterByClasspath(cpFiles, raw) } private def dependencyJars(