Don't serve stale JAR metadata from cache in classpath implementation

Adapted from scalac commit bcf47b165ccfd8e1827188f70aeb24e2cecfb80f by
Jason Zaugg in scala/scala#6064:

Use the last modified timestamp and the file inode to help detect
when the file has been overwritten (as is common in SBT builds
with `exportJars := true`, or when using snapshot dependencies).

Fixes scala/bug#10295

Author: allanrenucci

compiler/src/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactory.scala

@@ -5,37 +5,31 @@ package dotty.tools.dotc.classpath
 
 import java.io.File
 import java.net.URL
+import java.nio.file.Files
+import java.nio.file.attribute.{BasicFileAttributes, FileTime}
+
 import scala.annotation.tailrec
 import dotty.tools.io.{AbstractFile, ClassPath, ClassRepresentation, FileZipArchive, ManifestResources}
 import dotty.tools.dotc.core.Contexts.Context
 import FileUtils._
 
 /**
  * A trait providing an optional cache for classpath entries obtained from zip and jar files.
- * It's possible to create such a cache assuming that entries in such files won't change (at
- * least will be the same each time we'll load classpath during the lifetime of JVM process)
- * - unlike class and source files in directories, which can be modified and recompiled.
  * It allows us to e.g. reduce significantly memory used by PresentationCompilers in Scala IDE
  * when there are a lot of projects having a lot of common dependencies.
  */
 sealed trait ZipAndJarFileLookupFactory {
-  private val cache = collection.mutable.Map.empty[AbstractFile, ClassPath]
+  private val cache = new FileBasedCache[ClassPath]
 
   def create(zipFile: AbstractFile)(implicit ctx: Context): ClassPath = {
-    if (ctx.settings.YdisableFlatCpCaching.value) createForZipFile(zipFile)
+    if (ctx.settings.YdisableFlatCpCaching.value || zipFile.file == null) createForZipFile(zipFile)
     else createUsingCache(zipFile)
   }
 
   protected def createForZipFile(zipFile: AbstractFile): ClassPath
 
-  private def createUsingCache(zipFile: AbstractFile)(implicit ctx: Context): ClassPath = cache.synchronized {
-    def newClassPathInstance = {
-      if (ctx.settings.verbose.value || ctx.settings.Ylogcp.value)
-        println(s"$zipFile is not yet in the classpath cache")
-      createForZipFile(zipFile)
-    }
-    cache.getOrElseUpdate(zipFile, newClassPathInstance)
-  }
+  private def createUsingCache(zipFile: AbstractFile): ClassPath =
+    cache.getOrCreate(zipFile.file.toPath, () => createForZipFile(zipFile))
 }
 
 /**
@@ -179,3 +173,29 @@ object ZipAndJarSourcePathFactory extends ZipAndJarFileLookupFactory {
 
   override protected def createForZipFile(zipFile: AbstractFile): ClassPath = ZipArchiveSourcePath(zipFile.file)
 }
+
+final class FileBasedCache[T] {
+  private case class Stamp(lastModified: FileTime, fileKey: Object)
+  private val cache = collection.mutable.Map.empty[java.nio.file.Path, (Stamp, T)]
+
+  def getOrCreate(path: java.nio.file.Path, create: () => T): T = cache.synchronized {
+    val attrs = Files.readAttributes(path, classOf[BasicFileAttributes])
+    val lastModified = attrs.lastModifiedTime()
+    // only null on some platforms, but that's okay, we just use the last modified timestamp as our stamp
+    val fileKey = attrs.fileKey()
+    val stamp = Stamp(lastModified, fileKey)
+    cache.get(path) match {
+      case Some((cachedStamp, cached)) if cachedStamp == stamp => cached
+      case _ =>
+        val value = create()
+        cache.put(path, (stamp, value))
+        value
+    }
+  }
+
+  def clear(): Unit = cache.synchronized {
+    // TODO support closing
+    // cache.valuesIterator.foreach(_.close())
+    cache.clear()
+  }
+}

compiler/test/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactoryTest.scala

@@ -0,0 +1,68 @@
+package dotty.tools.dotc.classpath
+
+import org.junit.Test
+import java.nio.file._
+import java.nio.file.attribute.FileTime
+
+import dotty.tools.dotc.core.Contexts.{Context, ContextBase}
+import dotty.tools.io.AbstractFile
+
+
+
+class ZipAndJarFileLookupFactoryTest {
+  @Test def cacheInvalidation(): Unit = {
+    val f = Files.createTempFile("test-", ".jar")
+    Files.delete(f)
+
+    implicit val ctx: Context = new ContextBase().initialCtx
+    assert(!ctx.settings.YdisableFlatCpCaching.value) // we're testing with our JAR metadata caching enabled.
+
+    def createCp = ZipAndJarClassPathFactory.create(AbstractFile.getFile(f.toFile))
+    try {
+      createZip(f, Array(), "p1/C.class")
+      createZip(f, Array(), "p2/X.class")
+      createZip(f, Array(), "p3/Y.class")
+      val cp1 = createCp
+      assert(cp1.findClass("p1.C").isDefined)
+
+      // We expect get a cache hit as the underlying zip hasn't changed
+      val cp2 = createCp
+      assert(cp2 eq cp1)
+
+      // check things work after the cache hit
+      cp1.findClassFile("p2.X").get.toByteArray
+
+      val lastMod1 = Files.getLastModifiedTime(f)
+      // Create a new zip at the same path with different contents and last modified
+      Files.delete(f)
+      createZip(f, Array(), "p1/D.class")
+      Files.setLastModifiedTime(f, FileTime.fromMillis(lastMod1.toMillis + 2000))
+
+      // Our classpath cache should create a new instance
+      val cp3 = createCp
+      assert(cp1 ne cp3, (System.identityHashCode(cp1), System.identityHashCode(cp3)))
+      // And that instance should see D, not C, in package p1.
+      assert(cp3.findClass("p1.C").isEmpty)
+      assert(cp3.findClass("p1.D").isDefined)
+    } finally Files.delete(f)
+  }
+
+  def createZip(zipLocation: Path, content: Array[Byte], internalPath: String): Unit = {
+    val env = new java.util.HashMap[String, String]()
+    env.put("create", String.valueOf(Files.notExists(zipLocation)))
+    val fileUri = zipLocation.toUri
+    val zipUri = new java.net.URI("jar:" + fileUri.getScheme, fileUri.getPath, null)
+    val zipfs = FileSystems.newFileSystem(zipUri, env)
+    try {
+      try {
+        val internalTargetPath = zipfs.getPath(internalPath)
+        Files.createDirectories(internalTargetPath.getParent)
+        Files.write(internalTargetPath, content)
+      } finally {
+        if (zipfs != null) zipfs.close()
+      }
+    } finally {
+      zipfs.close()
+    }
+  }
+}