Fix shutdown hook failures in tests (#422)

* Remove duplication from ClassLoader.create * Prevent closing of context class loader in tests so that shutdown hooks can run
com-lihaoyi · Sep 2, 2018 · a6efead · a6efead
1 parent ed95f2d
commit a6efead
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 20 deletions.
diff --git a/main/core/src/mill/util/ClassLoader.scala b/main/core/src/mill/util/ClassLoader.scala
@@ -8,17 +8,7 @@ import io.github.retronym.java9rtexport.Export
 object ClassLoader {
 
   def create(urls: Seq[URL], parent: java.lang.ClassLoader)(
-      implicit ctx: Ctx.Home): URLClassLoader = {
-    new URLClassLoader(
-      makeUrls(urls).toArray,
-      refinePlatformParent(parent)
-    ) {
-      override def findClass(name: String): Class[_] = {
-        if (name.startsWith("com.sun.jna")) getClass.getClassLoader.loadClass(name)
-        else super.findClass(name)
-      }
-    }
-  }
+      implicit ctx: Ctx.Home): URLClassLoader = create(urls, parent, _ => None)
 
   def create(urls: Seq[URL],
              parent: java.lang.ClassLoader,

diff --git a/main/src/mill/modules/Jvm.scala b/main/src/mill/modules/Jvm.scala
@@ -89,7 +89,7 @@ object Jvm {
                classPath: Agg[Path],
                mainArgs: Seq[String] = Seq.empty)
               (implicit ctx: Ctx): Unit = {
-    inprocess(classPath, classLoaderOverrideSbtTesting = false, isolated = true, cl => {
+    inprocess(classPath, classLoaderOverrideSbtTesting = false, isolated = true, closeContextClassLoaderWhenDone = true, cl => {
       getMainMethod(mainClass, cl).invoke(null, mainArgs.toArray)
     })
   }
@@ -113,6 +113,7 @@ object Jvm {
   def inprocess[T](classPath: Agg[Path],
                    classLoaderOverrideSbtTesting: Boolean,
                    isolated: Boolean,
+                   closeContextClassLoaderWhenDone: Boolean,
                    body: ClassLoader => T)
                   (implicit ctx: Ctx.Home): T = {
     val urls = classPath.map(_.toIO.toURI.toURL)
@@ -123,19 +124,21 @@ object Jvm {
           Some(outerClassLoader.loadClass(name))
         else None
       })
-    } else if (isolated){
-
+    } else if (isolated) {
       mill.util.ClassLoader.create(urls.toVector, null)
-    }else{
+    } else {
       mill.util.ClassLoader.create(urls.toVector, getClass.getClassLoader)
     }
+
     val oldCl = Thread.currentThread().getContextClassLoader
     Thread.currentThread().setContextClassLoader(cl)
     try {
       body(cl)
-    }finally{
-      Thread.currentThread().setContextClassLoader(oldCl)
-      cl.close()
+    } finally {
+      if (closeContextClassLoaderWhenDone) {
+        Thread.currentThread().setContextClassLoader(oldCl)
+        cl.close()
+      }
     }
   }
 

diff --git a/scalalib/src/mill/scalalib/TestRunner.scala b/scalalib/src/mill/scalalib/TestRunner.scala
@@ -68,7 +68,8 @@ object TestRunner {
                testClassfilePath: Agg[Path],
                args: Seq[String])
               (implicit ctx: Ctx.Log with Ctx.Home): (String, Seq[mill.scalalib.TestRunner.Result]) = {
-    Jvm.inprocess(entireClasspath, classLoaderOverrideSbtTesting = true, isolated = true, cl => {
+    //Leave the context class loader set and open so that shutdown hooks can access it
+    Jvm.inprocess(entireClasspath, classLoaderOverrideSbtTesting = true, isolated = true, closeContextClassLoaderWhenDone = false, cl => {
       val frameworks = frameworkInstances(cl)
 
       val events = mutable.Buffer.empty[Event]

diff --git a/scalanativelib/src/mill/scalanativelib/ScalaNativeModule.scala b/scalanativelib/src/mill/scalanativelib/ScalaNativeModule.scala
@@ -265,7 +265,7 @@ trait TestScalaNativeModule extends ScalaNativeModule with TestModule { testOute
     val frameworkInstances = TestRunner.frameworks(testFrameworks()) _
 
     val testClasses =
-      Jvm.inprocess(testClasspathJvm().map(_.path), classLoaderOverrideSbtTesting = true, isolated = true,
+      Jvm.inprocess(testClasspathJvm().map(_.path), classLoaderOverrideSbtTesting = true, isolated = true, closeContextClassLoaderWhenDone = true,
         cl => {
           frameworkInstances(cl).flatMap { framework =>
             val df = Lib.discoverTests(cl, framework, Agg(compile().classes.path))