Include -release arg in cache key for ct.sym classpath element

The compiler has a per-classloader cache that backs the classpath
lookups of individual instances of `Global`. Elements in the cache are
used by `Global` instances that are concurrent (think parallel
compilation of sub-projects) or are sequential within a small timeout.

In scala#9557, this was extended to the classpath entry that backs the
`scalac -release` compiler option
([JEP-247](https://openjdk.java.net/jeps/247) support for viewing the
Java base library "as of" an older JDK version.

This change was buggy -- it did not include the selected release in the
cache key, which could lead to a compiler that specifies `-release X`
seeing the results of another compiler using `-release Y`.

This behaviour was tested by a JDK-9+ conditional test
(`MultiReleaseJarTest`) which unfortunately is not part of our CI on
the 2.12.x branch, so the regression went unnoticed.

While in this area, I followed up on a TODO comment in the same test
and discovered another bug in handling of multi-release JARs.
Again, this bug could manifest when different values of `-release` were
used in a build. It would manifest as an `IllegalArgumentException`
in `ResuableDataReader` when it used the size of the non-versioned
classfile when sizing buffers for the versioned classfile.

Author: retronym

src/compiler/scala/tools/nsc/classpath/DirectoryClassPath.scala

@@ -130,7 +130,7 @@ trait JFileDirectoryLookup[FileEntryType <: ClassRepresentation] extends Directo
 object JrtClassPath {
   import java.nio.file._, java.net.URI
   private val jrtClassPathCache = new FileBasedCache[Unit, JrtClassPath]()
-  private val ctSymClassPathCache = new FileBasedCache[Unit, CtSymClassPath]()
+  private val ctSymClassPathCache = new FileBasedCache[String, CtSymClassPath]()
   def apply(release: Option[String], closeableRegistry: CloseableRegistry): Option[ClassPath] = {
     import scala.util.Properties._
     if (!isJavaAtLeast("9")) None
@@ -149,7 +149,7 @@ object JrtClassPath {
             val ctSym = Paths.get(javaHome).resolve("lib").resolve("ct.sym")
             if (Files.notExists(ctSym)) None
             else {
-              val classPath = ctSymClassPathCache.getOrCreate((), ctSym :: Nil, () => new CtSymClassPath(ctSym, v.toInt), closeableRegistry, true)
+              val classPath = ctSymClassPathCache.getOrCreate(v, ctSym :: Nil, () => new CtSymClassPath(ctSym, v.toInt), closeableRegistry, true)
               Some(classPath)
             }
           } catch {

src/reflect/scala/reflect/io/ZipArchive.scala

@@ -220,7 +220,7 @@ final class FileZipArchive(file: JFile, release: Option[String]) extends ZipArch
         override def close(): Unit = { zipFilePool.release(zipFile) }
       }
     }
-    override def sizeOption: Option[Int] = Some(size) // could be stale
+    override def sizeOption: Option[Int] = Some(size)
   }
 
   private[this] val dirs = new java.util.HashMap[String, DirEntry]()
@@ -236,16 +236,19 @@ final class FileZipArchive(file: JFile, release: Option[String]) extends ZipArch
         if (!zipEntry.getName.startsWith("META-INF/versions/")) {
           if (!zipEntry.isDirectory) {
             val dir = getDir(dirs, zipEntry)
+            val mrEntry = if (release.isDefined) {
+              zipFile.getEntry(zipEntry.getName)
+            } else zipEntry
             val f =
               if (ZipArchive.closeZipFile)
                 new LazyEntry(
                   zipEntry.getName,
-                  zipEntry.getTime,
-                  zipEntry.getSize.toInt)
+                  mrEntry.getTime,
+                  mrEntry.getSize.toInt)
               else
                 new LeakyEntry(zipEntry.getName,
-                               zipEntry.getTime,
-                               zipEntry.getSize.toInt)
+                               mrEntry.getTime,
+                               mrEntry.getSize.toInt)
 
             dir.entries(f.name) = f
           }

test/junit/scala/tools/nsc/classpath/MultiReleaseJarTest.scala

@@ -4,10 +4,9 @@ import java.io.ByteArrayOutputStream
 import java.nio.file.{FileSystems, Files, Path}
 import java.util.jar.Attributes
 import java.util.jar.Attributes.Name
-
 import org.junit.{Assert, Test}
 
-import scala.tools.nsc.{Global, Settings}
+import scala.tools.nsc.{CloseableRegistry, Global, Settings}
 import scala.tools.testing.BytecodeTesting
 import scala.util.Properties
 
@@ -22,6 +21,7 @@ class MultiReleaseJarTest extends BytecodeTesting {
     // TODO test fails if both Global runs look at the same JAR on disk. Caching problem in our classpath implementation?
     // val temp2 = temp1
     val temp2 = Files.createTempFile("mr-jar-test-", ".jar")
+    val cleanup = new CloseableRegistry
 
     try {
       def code(newApi: String) = s"package p1; abstract class Versioned { def oldApi: Int; $newApi }"
@@ -39,6 +39,7 @@ class MultiReleaseJarTest extends BytecodeTesting {
         settings.usejavacp.value = true
         settings.classpath.value = jarPath.toAbsolutePath.toString
         val g = new Global(settings)
+        cleanup.registerClosable(g)
         settings.release.value = release
         new g.Run
         val decls = g.rootMirror.staticClass("p1.Versioned").info.decls.filterNot(_.isConstructor).map(_.name.toString).toList.sorted
@@ -47,28 +48,38 @@ class MultiReleaseJarTest extends BytecodeTesting {
 
       Assert.assertEquals(List("newApi", "oldApi"), declsOfC(temp1, "9"))
       Assert.assertEquals(List("oldApi"), declsOfC(temp2, "8"))
-    } finally
+    } finally {
+      cleanup.close()
       List(temp1, temp2).foreach(Files.deleteIfExists)
+    }
   }
 
   @Test
   def ctSymTest(): Unit = {
     if (!Properties.isJavaAtLeast("9")) { println("skipping mrJar() on old JDK"); return} // TODO test that the compiler warns that --release is unsupported.
+    val cleanup = new CloseableRegistry
 
     def lookup(className: String, release: String): Boolean = {
       val settings = new Settings()
       settings.usejavacp.value = true
       val g = new Global(settings)
+      cleanup.registerClosable(g)
       import g._
       settings.release.value = release
       new Run
       rootMirror.getClassIfDefined(TypeName(className)) != NoSymbol
     }
-    Assert.assertTrue(lookup("java.lang.invoke.LambdaMetafactory", "8"))
-    Assert.assertFalse(lookup("java.lang.invoke.LambdaMetafactory", "7"))
-    Assert.assertTrue(lookup("java.lang.invoke.LambdaMetafactory", "9"))
+    try {
+      Assert.assertTrue(lookup("java.lang.invoke.LambdaMetafactory", "8"))
+      Assert.assertFalse(lookup("java.lang.invoke.LambdaMetafactory", "7"))
+      Assert.assertTrue(lookup("java.lang.invoke.LambdaMetafactory", "9"))
+    } finally {
+      cleanup.close()
+    }
   }
 
+
+
   private def createManifest = {
     val manifest = new java.util.jar.Manifest()
     manifest.getMainAttributes.put(Name.MANIFEST_VERSION, "1.0")