Make all test and run classloaders parallel capable

A number of users were reporting issues with deadlocking when using
1.3.2: https://github.com/sbt/sbt/issues/5116. This seems to be because
most of the sbt created classloaders were not actually parallel capable.
In order for a classloader to be registered as a parallel capable, ALL
of the parent classes except for object in the class hierarchy must be
registered as a parallel capable:
https://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#registerAsParallelCapable--.
If a classloader is not registered as parallel capable, then a global
lock will be used internally for classloading and this can lead to deadlock.

It is impossible to register a scala 2 classloader as parallel capable
so I ported all of the classloaders to java.

This commit updates the java-serialization scripted test. Prior to the
port, the new version of the test would more or less always deadlock.
After this change, I haven't been able to reproduce a deadlock.

This had no significant performance impact when I reran
https://github.com/eatkins/scala-build-watch-performance

build.sbt

@@ -680,6 +680,12 @@ lazy val mainProj = (project in file("main"))
       exclude[IncompatibleMethTypeProblem]("sbt.Defaults.allTestGroupsTask"),
       exclude[DirectMissingMethodProblem]("sbt.StandardMain.shutdownHook"),
       exclude[MissingClassProblem]("sbt.internal.ResourceLoaderImpl"),
+      // Removed private internal classes
+      exclude[MissingClassProblem]("sbt.internal.ReverseLookupClassLoaderHolder$BottomClassLoader"),
+      exclude[MissingClassProblem]("sbt.internal.ReverseLookupClassLoaderHolder$ReverseLookupClassLoader$ResourceLoader"),
+      exclude[MissingClassProblem]("sbt.internal.ReverseLookupClassLoaderHolder$ClassLoadingLock"),
+      exclude[MissingClassProblem]("sbt.internal.ReverseLookupClassLoaderHolder$ReverseLookupClassLoader"),
+      exclude[MissingClassProblem]("sbt.internal.LayeredClassLoaderImpl"),
     )
   )
   .configure(

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

@@ -0,0 +1,76 @@
+/*
+ * 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.io.IOException;
+import java.net.URL;
+import sbt.util.Logger;
+
+/**
+ * The bottom most layer in our layering hierarchy. This layer should never be cached. The
+ * dependency layer may need access to classes only available at this layer using java reflection.
+ * To make this work, we register this loader with the parent in its constructor. We also add the
+ * lookupClass method which gives ReverseLookupClassLoader a public interface to findClass.
+ *
+ * <p>To improve performance, when loading classes from the parent, we call the loadClass method
+ * with the reverseLookup flag set to false. This prevents the ReverseLookupClassLoader from trying
+ * to call back into this loader when it can't find a particular class.
+ */
+final class BottomClassLoader extends ManagedClassLoader {
+  private final ReverseLookupClassLoaderHolder holder;
+  private final ReverseLookupClassLoader parent;
+  private final ClassLoadingLock classLoadingLock = new ClassLoadingLock();
+
+  BottomClassLoader(
+      final ReverseLookupClassLoaderHolder holder,
+      final URL[] dynamicClasspath,
+      final ReverseLookupClassLoader reverseLookupClassLoader,
+      final File tempDir,
+      final boolean allowZombies,
+      final Logger logger) {
+    super(dynamicClasspath, reverseLookupClassLoader, allowZombies, logger);
+    setTempDir(tempDir);
+    this.holder = holder;
+    this.parent = reverseLookupClassLoader;
+    parent.setDescendant(this);
+  }
+
+  static {
+    ClassLoader.registerAsParallelCapable();
+  }
+
+  @Override
+  public Class<?> findClass(final String name) throws ClassNotFoundException {
+    return classLoadingLock.withLock(
+        name,
+        () -> {
+          final Class<?> prev = findLoadedClass(name);
+          if (prev != null) return prev;
+          return super.findClass(name);
+        });
+  }
+
+  @Override
+  protected Class<?> loadClass(final String name, final boolean resolve)
+      throws ClassNotFoundException {
+    try {
+      return parent.loadClass(name, resolve, false);
+    } catch (final ClassNotFoundException e) {
+      final Class<?> clazz = findClass(name);
+      if (resolve) resolveClass(clazz);
+      return clazz;
+    }
+  }
+
+  @Override
+  public void close() throws IOException {
+    holder.checkin(parent);
+    super.close();
+  }
+}

main/src/main/java/sbt/internal/ClassLoadingLock.java

@@ -0,0 +1,36 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.internal;
+
+import java.util.concurrent.ConcurrentHashMap;
+
+final class ClassLoadingLock {
+  interface ThrowsClassNotFound<R> {
+    R get() throws ClassNotFoundException;
+  }
+
+  private final ConcurrentHashMap<String, Object> locks = new ConcurrentHashMap<>();
+
+  <R> R withLock(final String name, final ThrowsClassNotFound<R> supplier) throws ClassNotFoundException {
+    final Object newLock = new Object();
+    Object prevLock;
+    synchronized (locks) {
+      prevLock = locks.putIfAbsent(name, newLock);
+    }
+    final Object lock = prevLock == null ? newLock : prevLock;
+    try {
+      synchronized (lock) {
+        return supplier.get();
+      }
+    } finally {
+      synchronized (locks) {
+        locks.remove(name);
+      }
+    }
+  }
+}

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

@@ -10,20 +10,20 @@ package sbt.internal;
 import java.io.File;
 import java.net.URL;
 import sbt.util.Logger;
-import scala.collection.Seq;
 
-final class FlatLoader extends LayeredClassLoaderImpl {
+final class FlatLoader extends ManagedClassLoader {
   static {
     ClassLoader.registerAsParallelCapable();
   }
 
   FlatLoader(
-      final Seq<File> files,
+      final URL[] urls,
       final ClassLoader parent,
       final File file,
       final boolean allowZombies,
       final Logger logger) {
-    super(files, parent, file, allowZombies, logger);
+    super(urls, parent, allowZombies, logger);
+    setTempDir(file);
   }
 
   @Override

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

@@ -8,13 +8,14 @@
 package sbt.internal;
 
 import java.io.File;
+import java.net.URL;
 import sbt.util.Logger;
-import scala.collection.Seq;
 
-final class LayeredClassLoader extends LayeredClassLoaderImpl {
-  LayeredClassLoader(final Seq<File> classpath, final ClassLoader parent, final File tempDir, final
+final class LayeredClassLoader extends ManagedClassLoader {
+  LayeredClassLoader(final URL[] classpath, final ClassLoader parent, final File tempDir, final
       boolean allowZombies, final Logger logger) {
-    super(classpath, parent, tempDir, allowZombies, logger);
+    super(classpath, parent, allowZombies, logger);
+    setTempDir(tempDir);
   }
 
   static {

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

@@ -0,0 +1,111 @@
+/*
+ * 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.io.IOException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+import sbt.util.Logger;
+
+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 allowZombies;
+  private final Logger logger;
+  private final NativeLookup nativeLookup = new NativeLookup();
+
+  static {
+    ClassLoader.registerAsParallelCapable();
+  }
+
+  ManagedClassLoader(
+      final URL[] urls, final ClassLoader parent, final boolean allowZombies, final Logger logger) {
+    super(urls, parent);
+    this.allowZombies = allowZombies;
+    this.logger = logger;
+  }
+
+  private class ZombieClassLoader extends URLClassLoader {
+    private final URL[] urls;
+
+    ZombieClassLoader(URL[] urls) {
+      super(urls, ManagedClassLoader.this);
+      this.urls = urls;
+    }
+
+    Class<?> lookupClass(final String name) throws ClassNotFoundException {
+      try {
+        return findClass(name);
+      } catch (final ClassNotFoundException e) {
+        final StringBuilder builder = new StringBuilder();
+        for (final URL u : urls) {
+          final File f = new File(u.getPath());
+          if (f.exists()) builder.append(f.toString()).append('\n');
+        }
+        final String deleted = builder.toString();
+        if (!deleted.isEmpty()) {
+          final String msg =
+              "Couldn't load class $name. "
+                  + "The following urls on the classpath do not exist:\n"
+                  + deleted
+                  + "This may be due to shutdown hooks added during an invocation of `run`.";
+          System.err.println(msg);
+        }
+        throw e;
+      }
+    }
+  }
+
+  private ZombieClassLoader getZombieLoader(final String name) {
+    if (printedWarning.compareAndSet(false, true) && !allowZombies) {
+      final String msg =
+          (Thread.currentThread() + " loading " + name + " after test or run ")
+              + "has completed. This is a likely resource leak";
+      logger.warn((Supplier<String>) () -> msg);
+    }
+    final ZombieClassLoader maybeLoader = zombieLoader.get();
+    if (maybeLoader != null) return maybeLoader;
+    else {
+      final ZombieClassLoader zb = new ZombieClassLoader(getURLs());
+      zombieLoader.set(zb);
+      return zb;
+    }
+  }
+
+  @Override
+  public URL findResource(String name) {
+    return closed.get() ? getZombieLoader(name).findResource(name) : super.findResource(name);
+  }
+
+  @Override
+  protected Class<?> findClass(String name) throws ClassNotFoundException {
+    return closed.get() ? getZombieLoader(name).lookupClass(name) : super.findClass(name);
+  }
+
+  @Override
+  public void close() throws IOException {
+    final ZombieClassLoader zb = zombieLoader.getAndSet(null);
+    if (zb != null) zb.close();
+    if (closed.compareAndSet(false, true)) super.close();
+  }
+
+  @Override
+  public String findLibrary(final String name) {
+    return nativeLookup.findLibrary(name);
+  }
+
+  @Override
+  public void setTempDir(final File file) {
+    nativeLookup.setTempDir(file);
+  }
+}

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

@@ -0,0 +1,127 @@
+/*
+ * 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.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;
+
+/**
+ * A ClassLoader for the dependency layer of a run or test task. It is almost a normal
+ * URLClassLoader except that it has the ability to look one level down the classloading hierarchy
+ * to load a class via reflection that is not directly available to it. The ClassLoader that is
+ * below it in the hierarchy must be registered via setDescendant. If it ever loads a class from its
+ * descendant, then it cannot be used in a subsequent run because it will not be possible to reload
+ * that class.
+ *
+ * <p>The descendant classloader checks it in and out via [[checkout]] and [[checkin]]. When it
+ * returns the loader via [[checkin]], if the loader is dirty, we close it. Otherwise we cache it if
+ * there is no existing cache entry.
+ *
+ * <p>Performance degrades if loadClass is constantly looking back up to the provided
+ * BottomClassLoader so we provide an alternate loadClass definition that takes a reverseLookup
+ * boolean parameter. Because the [[BottomClassLoader]] knows what loader is calling into, when it
+ * delegates its search to the ReverseLookupClassLoader, it passes false for the reverseLookup flag.
+ * By default the flag is true. Most of the time, the default loadClass will only be invoked by java
+ * reflection calls. Even then, there's some chance that the class being loaded by java reflection
+ * is _not_ available on the bottom classpath so it is not guaranteed that performing the reverse
+ * lookup will invalidate this loader.
+ */
+final class ReverseLookupClassLoader extends ManagedClassLoader {
+  ReverseLookupClassLoader(
+      final URL[] urls, final ClassLoader parent, final boolean allowZombies, final Logger logger) {
+    super(urls, parent, allowZombies, logger);
+    this.parent = parent;
+  }
+
+  private final AtomicReference<BottomClassLoader> directDescendant = new AtomicReference<>();
+  private final AtomicBoolean dirty = new AtomicBoolean(false);
+  private final ClassLoadingLock classLoadingLock = new ClassLoadingLock();
+  private final AtomicReference<ResourceLoader> resourceLoader = new AtomicReference<>();
+  private final ClassLoader parent;
+
+  boolean isDirty() {
+    return dirty.get();
+  }
+
+  void setDescendant(final BottomClassLoader bottomClassLoader) {
+    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);
+  }
+
+  void setup(final File tmpDir, final URL[] urls) throws IOException {
+    setTempDir(tmpDir);
+    final ResourceLoader previous = resourceLoader.getAndSet(new ResourceLoader(urls));
+    if (previous != null) previous.close();
+  }
+
+  @Override
+  protected Class<?> loadClass(final String name, final boolean resolve)
+      throws ClassNotFoundException {
+    return loadClass(name, resolve, true);
+  }
+
+  Class<?> loadClass(final String name, final boolean resolve, final boolean childLookup)
+      throws ClassNotFoundException {
+    Class<?> result;
+    try {
+      result = parent.loadClass(name);
+    } catch (final ClassNotFoundException e) {
+      result = findClass(name, childLookup);
+    }
+    if (result == null) throw new ClassNotFoundException(name);
+    if (resolve) resolveClass(result);
+    return result;
+  }
+
+  private Class<?> findClass(final String name, final boolean childLookup)
+      throws ClassNotFoundException {
+    return classLoadingLock.withLock(
+        name,
+        () -> {
+          try {
+            final Class<?> prev = findLoadedClass(name);
+            if (prev != null) return prev;
+            return findClass(name);
+          } catch (final ClassNotFoundException e) {
+            if (childLookup) {
+              final BottomClassLoader loader = directDescendant.get();
+              if (loader == null) throw e;
+              final Class<?> clazz = loader.findClass(name);
+              dirty.set(true);
+              return clazz;
+            } else {
+              throw e;
+            }
+          }
+        });
+  }
+
+  static {
+    registerAsParallelCapable();
+  }
+}

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

@@ -9,6 +9,7 @@ package sbt
 package internal
 
 import java.io.File
+import java.net.URL
 import java.nio.file.Path
 
 import sbt.ClassLoaderLayeringStrategy._
@@ -25,6 +26,9 @@ import sbt.nio.Keys._
 import sbt.util.Logger
 
 private[sbt] object ClassLoaders {
+  private implicit class SeqFileOps(val files: Seq[File]) extends AnyVal {
+    def urls: Array[URL] = files.toArray.map(_.toURI.toURL)
+  }
   private[this] val interfaceLoader = classOf[sbt.testing.Framework].getClassLoader
   /*
    * Get the class loader for a test task. The configuration could be IntegrationTest or Test.
@@ -136,7 +140,7 @@ private[sbt] object ClassLoaders {
   ): ClassLoader = {
     val cpFiles = fullCP.map(_._1)
     strategy match {
-      case Flat => new FlatLoader(cpFiles, interfaceLoader, tmp, allowZombies, logger)
+      case Flat => new FlatLoader(cpFiles.urls, interfaceLoader, tmp, allowZombies, logger)
       case _ =>
         val layerDependencies = strategy match {
           case _: AllLibraryJars => true
@@ -194,7 +198,8 @@ private[sbt] object ClassLoaders {
           case cl =>
             cl.getParent match {
               case dl: ReverseLookupClassLoaderHolder => dl.checkout(cpFiles, tmp)
-              case _                                  => new LayeredClassLoader(dynamicClasspath, cl, tmp, allowZombies, logger)
+              case _ =>
+                new LayeredClassLoader(dynamicClasspath.urls, cl, tmp, allowZombies, logger)
             }
         }
     }

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

@@ -8,7 +8,7 @@
 package sbt.internal
 
 import java.io.File
-import java.net.{ URL, URLClassLoader }
+import java.net.URLClassLoader
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.{ AtomicBoolean, AtomicReference }
 
@@ -17,23 +17,6 @@ import sbt.util.Logger
 
 import scala.collection.JavaConverters._
 
-/**
- * A simple ClassLoader that copies native libraries to a temporary directory before loading them.
- * Otherwise the same as a normal URLClassLoader.
- * @param classpath the classpath of the url
- * @param parent the parent loader
- * @param tempDir the directory into which native libraries are copied before loading
- */
-private[internal] class LayeredClassLoaderImpl(
-    classpath: Seq[File],
-    parent: ClassLoader,
-    tempDir: File,
-    allowZombies: Boolean,
-    logger: Logger
-) extends ManagedClassLoader(classpath.toArray.map(_.toURI.toURL), parent, allowZombies, logger) {
-  setTempDir(tempDir)
-}
-
 /**
  * This classloader doesn't load any classes. It is able to create a two layer bundled ClassLoader
  * that is able to load the full project classpath. The top layer is able to directly load the
@@ -79,14 +62,21 @@ private[internal] final class ReverseLookupClassLoaderHolder(
       throw new IllegalStateException(msg)
     }
     val reverseLookupClassLoader = cached.getAndSet(null) match {
-      case null => new ReverseLookupClassLoader
+      case null => new ReverseLookupClassLoader(urls, parent, allowZombies, logger)
       case c    => c
     }
-    reverseLookupClassLoader.setup(tempDir, fullClasspath)
-    new BottomClassLoader(fullClasspath, reverseLookupClassLoader, tempDir)
+    reverseLookupClassLoader.setup(tempDir, fullClasspath.map(_.toURI.toURL).toArray)
+    new BottomClassLoader(
+      ReverseLookupClassLoaderHolder.this,
+      fullClasspath.map(_.toURI.toURL).toArray,
+      reverseLookupClassLoader,
+      tempDir,
+      allowZombies,
+      logger
+    )
   }
 
-  private def checkin(reverseLookupClassLoader: ReverseLookupClassLoader): Unit = {
+  private[sbt] def checkin(reverseLookupClassLoader: ReverseLookupClassLoader): Unit = {
     if (reverseLookupClassLoader.isDirty) reverseLookupClassLoader.close()
     else {
       if (closed.get()) reverseLookupClassLoader.close()
@@ -97,6 +87,7 @@ private[internal] final class ReverseLookupClassLoaderHolder(
         }
     }
   }
+
   override def close(): Unit = {
     closed.set(true)
     cached.get() match {
@@ -104,149 +95,6 @@ private[internal] final class ReverseLookupClassLoaderHolder(
       case c    => c.close()
     }
   }
-
-  private class ClassLoadingLock {
-    private[this] val locks = new ConcurrentHashMap[String, AnyRef]()
-    def withLock[R](name: String)(f: => R): R = {
-      val newLock = new AnyRef
-      val lock = locks.synchronized(locks.put(name, newLock) match {
-        case null => newLock
-        case l    => l
-      })
-      try lock.synchronized(f)
-      finally locks.synchronized {
-        locks.remove(name)
-        ()
-      }
-    }
-  }
-
-  /**
-   * A ClassLoader for the dependency layer of a run or test task. It is almost a normal
-   * URLClassLoader except that it has the ability to look one level down the classloading
-   * hierarchy to load a class via reflection that is not directly available to it. The ClassLoader
-   * that is below it in the hierarchy must be registered via setDescendant. If it ever loads a
-   * class from its descendant, then it cannot be used in a subsequent run because it will not be
-   * possible to reload that class.
-   *
-   * The descendant classloader checks it in and out via [[checkout]] and [[checkin]]. When it
-   * returns the loader via [[checkin]], if the loader is dirty, we close it. Otherwise we
-   * cache it if there is no existing cache entry.
-   *
-   * Performance degrades if loadClass is constantly looking back up to the provided
-   * BottomClassLoader so we provide an alternate loadClass definition that takes a reverseLookup
-   * boolean parameter. Because the [[BottomClassLoader]] knows what loader is calling into, when
-   * it delegates its search to the ReverseLookupClassLoader, it passes false for the reverseLookup
-   * flag. By default the flag is true. Most of the time, the default loadClass will only be
-   * invoked by java reflection calls. Even then, there's some chance that the class being loaded
-   * by java reflection is _not_ available on the bottom classpath so it is not guaranteed that
-   * performing the reverse lookup will invalidate this loader.
-   *
-   */
-  private class ReverseLookupClassLoader
-      extends ManagedClassLoader(urls, parent, allowZombies, logger)
-      with NativeLoader {
-    override def getURLs: Array[URL] = urls
-    private[this] val directDescendant: AtomicReference[BottomClassLoader] =
-      new AtomicReference
-    private[this] val dirty = new AtomicBoolean(false)
-    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)
-        catch {
-          case e: ClassNotFoundException if reverseLookup =>
-            directDescendant.get match {
-              case null => throw e
-              case cl =>
-                val res = cl.lookupClass(name)
-                dirty.set(true)
-                res
-            }
-        }
-      }
-    }
-    override def loadClass(name: String, resolve: Boolean): Class[_] =
-      loadClass(name, resolve, reverseLookup = true)
-  }
-
-  /**
-   * The bottom most layer in our layering hierarchy. This layer should never be cached. The
-   * dependency layer may need access to classes only available at this layer using java
-   * reflection. To make this work, we register this loader with the parent in its
-   * constructor. We also add the lookupClass method which gives ReverseLookupClassLoader
-   * a public interface to findClass.
-   *
-   * To improve performance, when loading classes from the parent, we call the loadClass
-   * method with the reverseLookup flag set to false. This prevents the ReverseLookupClassLoader
-   * from trying to call back into this loader when it can't find a particular class.
-   *
-   * @param dynamicClasspath the classpath for the run or test task excluding the dependency jars
-   * @param parent the ReverseLookupClassLoader with which this loader needs to register itself
-   *               so that reverse lookups required by java reflection will work
-   * @param tempDir the temp directory to copy native libraries
-   */
-  private class BottomClassLoader(
-      dynamicClasspath: Seq[File],
-      parent: ReverseLookupClassLoader,
-      tempDir: File
-  ) extends ManagedClassLoader(
-        dynamicClasspath.map(_.toURI.toURL).toArray,
-        parent,
-        allowZombies,
-        logger
-      )
-      with NativeLoader {
-    parent.setDescendant(this)
-    setTempDir(tempDir)
-    val classLoadingLock = new ClassLoadingLock
-
-    final def lookupClass(name: String): Class[_] = findClass(name)
-
-    override def findClass(name: String): Class[_] = {
-      findLoadedClass(name) match {
-        case null =>
-          classLoadingLock.withLock(name) {
-            findLoadedClass(name) match {
-              case null => super.findClass(name)
-              case c    => c
-            }
-          }
-        case c => c
-      }
-    }
-    override def loadClass(name: String, resolve: Boolean): Class[_] = {
-      val clazz = findLoadedClass(name) match {
-        case null =>
-          val c = try parent.loadClass(name, resolve = false, reverseLookup = false)
-          catch { case _: ClassNotFoundException => findClass(name) }
-          if (resolve) resolveClass(c)
-          c
-        case c => c
-      }
-      if (resolve) resolveClass(clazz)
-      clazz
-    }
-    override def close(): Unit = {
-      checkin(parent)
-      super.close()
-    }
-  }
 }
 
 /**
@@ -255,17 +103,18 @@ private[internal] final class ReverseLookupClassLoaderHolder(
  * to be dynamically reset. The explicit mappings feature isn't used by sbt. The dynamic
  * temp directory use case is needed in some layered class loading scenarios.
  */
-private trait NativeLoader extends ClassLoader with AutoCloseable {
+private[internal] trait NativeLoader extends AutoCloseable {
+  private[internal] def setTempDir(file: File): Unit = {}
+}
+private[internal] class NativeLookup extends NativeLoader {
   private[this] val mapped = new ConcurrentHashMap[String, String]
   private[this] val searchPaths =
     sys.props.get("java.library.path").map(IO.parseClasspath).getOrElse(Nil)
   private[this] val tempDir = new AtomicReference(new File("/dev/null"))
 
-  abstract override def close(): Unit = {
-    setTempDir(new File("/dev/null"))
-    super.close()
-  }
-  override protected def findLibrary(name: String): String = synchronized {
+  override def close(): Unit = setTempDir(new File("/dev/null"))
+
+  def findLibrary(name: String): String = synchronized {
     mapped.get(name) match {
       case null =>
         findLibrary0(name) match {
@@ -278,23 +127,28 @@ private trait NativeLoader extends ClassLoader with AutoCloseable {
       case n => n
     }
   }
-  private[internal] def setTempDir(file: File): Unit = {
+
+  private[internal] override def setTempDir(file: File): Unit = {
     deleteNativeLibs()
     tempDir.set(file)
   }
+
   private[this] def deleteNativeLibs(): Unit = {
     mapped.values().forEach(NativeLibs.delete)
     mapped.clear()
   }
+
   private[this] def findLibrary0(name: String): String = {
     val mappedName = System.mapLibraryName(name)
     val search = searchPaths.toStream flatMap relativeLibrary(mappedName)
     search.headOption.map(copy).orNull
   }
+
   private[this] def relativeLibrary(mappedName: String)(base: File): Seq[File] = {
     val f = new File(base, mappedName)
     if (f.isFile) f :: Nil else Nil
   }
+
   private[this] def copy(f: File): String = {
     val target = new File(tempDir.get(), f.getName)
     IO.copyFile(f, target)
@@ -320,64 +174,3 @@ private[internal] object NativeLibs {
     ()
   }
 }
-
-private sealed abstract class ManagedClassLoader(
-    urls: Array[URL],
-    parent: ClassLoader,
-    allowZombies: Boolean,
-    logger: Logger
-) extends URLClassLoader(urls, parent)
-    with NativeLoader {
-  private[this] val closed = new AtomicBoolean(false)
-  private[this] val printedWarning = new AtomicBoolean(false)
-  private[this] val zombieLoader = new AtomicReference[ZombieClassLoader]
-  private class ZombieClassLoader extends URLClassLoader(urls, this) {
-    def lookupClass(name: String): Class[_] =
-      try findClass(name)
-      catch {
-        case e: ClassNotFoundException =>
-          val deleted = urls.flatMap { u =>
-            val f = new File(u.getPath)
-            if (f.exists) None else Some(f)
-          }
-          if (deleted.toSeq.nonEmpty) {
-            // TODO - add doc link
-            val msg = s"Couldn't load class $name. " +
-              s"The following urls on the classpath do not exist:\n${deleted mkString "\n"}\n" +
-              "This may be due to shutdown hooks added during an invocation of `run`."
-            // logging may be shutdown at this point so we need to print directly to System.err.
-            System.err.println(msg)
-          }
-          throw e
-      }
-  }
-  private def getZombieLoader(name: String): ZombieClassLoader = {
-    if (printedWarning.compareAndSet(false, true) && !allowZombies) {
-      // TODO - Need to add link to documentation in website
-      val thread = Thread.currentThread
-      val msg =
-        s"$thread loading $name after test or run has completed. This is a likely resource leak."
-      logger.warn(msg)
-    }
-    zombieLoader.get match {
-      case null =>
-        val zb = new ZombieClassLoader
-        zombieLoader.set(zb)
-        zb
-      case zb => zb
-    }
-  }
-  override def findResource(name: String): URL = {
-    if (closed.get) getZombieLoader(name).findResource(name)
-    else super.findResource(name)
-  }
-  override def findClass(name: String): Class[_] = {
-    if (closed.get) getZombieLoader(name).lookupClass(name)
-    else super.findClass(name)
-  }
-  override def close(): Unit = {
-    closed.set(true)
-    Option(zombieLoader.getAndSet(null)).foreach(_.close())
-    super.close()
-  }
-}

sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/Bar.scala

@@ -0,0 +1,3 @@
+package test
+
+trait Bar

sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/Foo.scala

@@ -1,6 +1,6 @@
 package test
 
-class Foo extends Serializable {
+class Foo extends Bar with Serializable {
   private[this] var value: Int = 0
   def getValue(): Int = value
   def setValue(newValue: Int): Unit = value = newValue

sbt/src/sbt-test/classloader-cache/java-serialization/descendant/src/test/scala/test/ReflectionTest.scala

@@ -1,12 +1,32 @@
 package test
 
+import java.util.concurrent.{ CountDownLatch, TimeUnit }
+
 import org.scalatest._
 
 class ReflectionTest extends FlatSpec {
+  val procs = 2
+  val initLatch = new CountDownLatch(procs)
+  val loader = this.getClass.getClassLoader
+  val latch = new CountDownLatch(procs)
+  (1 to procs).foreach { i =>
+    new Thread() {
+      setDaemon(true)
+      start()
+      override def run(): Unit = {
+        initLatch.countDown()
+        initLatch.await(5, TimeUnit.SECONDS)
+        val className = if (i % 2 == 0) "test.Foo" else "test.Bar"
+        loader.loadClass(className)
         val foo = new Foo
         foo.setValue(3)
         val newFoo = reflection.Reflection.roundTrip(foo)
         assert(newFoo == foo)
         assert(System.identityHashCode(newFoo) != System.identityHashCode(foo))
+        latch.countDown()
+      }
+    }
+  }
+  assert(latch.await(5, TimeUnit.SECONDS))
 }
 

sbt/src/sbt-test/classloader-cache/java-serialization/test

@@ -1,12 +1,20 @@
-> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars
+> set descendant / Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars
+
+# We run test a number of times to ensure that it doesn't deadlock
+> test
+
+> test
+
+> test
+
+> test
 
 > test
 
 > testOnly
 
-> set classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ScalaLibrary
+> set descendant / Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.ScalaLibrary
 
 > test
 
 > testOnly
-