Properly close Scala compiler classloaders in server and unit tests (#…

…3468) Previously we were just leaking the `URLClassLoaders`, which would give `CodeHeap 'non-profiled nmethods' is full. Compiler has been disabled.` in `scalajslib.test` and sometimes in actual usage for a long lived server. This PR ensures we `ZincWorkerImpl.close` actually calls `URLClassLoader.close`, and also replaced `val eval = UnitTester` in our test suite with `UnitTester.scoped{ eval =>` so we can ensure closing of the `UnitTester` and `URLClassLoader` after each test completes With this change, `scalajslib.test` no longer prints the above error. Hopefully it makes it stop happening in production as well. `scalajslib.test` also drops from 7.5min to 6.5min (and drops further to 5min when I removed Scala.js 1.3.1 from the test matrix)
com-lihaoyi · Sep 6, 2024 · 588e1b7 · 588e1b7
1 parent 2410fc0
commit 588e1b7
Show file tree

Hide file tree

Showing 38 changed files with 1,087 additions and 1,150 deletions.
diff --git a/contrib/buildinfo/test/src/mill/contrib/buildinfo/BuildInfoTests.scala b/contrib/buildinfo/test/src/mill/contrib/buildinfo/BuildInfoTests.scala
@@ -95,48 +95,52 @@ object BuildInfoTests extends TestSuite {
     eval.outPath / "buildInfoResources.dest" / "foo" / "BuildInfo.buildinfo.properties"
   def tests: Tests = Tests {
 
-    test("notCreateEmptySourcefile") {
-      val eval = UnitTester(EmptyBuildInfo, testModuleSourcesPath / "scala")
+    test("notCreateEmptySourcefile") - UnitTester(
+      EmptyBuildInfo,
+      testModuleSourcesPath / "scala"
+    ).scoped { eval =>
       val Right(_) = eval.apply(EmptyBuildInfo.buildInfoSources)
       assert(!os.exists(buildInfoSourcePath(eval)))
     }
 
-    test("fileGeneration") {
-      val eval = UnitTester(BuildInfoComment, testModuleSourcesPath / "scala")
-      val Right(_) = eval.apply(BuildInfoComment.compile)
-
-      // Make sure that the buildinfo Scala file buildinfo is created and buildinfo
-      // resource file is *not* created when we compile the Scala code
-      assert(os.exists(buildInfoSourcePath(eval)))
-      assert(!os.exists(buildInfoResourcePath(eval)))
-
-      val expectedSource = Seq(
-        """  /** a helpful comment explaining what scalaVersion is all about */
-          |  val scalaVersion = buildInfoProperties.getProperty("scalaVersion")""".stripMargin,
-        """  /**
-          |    * a helpful comment explaining what scalaVersion
-          |    * is all about
-          |    */
-          |  val scalaVersion2 = buildInfoProperties.getProperty("scalaVersion2")""".stripMargin
-      )
-
-      val buildInfoCode = os.read(buildInfoSourcePath(eval)).linesIterator.mkString("\n")
-      for (e <- expectedSource) {
-        assert(buildInfoCode.contains(e.linesIterator.mkString("\n")))
-      }
-
-      // But becomes created once we package the jar for running
-      val Right(_) = eval.apply(BuildInfoComment.jar)
-
-      val expectedResource = "mill.contrib.buildinfo.BuildInfo for foo."
-
-      assert(os.exists(buildInfoResourcePath(eval)))
-      val buildInfoResource = os.read(buildInfoResourcePath(eval))
-      assert(buildInfoResource.contains(expectedResource))
+    test("fileGeneration") - UnitTester(BuildInfoComment, testModuleSourcesPath / "scala").scoped {
+      eval =>
+        val Right(_) = eval.apply(BuildInfoComment.compile)
+
+        // Make sure that the buildinfo Scala file buildinfo is created and buildinfo
+        // resource file is *not* created when we compile the Scala code
+        assert(os.exists(buildInfoSourcePath(eval)))
+        assert(!os.exists(buildInfoResourcePath(eval)))
+
+        val expectedSource = Seq(
+          """  /** a helpful comment explaining what scalaVersion is all about */
+            |  val scalaVersion = buildInfoProperties.getProperty("scalaVersion")""".stripMargin,
+          """  /**
+            |    * a helpful comment explaining what scalaVersion
+            |    * is all about
+            |    */
+            |  val scalaVersion2 = buildInfoProperties.getProperty("scalaVersion2")""".stripMargin
+        )
+
+        val buildInfoCode = os.read(buildInfoSourcePath(eval)).linesIterator.mkString("\n")
+        for (e <- expectedSource) {
+          assert(buildInfoCode.contains(e.linesIterator.mkString("\n")))
+        }
+
+        // But becomes created once we package the jar for running
+        val Right(_) = eval.apply(BuildInfoComment.jar)
+
+        val expectedResource = "mill.contrib.buildinfo.BuildInfo for foo."
+
+        assert(os.exists(buildInfoResourcePath(eval)))
+        val buildInfoResource = os.read(buildInfoResourcePath(eval))
+        assert(buildInfoResource.contains(expectedResource))
     }
 
-    test("supportCustomSettings") {
-      val eval = UnitTester(BuildInfoSettings, testModuleSourcesPath / "scala")
+    test("supportCustomSettings") - UnitTester(
+      BuildInfoSettings,
+      testModuleSourcesPath / "scala"
+    ).scoped { eval =>
       val Right(result) = eval.apply(BuildInfoSettings.buildInfoSources)
       val path = result.value.head.path
 
@@ -146,14 +150,12 @@ object BuildInfoTests extends TestSuite {
       assert(found.contains("object bar"))
     }
 
-    test("compile") {
-      val eval = UnitTester(BuildInfoPlain, testModuleSourcesPath / "scala")
+    test("compile") - UnitTester(BuildInfoPlain, testModuleSourcesPath / "scala").scoped { eval =>
       val Right(_) = eval.apply(BuildInfoPlain.compile)
       assert(true)
     }
 
-    test("run") {
-      val eval = UnitTester(BuildInfoPlain, testModuleSourcesPath / "scala")
+    test("run") - UnitTester(BuildInfoPlain, testModuleSourcesPath / "scala").scoped { eval =>
       val runResult = eval.outPath / "hello-mill"
       val Right(_) =
         eval.apply(BuildInfoPlain.run(T.task(Args(runResult.toString))))
@@ -164,14 +166,13 @@ object BuildInfoTests extends TestSuite {
       )
     }
 
-    test("scalajs") {
-      val eval = UnitTester(BuildInfoScalaJS, testModuleSourcesPath / "scala-simple")
-      val runResult = eval.outPath / "hello-mill"
-      assert(eval.apply(BuildInfoScalaJS.fastLinkJS).isRight)
+    test("scalajs") - UnitTester(BuildInfoScalaJS, testModuleSourcesPath / "scala-simple").scoped {
+      eval =>
+        val runResult = eval.outPath / "hello-mill"
+        assert(eval.apply(BuildInfoScalaJS.fastLinkJS).isRight)
     }
 
-    test("static") {
-      val eval = UnitTester(BuildInfoStatic, testModuleSourcesPath / "scala")
+    test("static") - UnitTester(BuildInfoStatic, testModuleSourcesPath / "scala").scoped { eval =>
       // When `buildInfoStaticCompiled = true`, make sure we always create the
       // buildinfo Scala file and never create the resource file
       val runResult = eval.outPath / "hello-mill"
@@ -185,8 +186,7 @@ object BuildInfoTests extends TestSuite {
       assert(os.read(runResult) == scalaVersionString)
     }
 
-    test("java") {
-      val eval = UnitTester(BuildInfoJava, testModuleSourcesPath / "java")
+    test("java") - UnitTester(BuildInfoJava, testModuleSourcesPath / "java").scoped { eval =>
       val runResult = eval.outPath / "hello-mill"
       val Right(_) =
         eval.apply(BuildInfoJava.run(T.task(Args(runResult.toString))))
@@ -197,22 +197,24 @@ object BuildInfoTests extends TestSuite {
       )
     }
 
-    test("java-static") {
-      val eval = UnitTester(BuildInfoJavaStatic, testModuleSourcesPath / "java")
-      val runResult = eval.outPath / "hello-mill"
-      val generatedSrc = eval.outPath / "buildInfoSources.dest" / "foo" / "BuildInfo.java"
-      val Right(_) =
-        eval.apply(BuildInfoJavaStatic.run(T.task(Args(runResult.toString))))
-
-      assert(
-        os.exists(runResult),
-        os.exists(generatedSrc),
-        os.read(runResult) == "not-provided-for-java-modules"
-      )
+    test("java-static") - UnitTester(BuildInfoJavaStatic, testModuleSourcesPath / "java").scoped {
+      eval =>
+        val runResult = eval.outPath / "hello-mill"
+        val generatedSrc = eval.outPath / "buildInfoSources.dest" / "foo" / "BuildInfo.java"
+        val Right(_) =
+          eval.apply(BuildInfoJavaStatic.run(T.task(Args(runResult.toString))))
+
+        assert(
+          os.exists(runResult),
+          os.exists(generatedSrc),
+          os.read(runResult) == "not-provided-for-java-modules"
+        )
     }
 
-    test("generatedSources must be a folder") {
-      val eval = UnitTester(BuildInfoPlain, testModuleSourcesPath / "scala")
+    test("generatedSources must be a folder") - UnitTester(
+      BuildInfoPlain,
+      testModuleSourcesPath / "scala"
+    ).scoped { eval =>
       val buildInfoGeneratedSourcesFolder = eval.outPath / "buildInfoSources.dest"
       val Right(result) = eval.apply(BuildInfoPlain.generatedSources)
       assert(

diff --git a/contrib/docker/test/src/mill/contrib/docker/DockerModuleTest.scala b/contrib/docker/test/src/mill/contrib/docker/DockerModuleTest.scala
@@ -89,8 +89,7 @@ object DockerModuleTest extends TestSuite {
     }
 
     test("dockerfile contents") {
-      test("default options") {
-        val eval = UnitTester(Docker, null)
+      test("default options") - UnitTester(Docker, null).scoped { eval =>
         val Right(result) = eval(Docker.dockerDefault.dockerfile)
         val expected = multineRegex.replaceAllIn(
           """
@@ -106,8 +105,7 @@ object DockerModuleTest extends TestSuite {
         assert(dockerfileStringRefined == expected)
       }
 
-      test("all options") {
-        val eval = UnitTester(Docker, null)
+      test("all options") - UnitTester(Docker, null).scoped { eval =>
         val Right(result) = eval(Docker.dockerAll.dockerfile)
         val expected = multineRegex.replaceAllIn(
           """
@@ -132,8 +130,7 @@ object DockerModuleTest extends TestSuite {
         assert(dockerfileStringRefined == expected)
       }
 
-      test("extra jvm options") {
-        val eval = UnitTester(Docker, null)
+      test("extra jvm options") - UnitTester(Docker, null).scoped { eval =>
         val Right(result) = eval(Docker.dockerJvmOptions.dockerfile)
         val expected = multineRegex.replaceAllIn(
           """

diff --git a/contrib/flyway/test/src/mill/contrib/flyway/BuildTest.scala b/contrib/flyway/test/src/mill/contrib/flyway/BuildTest.scala
@@ -21,14 +21,12 @@ object BuildTest extends TestSuite {
   }
 
   def tests = Tests {
-    test("clean") {
-      val eval = UnitTester(Build, null)
+    test("clean") - UnitTester(Build, null).scoped { eval =>
       val Right(result) = eval(Build.build.flywayClean())
       assert(result.evalCount > 0)
     }
 
-    test("migrate") {
-      val eval = UnitTester(Build, null)
+    test("migrate") - UnitTester(Build, null).scoped { eval =>
       val Right(result) = eval(Build.build.flywayMigrate())
       assert(
         result.evalCount > 0,
@@ -41,8 +39,7 @@ object BuildTest extends TestSuite {
       )
     }
 
-    test("info") {
-      val eval = UnitTester(Build, null)
+    test("info") - UnitTester(Build, null).scoped { eval =>
       val Right(result) = eval(Build.build.flywayInfo())
       assert(result.evalCount > 0)
     }

diff --git a/contrib/gitlab/test/src/mill/contrib/gitlab/GitlabModuleTests.scala b/contrib/gitlab/test/src/mill/contrib/gitlab/GitlabModuleTests.scala
@@ -37,8 +37,10 @@ object GitlabModuleTests extends TestSuite {
 
   override def tests: Tests = Tests {
 
-    test("GitlabPublishModule produces sane error message") {
-      val eval = UnitTester(GitlabModule, null)
+    test("GitlabPublishModule produces sane error message") - UnitTester(
+      GitlabModule,
+      null
+    ).scoped { eval =>
       val e = eval(GitlabModule.gitlabHeaders(Map.empty))
 
       assertMatch(e) {
@@ -47,14 +49,14 @@ object GitlabModuleTests extends TestSuite {
       }
     }
 
-    test("GitlabMavenRepository produces sane error message") {
-      val eval = UnitTester(GLMvnRepo, null)
-      val e = eval(GLMvnRepo.mavenRepository)
+    test("GitlabMavenRepository produces sane error message") - UnitTester(GLMvnRepo, null).scoped {
+      eval =>
+        val e = eval(GLMvnRepo.mavenRepository)
 
-      assertMatch(e) {
-        case Left(Failure(s, _))
-            if s.startsWith("Token lookup for PACKAGE repository") =>
-      }
+        assertMatch(e) {
+          case Left(Failure(s, _))
+              if s.startsWith("Token lookup for PACKAGE repository") =>
+        }
     }
   }
 

diff --git a/contrib/jmh/test/src/mill/contrib/jmh/JmhModuleTest.scala b/contrib/jmh/test/src/mill/contrib/jmh/JmhModuleTest.scala
@@ -20,8 +20,7 @@ object JmhModuleTest extends TestSuite {
 
   def tests = Tests {
     test("jmh") {
-      test("listJmhBenchmarks") {
-        val eval = UnitTester(jmh, testModuleSourcesPath)
+      test("listJmhBenchmarks") - UnitTester(jmh, testModuleSourcesPath).scoped { eval =>
         val paths = EvaluatorPaths.resolveDestPaths(eval.outPath, jmh.listJmhBenchmarks())
         val outFile = paths.dest / "benchmarks.out"
         val Right(result) = eval(jmh.listJmhBenchmarks("-o", outFile.toString))