From ec09e734376886c5d7aeac8e83f2690de8336b5b Mon Sep 17 00:00:00 2001
From: Ethan Atkins <ethan.atkins@gmail.com>
Date: Thu, 9 May 2019 11:58:21 -0700
Subject: [PATCH] Improve cache invalidation strategy

Previously the persistent attribute map was only reset when the file
event monitor detected a change. This made it possible for the cache to
be inconsistent with the state of the file system*. To fix this, I add an
observer on the file tree repository used by the continuous build that
invalidates the cache entry for any path for which it detects a change.
Invalidating the cache does not stamp the file. That only happens either
when a task asks for the stamp for that file or when the file event
monitor reports an event and we must check if the file was updated or
not.

After this change, touching a source file will not trigger a build
unless the contents of the file actually changes.

I added a test that touches one source file in a project and updates the
content of the other. If the source file that is only ever touched ever
triggers a build, then the test fails.

* This could lead to under-compilation because ExternalHooks would not
detect that the file had been updated.
---
 main/src/main/scala/sbt/Defaults.scala        |  8 +-
 .../main/scala/sbt/internal/Continuous.scala  | 69 +++++++++++++---
 .../scala/sbt/internal/ExternalHooks.scala    | 53 ++++--------
 main/src/main/scala/sbt/nio/FileStamp.scala   | 80 +++++++++++++++++--
 main/src/main/scala/sbt/nio/Keys.scala        | 10 +--
 main/src/main/scala/sbt/nio/Settings.scala    | 23 ++----
 main/src/main/scala/sbt/nio/Watch.scala       | 18 ++++-
 .../sbt/internal/FileStampJsonSpec.scala      |  2 +-
 sbt/src/sbt-test/nio/glob-dsl/build.sbt       |  6 +-
 sbt/src/sbt-test/watch/on-change/build.sbt    | 36 +++++++++
 .../sbt-test/watch/on-change/changes/A.scala  |  3 +
 .../sbt-test/watch/on-change/changes/B.scala  |  3 +
 .../sbt-test/watch/on-change/changes/C.scala  |  3 +
 .../on-change/src/main/scala/sbt/test/A.scala |  3 +
 .../on-change/src/main/scala/sbt/test/B.scala |  3 +
 .../on-change/src/main/scala/sbt/test/C.scala |  3 +
 sbt/src/sbt-test/watch/on-change/test         |  3 +
 .../sbt-test/watch/on-start-watch/build.sbt   |  3 +-
 18 files changed, 240 insertions(+), 89 deletions(-)

diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala
index 21c02c79a..e8784039a 100755
--- a/main/src/main/scala/sbt/Defaults.scala
+++ b/main/src/main/scala/sbt/Defaults.scala
@@ -155,10 +155,10 @@ object Defaults extends BuildCommon {
       watchForceTriggerOnAnyChange :== true,
       watchTriggers :== Nil,
       clean := { () },
-      sbt.nio.Keys.fileAttributeMap := {
+      sbt.nio.Keys.fileStampCache := {
         state.value
-          .get(sbt.nio.Keys.persistentFileAttributeMap)
-          .getOrElse(new sbt.nio.Keys.FileAttributeMap)
+          .get(sbt.nio.Keys.persistentFileStampCache)
+          .getOrElse(new sbt.nio.FileStamp.Cache)
       },
     ) ++ TaskRepository
       .proxy(GlobalScope / classLoaderCache, ClassLoaderCache(4)) ++ globalIvyCore ++ globalJvmCore
@@ -1621,7 +1621,7 @@ object Defaults extends BuildCommon {
       val contents = AnalysisContents.create(analysisResult.analysis(), analysisResult.setup())
       store.set(contents)
     }
-    val map = sbt.nio.Keys.fileAttributeMap.value
+    val map = sbt.nio.Keys.fileStampCache.value
     val analysis = analysisResult.analysis
     import scala.collection.JavaConverters._
     analysis.readStamps.getAllProductStamps.asScala.foreach {
diff --git a/main/src/main/scala/sbt/internal/Continuous.scala b/main/src/main/scala/sbt/internal/Continuous.scala
index 4499488b0..5aa62184a 100644
--- a/main/src/main/scala/sbt/internal/Continuous.scala
+++ b/main/src/main/scala/sbt/internal/Continuous.scala
@@ -125,8 +125,10 @@ private[sbt] object Continuous extends DeprecatedContinuous {
   private[sbt] val dynamicInputs = taskKey[Option[mutable.Set[DynamicInput]]](
     "The input globs found during task evaluation that are used in watch."
   )
+
   private[sbt] def dynamicInputsImpl: Def.Initialize[Task[Option[mutable.Set[DynamicInput]]]] =
     Def.task(Keys.state.value.get(DynamicInputs))
+
   private[sbt] val DynamicInputs =
     AttributeKey[mutable.Set[DynamicInput]](
       "dynamic-inputs",
@@ -136,6 +138,7 @@ private[sbt] object Continuous extends DeprecatedContinuous {
 
   private[this] val continuousParser: State => Parser[(Int, String)] = {
     def toInt(s: String): Int = Try(s.toInt).getOrElse(0)
+
     // This allows us to re-enter the watch with the previous count.
     val digitParser: Parser[Int] =
       (Parsers.Space.* ~> matched(Parsers.Digit.+) <~ Parsers.Space.*).map(toInt)
@@ -189,6 +192,7 @@ private[sbt] object Continuous extends DeprecatedContinuous {
       watchSettings
     )
   }
+
   private def getRepository(state: State): FileTreeRepository[FileAttributes] = {
     lazy val exception =
       new IllegalStateException("Tried to access FileTreeRepository for uninitialized state")
@@ -287,17 +291,20 @@ private[sbt] object Continuous extends DeprecatedContinuous {
     } else {
       FileTreeRepository.default
     }
+    val attributeMap = new FileStamp.Cache
+    repo.addObserver(t => attributeMap.invalidate(t.path))
     try {
       val stateWithRepo = state
         .put(globalFileTreeRepository, repo)
-        .put(persistentFileAttributeMap, new sbt.nio.Keys.FileAttributeMap)
+        .put(persistentFileStampCache, attributeMap)
       setup(stateWithRepo, command) { (commands, s, valid, invalid) =>
         EvaluateTask.withStreams(extracted.structure, s)(_.use(streams in Global) { streams =>
           implicit val logger: Logger = streams.log
           if (invalid.isEmpty) {
             val currentCount = new AtomicInteger(count)
             val configs = getAllConfigs(valid.map(v => v._1 -> v._2))
-            val callbacks = aggregate(configs, logger, in, s, currentCount, isCommand, commands)
+            val callbacks =
+              aggregate(configs, logger, in, s, currentCount, isCommand, commands, attributeMap)
             val task = () => {
               currentCount.getAndIncrement()
               // abort as soon as one of the tasks fails
@@ -311,6 +318,12 @@ private[sbt] object Continuous extends DeprecatedContinuous {
             // additional times whenever the state transition callbacks return Watched.Trigger.
             try {
               val terminationAction = Watch(task, callbacks.onStart, callbacks.nextEvent)
+              terminationAction match {
+                case e: Watch.HandleUnexpectedError =>
+                  System.err.println("Caught unexpected error running continuous build:")
+                  e.throwable.printStackTrace(System.err)
+                case _ =>
+              }
               callbacks.onTermination(terminationAction, command, currentCount.get(), state)
             } finally {
               callbacks.onExit()
@@ -340,6 +353,7 @@ private[sbt] object Continuous extends DeprecatedContinuous {
       case _ => Nil: Seq[ScopedKey[_]]
     }
   }
+
   private def getAllConfigs(
       inputs: Seq[(String, State)]
   )(implicit extracted: Extracted, logger: Logger): Seq[Config] = {
@@ -386,7 +400,8 @@ private[sbt] object Continuous extends DeprecatedContinuous {
       state: State,
       count: AtomicInteger,
       isCommand: Boolean,
-      commands: Seq[String]
+      commands: Seq[String],
+      attributeMap: FileStamp.Cache
   )(
       implicit extracted: Extracted
   ): Callbacks = {
@@ -396,7 +411,7 @@ private[sbt] object Continuous extends DeprecatedContinuous {
     val onStart: () => Watch.Action = getOnStart(project, commands, configs, rawLogger, count)
     val nextInputEvent: () => Watch.Action = parseInputEvents(configs, state, inputStream, logger)
     val (nextFileEvent, cleanupFileMonitor): (() => Option[(Watch.Event, Watch.Action)], () => Unit) =
-      getFileEvents(configs, rawLogger, state, count, commands)
+      getFileEvents(configs, rawLogger, state, count, commands, attributeMap)
     val nextEvent: () => Watch.Action =
       combineInputAndFileEvents(nextInputEvent, nextFileEvent, logger)
     val onExit = () => {
@@ -454,14 +469,15 @@ private[sbt] object Continuous extends DeprecatedContinuous {
       res
     }
   }
+
   private def getFileEvents(
       configs: Seq[Config],
       logger: Logger,
       state: State,
       count: AtomicInteger,
-      commands: Seq[String]
+      commands: Seq[String],
+      attributeMap: FileStamp.Cache
   )(implicit extracted: Extracted): (() => Option[(Watch.Event, Watch.Action)], () => Unit) = {
-    val attributeMap = state.get(persistentFileAttributeMap).get
     val trackMetaBuild = configs.forall(_.watchSettings.trackMetaBuild)
     val buildGlobs =
       if (trackMetaBuild) extracted.getOpt(fileInputs in settingsData).getOrElse(Nil)
@@ -471,24 +487,38 @@ private[sbt] object Continuous extends DeprecatedContinuous {
     val quarantinePeriod = configs.map(_.watchSettings.deletionQuarantinePeriod).max
     val onEvent: Event => Seq[(Watch.Event, Watch.Action)] = event => {
       val path = event.path
+
       def watchEvent(stamper: FileStamper, forceTrigger: Boolean): Option[Watch.Event] = {
-        val stamp = FileStamp(path, stamper)
         if (!event.exists) {
+          Some(Deletion(event))
           attributeMap.remove(event.path) match {
             case null => None
             case _    => Some(Deletion(event))
           }
         } else {
-          import sbt.internal.inc.Stamp.equivStamp
-          attributeMap.put(path, stamp) match {
-            case null => Some(Creation(event))
-            case s =>
-              if (forceTrigger || !equivStamp.equiv(s.stamp, stamp.stamp))
+          fileStampCache.update(path, stamper) match {
+            case (None, Some(_)) => Some(Creation(event))
+            case (Some(_), None) => Some(Deletion(event))
+            case (Some(p), Some(c)) =>
+              if (forceTrigger) {
+                val msg =
+                  s"Creating forced update event for path $path (previous stamp: $p, current stamp: $c)"
+                logger.debug(msg)
                 Some(Update(event))
-              else None
+              } else if (p == c) {
+                logger.debug(s"Dropping event for unmodified path $path")
+                None
+              } else {
+                val msg =
+                  s"Creating update event for modified $path (previous stamp: $p, current stamp: $c)"
+                logger.debug(msg)
+                Some(Update(event))
+              }
+            case _ => None
           }
         }
       }
+
       if (buildGlobs.exists(_.matches(path))) {
         watchEvent(FileStamper.Hash, forceTrigger = false).map(e => e -> Watch.Reload).toSeq
       } else {
@@ -519,6 +549,7 @@ private[sbt] object Continuous extends DeprecatedContinuous {
       private implicit class WatchLogger(val l: Logger) extends sbt.internal.nio.WatchLogger {
         override def debug(msg: Any): Unit = l.debug(msg.toString)
       }
+
       // TODO make this a normal monitor
       private[this] val monitors: Seq[FileEventMonitor[Event]] =
         configs.map { config =>
@@ -543,11 +574,13 @@ private[sbt] object Continuous extends DeprecatedContinuous {
                   retentionPeriod
                 ) :: Nil
               } else Nil)
+
       override def poll(duration: Duration, filter: Event => Boolean): Seq[Event] = {
         val res = monitors.flatMap(_.poll(0.millis, filter)).toSet.toVector
         if (res.isEmpty) Thread.sleep(duration.toMillis)
         res
       }
+
       override def close(): Unit = monitors.foreach(_.close())
     }
     val watchLogger: WatchLogger = msg => logger.debug(msg.toString)
@@ -640,7 +673,9 @@ private[sbt] object Continuous extends DeprecatedContinuous {
       val parser = any ~> inputParser ~ matched(any)
       // Each parser gets its own copy of System.in that it can modify while parsing.
       val systemInBuilder = new StringBuilder
+
       def inputStream(string: String): InputStream = new ByteArrayInputStream(string.getBytes)
+
       // This string is provided in the closure below by reading from System.in
       val default: String => Watch.Action =
         string => parse(inputStream(string), systemInBuilder, parser)
@@ -726,7 +761,9 @@ private[sbt] object Continuous extends DeprecatedContinuous {
      */
     new Logger {
       override def trace(t: => Throwable): Unit = logger.trace(t)
+
       override def success(message: => String): Unit = logger.success(message)
+
       override def log(level: Level.Value, message: => String): Unit = {
         val levelString = if (level < delegateLevel) s"[$level] " else ""
         val newMessage = s"[watch] $levelString$message"
@@ -812,12 +849,15 @@ private[sbt] object Continuous extends DeprecatedContinuous {
   ) {
     private[sbt] def watchState(count: Int): DeprecatedWatchState =
       WatchState.empty(inputs().map(_.glob)).withCount(count)
+
     def arguments(logger: Logger): Arguments = new Arguments(logger, inputs())
   }
+
   private def getStartMessage(key: ScopedKey[_])(implicit e: Extracted): StartMessage = Some {
     lazy val default = key.get(watchStartMessage).getOrElse(Watch.defaultStartWatch)
     key.get(deprecatedWatchingMessage).map(Left(_)).getOrElse(Right(default))
   }
+
   private def getTriggerMessage(
       key: ScopedKey[_]
   )(implicit e: Extracted): TriggerMessage = {
@@ -926,9 +966,12 @@ private[sbt] object Continuous extends DeprecatedContinuous {
      */
     def withPrefix(prefix: String): Logger = new Logger {
       override def trace(t: => Throwable): Unit = logger.trace(t)
+
       override def success(message: => String): Unit = logger.success(message)
+
       override def log(level: Level.Value, message: => String): Unit =
         logger.log(level, s"$prefix - $message")
     }
   }
+
 }
diff --git a/main/src/main/scala/sbt/internal/ExternalHooks.scala b/main/src/main/scala/sbt/internal/ExternalHooks.scala
index 400ced085..ffc675f5a 100644
--- a/main/src/main/scala/sbt/internal/ExternalHooks.scala
+++ b/main/src/main/scala/sbt/internal/ExternalHooks.scala
@@ -12,11 +12,13 @@ import java.util.Optional
 
 import sbt.Def
 import sbt.Keys._
-import sbt.internal.inc.{ EmptyStamp, ExternalLookup, Stamper }
+import sbt.internal.inc.ExternalLookup
+import sbt.internal.inc.Stamp.equivStamp.equiv
 import sbt.io.syntax._
 import sbt.nio.Keys._
 import sbt.nio.file.RecursiveGlob
 import sbt.nio.file.syntax._
+import sbt.nio.{ FileStamp, FileStamper }
 import xsbti.compile._
 import xsbti.compile.analysis.Stamp
 
@@ -25,37 +27,22 @@ import scala.collection.mutable
 
 private[sbt] object ExternalHooks {
   private val javaHome = Option(System.getProperty("java.home")).map(Paths.get(_))
-  private[this] implicit class StampOps(val s: Stamp) extends AnyVal {
-    def hash: String = s.getHash.orElse("")
-    def lastModified: Long = s.getLastModified.orElse(-1L)
-  }
   def default: Def.Initialize[sbt.Task[ExternalHooks]] = Def.task {
-    val attributeMap = fileAttributeMap.value
+    val cache = fileStampCache.value
     val cp = dependencyClasspath.value.map(_.data)
     cp.foreach { file =>
       val path = file.toPath
-      attributeMap.get(path) match {
-        case null => attributeMap.put(path, sbt.nio.FileStamp.lastModified(path))
-        case _    =>
-      }
+      cache.getOrElseUpdate(path, FileStamper.LastModified)
     }
     val classGlob = classDirectory.value.toGlob / RecursiveGlob / "*.class"
     fileTreeView.value.list(classGlob).foreach {
-      case (path, _) => attributeMap.put(path, sbt.nio.FileStamp.lastModified(path))
+      case (path, _) => cache.update(path, FileStamper.LastModified)
     }
-    apply(
-      (compileOptions in compile).value,
-      (file: File) => {
-        attributeMap.get(file.toPath) match {
-          case null => EmptyStamp
-          case s    => s.stamp
-        }
-      }
-    )
+    apply((compileOptions in compile).value, cache)
   }
   private def apply(
       options: CompileOptions,
-      attributeMap: File => Stamp
+      fileStampCache: FileStamp.Cache
   ): DefaultExternalHooks = {
     val lookup = new ExternalLookup {
       override def changedSources(previousAnalysis: CompileAnalysis): Option[Changes[File]] = Some {
@@ -70,16 +57,10 @@ private[sbt] object ExternalHooks {
             previousAnalysis.readStamps().getAllSourceStamps.asScala
           prevSources.foreach {
             case (file: File, s: Stamp) =>
-              attributeMap(file) match {
-                case null =>
-                  getRemoved.add(file)
-                case stamp =>
-                  val hash = (if (stamp.getHash.isPresent) stamp else Stamper.forHash(file)).hash
-                  if (hash == s.hash) {
-                    getUnmodified.add(file)
-                  } else {
-                    getChanged.add(file)
-                  }
+              fileStampCache.getOrElseUpdate(file.toPath, FileStamper.Hash) match {
+                case None => getRemoved.add(file)
+                case Some(stamp) =>
+                  if (equiv(stamp.stamp, s)) getUnmodified.add(file) else getChanged.add(file)
               }
           }
           options.sources.foreach(file => if (!prevSources.contains(file)) getAdded.add(file))
@@ -98,8 +79,8 @@ private[sbt] object ExternalHooks {
       override def changedBinaries(previousAnalysis: CompileAnalysis): Option[Set[File]] = {
         Some(previousAnalysis.readStamps.getAllBinaryStamps.asScala.flatMap {
           case (file, stamp) =>
-            attributeMap(file) match {
-              case cachedStamp if stamp.getLastModified == cachedStamp.getLastModified => None
+            fileStampCache.get(file.toPath) match {
+              case Some(cachedStamp) if equiv(cachedStamp.stamp, stamp) => None
               case _ =>
                 javaHome match {
                   case Some(h) if file.toPath.startsWith(h) => None
@@ -112,9 +93,9 @@ private[sbt] object ExternalHooks {
       override def removedProducts(previousAnalysis: CompileAnalysis): Option[Set[File]] = {
         Some(previousAnalysis.readStamps.getAllProductStamps.asScala.flatMap {
           case (file, stamp) =>
-            attributeMap(file) match {
-              case s if s.getLastModified == stamp.getLastModified => None
-              case _                                               => Some(file)
+            fileStampCache.get(file.toPath) match {
+              case Some(s) if equiv(s.stamp, stamp) => None
+              case _                                => Some(file)
             }
         }.toSet)
       }
diff --git a/main/src/main/scala/sbt/nio/FileStamp.scala b/main/src/main/scala/sbt/nio/FileStamp.scala
index b6a9d99e0..dafdca57f 100644
--- a/main/src/main/scala/sbt/nio/FileStamp.scala
+++ b/main/src/main/scala/sbt/nio/FileStamp.scala
@@ -16,8 +16,6 @@ import sbt.nio.file.FileAttributes
 import sjsonnew.{ Builder, JsonFormat, Unbuilder, deserializationError }
 import xsbti.compile.analysis.{ Stamp => XStamp }
 
-import scala.util.Try
-
 sealed trait FileStamper
 object FileStamper {
   case object Hash extends FileStamper
@@ -36,12 +34,11 @@ private[sbt] object FileStamp {
     }
   }
 
-  private[sbt] val converter: (Path, FileAttributes) => Try[FileStamp] = (p, a) => Try(apply(p, a))
-  def apply(path: Path, fileStamper: FileStamper): FileStamp = fileStamper match {
+  def apply(path: Path, fileStamper: FileStamper): Option[FileStamp] = fileStamper match {
     case FileStamper.Hash         => hash(path)
     case FileStamper.LastModified => lastModified(path)
   }
-  def apply(path: Path, fileAttributes: FileAttributes): FileStamp =
+  def apply(path: Path, fileAttributes: FileAttributes): Option[FileStamp] =
     try {
       if (fileAttributes.isDirectory) lastModified(path)
       else
@@ -51,11 +48,17 @@ private[sbt] object FileStamp {
           case _                         => hash(path)
         }
     } catch {
-      case e: IOException => Error(e)
+      case e: IOException => Some(Error(e))
     }
   def hash(string: String): Hash = new FileHashImpl(sbt.internal.inc.Hash.unsafeFromString(string))
-  def hash(path: Path): Hash = new FileHashImpl(Stamper.forHash(path.toFile))
-  def lastModified(path: Path): LastModified = LastModified(IO.getModifiedTimeOrZero(path.toFile))
+  def hash(path: Path): Option[Hash] = Stamper.forHash(path.toFile) match {
+    case EmptyStamp => None
+    case s          => Some(new FileHashImpl(s))
+  }
+  def lastModified(path: Path): Option[LastModified] = IO.getModifiedTimeOrZero(path.toFile) match {
+    case 0 => None
+    case l => Some(LastModified(l))
+  }
   private[this] class FileHashImpl(val xstamp: XStamp) extends Hash(xstamp.getHash.orElse(""))
   sealed abstract case class Hash private[sbt] (hex: String) extends FileStamp
   final case class LastModified private[sbt] (time: Long) extends FileStamp
@@ -209,4 +212,65 @@ private[sbt] object FileStamp {
         }
     }
 
+  private implicit class EitherOps(val e: Either[FileStamp, FileStamp]) extends AnyVal {
+    def value: Option[FileStamp] = if (e == null) None else Some(e.fold(identity, identity))
+  }
+
+  private[sbt] class Cache {
+    private[this] val underlying = new java.util.HashMap[Path, Either[FileStamp, FileStamp]]
+
+    /**
+     * Invalidate the cache entry, but don't re-stamp the file until it's actually used
+     * in a call to get or update.
+     *
+     * @param path the file whose stamp we are invalidating
+     */
+    def invalidate(path: Path): Unit = underlying.get(path) match {
+      case Right(s) =>
+        underlying.put(path, Left(s))
+        ()
+      case _ => ()
+    }
+    def get(path: Path): Option[FileStamp] =
+      underlying.get(path) match {
+        case null     => None
+        case Left(v)  => updateImpl(path, fileStampToStamper(v))
+        case Right(v) => Some(v)
+      }
+    def getOrElseUpdate(path: Path, stamper: FileStamper): Option[FileStamp] =
+      underlying.get(path) match {
+        case null     => updateImpl(path, stamper)
+        case Left(v)  => updateImpl(path, stamper)
+        case Right(v) => Some(v)
+      }
+    def remove(key: Path): Option[FileStamp] = {
+      underlying.remove(key).value
+    }
+    def put(key: Path, fileStamp: FileStamp): Option[FileStamp] =
+      underlying.put(key, Right(fileStamp)) match {
+        case null => None
+        case e    => e.value
+      }
+    def update(key: Path, stamper: FileStamper): (Option[FileStamp], Option[FileStamp]) = {
+      underlying.get(key) match {
+        case null => (None, updateImpl(key, stamper))
+        case v    => (v.value, updateImpl(key, stamper))
+      }
+    }
+
+    private def fileStampToStamper(stamp: FileStamp): FileStamper = stamp match {
+      case _: Hash => FileStamper.Hash
+      case _       => FileStamper.LastModified
+    }
+
+    private def updateImpl(path: Path, stamper: FileStamper): Option[FileStamp] = {
+      val stamp = FileStamp(path, stamper)
+      stamp match {
+        case None    => underlying.remove(path)
+        case Some(s) => underlying.put(path, Right(s))
+      }
+      stamp
+    }
+
+  }
 }
diff --git a/main/src/main/scala/sbt/nio/Keys.scala b/main/src/main/scala/sbt/nio/Keys.scala
index 36e4d6c38..d7577d53c 100644
--- a/main/src/main/scala/sbt/nio/Keys.scala
+++ b/main/src/main/scala/sbt/nio/Keys.scala
@@ -124,16 +124,16 @@ object Keys {
     taskKey[Seq[(Path, FileStamp)]]("Retrieves the hashes for a set of task output files")
       .withRank(Invisible)
   private[sbt] type FileAttributeMap =
-    java.util.HashMap[Path, FileStamp]
-  private[sbt] val persistentFileAttributeMap =
-    AttributeKey[FileAttributeMap]("persistent-file-attribute-map", Int.MaxValue)
+    java.util.Map[Path, FileStamp]
+  private[sbt] val persistentFileStampCache =
+    AttributeKey[FileStamp.Cache]("persistent-file-stamp-cache", Int.MaxValue)
   private[sbt] val allInputPathsAndAttributes =
     taskKey[Seq[(Path, FileAttributes)]]("Get all of the file inputs for a task")
       .withRank(Invisible)
-  private[sbt] val fileAttributeMap = taskKey[FileAttributeMap](
+  private[sbt] val fileStampCache = taskKey[FileStamp.Cache](
     "Map of file stamps that may be cleared between task evaluation runs."
   ).withRank(Invisible)
-  private[sbt] val pathToFileStamp = taskKey[Path => FileStamp](
+  private[sbt] val pathToFileStamp = taskKey[Path => Option[FileStamp]](
     "A function that computes a file stamp for a path. It may have the side effect of updating a cache."
   ).withRank(Invisible)
 }
diff --git a/main/src/main/scala/sbt/nio/Settings.scala b/main/src/main/scala/sbt/nio/Settings.scala
index 97b122b41..a21a89016 100644
--- a/main/src/main/scala/sbt/nio/Settings.scala
+++ b/main/src/main/scala/sbt/nio/Settings.scala
@@ -319,8 +319,9 @@ private[sbt] object Settings {
   private[sbt] def fileStamps(scopedKey: Def.ScopedKey[_]): Def.Setting[_] =
     addTaskDefinition(Keys.inputFileStamps in scopedKey.scope := {
       val stamper = (Keys.pathToFileStamp in scopedKey.scope).value
-      (Keys.allInputPathsAndAttributes in scopedKey.scope).value.collect {
-        case (p, a) if a.isRegularFile && !Files.isHidden(p) => p -> stamper(p)
+      (Keys.allInputPathsAndAttributes in scopedKey.scope).value.flatMap {
+        case (p, a) if a.isRegularFile && !Files.isHidden(p) => stamper(p).map(p -> _)
+        case _                                               => None
       }
     })
   private[this] def outputsAndStamps[T: JsonFormat: ToSeqPath](
@@ -340,11 +341,11 @@ private[sbt] object Settings {
     })
   private[this] def outputFileStampsImpl(scope: Scope): Def.Setting[_] =
     addTaskDefinition(outputFileStamps in scope := {
-      val stamper: Path => FileStamp = (outputFileStamper in scope).value match {
+      val stamper: Path => Option[FileStamp] = (outputFileStamper in scope).value match {
         case LastModified => FileStamp.lastModified
         case Hash         => FileStamp.hash
       }
-      (allOutputFiles in scope).value.map(p => p -> stamper(p))
+      (allOutputFiles in scope).value.flatMap(p => stamper(p).map(p -> _))
     })
 
   /**
@@ -356,18 +357,8 @@ private[sbt] object Settings {
    */
   private[this] def stamper(scopedKey: Def.ScopedKey[_]): Def.Setting[_] =
     addTaskDefinition((Keys.pathToFileStamp in scopedKey.scope) := {
-      val attributeMap = Keys.fileAttributeMap.value
+      val attributeMap = Keys.fileStampCache.value
       val stamper = (Keys.inputFileStamper in scopedKey.scope).value
-      path: Path =>
-        attributeMap.get(path) match {
-          case null =>
-            val stamp = stamper match {
-              case Hash         => FileStamp.hash(path)
-              case LastModified => FileStamp.lastModified(path)
-            }
-            attributeMap.put(path, stamp)
-            stamp
-          case s => s
-        }
+      path: Path => attributeMap.getOrElseUpdate(path, stamper)
     })
 }
diff --git a/main/src/main/scala/sbt/nio/Watch.scala b/main/src/main/scala/sbt/nio/Watch.scala
index c3d22ad6f..3091fa285 100644
--- a/main/src/main/scala/sbt/nio/Watch.scala
+++ b/main/src/main/scala/sbt/nio/Watch.scala
@@ -217,7 +217,7 @@ object Watch {
    * Action that indicates that an error has occurred. The watch will be terminated when this action
    * is produced.
    */
-  final class HandleError(val throwable: Throwable) extends CancelWatch {
+  sealed class HandleError(val throwable: Throwable) extends CancelWatch {
     override def equals(o: Any): Boolean = o match {
       case that: HandleError => this.throwable == that.throwable
       case _                 => false
@@ -226,6 +226,15 @@ object Watch {
     override def toString: String = s"HandleError($throwable)"
   }
 
+  /**
+   * Action that indicates that an error has occurred. The watch will be terminated when this action
+   * is produced.
+   */
+  private[sbt] final class HandleUnexpectedError(override val throwable: Throwable)
+      extends HandleError(throwable) {
+    override def toString: String = s"HandleUnexpectedError($throwable)"
+  }
+
   /**
    * Action that indicates that the watch should continue as though nothing happened. This may be
    * because, for example, no user input was yet available.
@@ -279,7 +288,12 @@ object Watch {
   def apply(task: () => Unit, onStart: NextAction, nextAction: NextAction): Watch.Action = {
     def safeNextAction(delegate: NextAction): Watch.Action =
       try delegate()
-      catch { case NonFatal(t) => new HandleError(t) }
+      catch {
+        case NonFatal(t) =>
+          System.err.println(s"Watch caught unexpected error:")
+          t.printStackTrace(System.err)
+          new HandleError(t)
+      }
     @tailrec def next(): Watch.Action = safeNextAction(nextAction) match {
       // This should never return Ignore due to this condition.
       case Ignore => next()
diff --git a/main/src/test/scala/sbt/internal/FileStampJsonSpec.scala b/main/src/test/scala/sbt/internal/FileStampJsonSpec.scala
index bcf002e77..75cb961fd 100644
--- a/main/src/test/scala/sbt/internal/FileStampJsonSpec.scala
+++ b/main/src/test/scala/sbt/internal/FileStampJsonSpec.scala
@@ -45,6 +45,6 @@ class FileStampJsonSpec extends FlatSpec {
     val both: Seq[(Path, FileStamp)] = hashes ++ lastModifiedTimes
     val json = Converter.toJsonUnsafe(both)(fileStampJsonFormatter)
     val deserialized = Converter.fromJsonUnsafe(json)(fileStampJsonFormatter)
-    assert(both.sameElements(deserialized))
+    assert(both == deserialized)
   }
 }
diff --git a/sbt/src/sbt-test/nio/glob-dsl/build.sbt b/sbt/src/sbt-test/nio/glob-dsl/build.sbt
index c2454aaf8..0f519f646 100644
--- a/sbt/src/sbt-test/nio/glob-dsl/build.sbt
+++ b/sbt/src/sbt-test/nio/glob-dsl/build.sbt
@@ -14,13 +14,13 @@ checkFoo := assert(foo.value == Seq(baseDirectory.value / "base/subdir/nested-su
 // Check that we can correctly extract Bar.md with a non-recursive source
 val bar = taskKey[Seq[File]]("Retrieve Bar.md")
 
-bar / fileInputs += baseDirectory.value.toGlob / "base/subdir/nested-subdir/*.md"
+bar / fileInputs += baseDirectory.value.toGlob / "base" / "subdir" / "nested-subdir" / "*.md"
 
 bar := (bar / allInputFiles).value.map(_.toFile)
 
 val checkBar = taskKey[Unit]("Check that the Bar.md file is retrieved")
 
-checkBar := assert(bar.value == Seq(baseDirectory.value / "base/subdir/nested-subdir/Bar.md"))
+checkBar := assert(bar.value == Seq(baseDirectory.value / "base" / "subdir" / "nested-subdir" / "Bar.md"))
 
 // Check that we can correctly extract Bar.md and Foo.md with a non-recursive source
 val all = taskKey[Seq[File]]("Retrieve all files")
@@ -31,7 +31,7 @@ val checkAll = taskKey[Unit]("Check that the Bar.md file is retrieved")
 
 checkAll := {
   import sbt.dsl.LinterLevel.Ignore
-  val expected = Set("Foo.txt", "Bar.md").map(baseDirectory.value / "base/subdir/nested-subdir" / _)
+  val expected = Set("Foo.txt", "Bar.md").map(baseDirectory.value / "base" / "subdir" / "nested-subdir" / _)
   val actual = (all / allInputFiles).value.map(_.toFile).toSet
   assert(actual == expected)
 }
diff --git a/sbt/src/sbt-test/watch/on-change/build.sbt b/sbt/src/sbt-test/watch/on-change/build.sbt
index e69de29bb..b1ccae48d 100644
--- a/sbt/src/sbt-test/watch/on-change/build.sbt
+++ b/sbt/src/sbt-test/watch/on-change/build.sbt
@@ -0,0 +1,36 @@
+import java.nio.file._
+import sbt.nio.Keys._
+import sbt.nio._
+import scala.concurrent.duration._
+import StandardCopyOption.{ REPLACE_EXISTING => replace }
+
+watchTriggeredMessage := { (i, path: Path, c) =>
+  val prev = watchTriggeredMessage.value
+  if (path.getFileName.toString == "C.scala")
+    throw new IllegalStateException("C.scala should not trigger")
+  prev(i, path, c)
+}
+
+watchOnIteration := { i: Int =>
+  val base = baseDirectory.value.toPath
+  val src =
+    base.resolve("src").resolve("main").resolve("scala").resolve("sbt").resolve("test")
+  val changes = base.resolve("changes")
+  Files.copy(changes.resolve("C.scala"), src.resolve("C.scala"), replace)
+  if (i < 5) {
+    val content =
+      new String(Files.readAllBytes(changes.resolve("A.scala"))) + "\n" + ("//" * i)
+    Files.write(src.resolve("A.scala"), content.getBytes)
+  } else {
+    Files.copy(changes.resolve("B.scala"), src.resolve("B.scala"), replace)
+  }
+  println(s"Waiting for changes...")
+  Watch.Ignore
+}
+
+watchOnFileInputEvent := { (_, event: Watch.Event) =>
+  if (event.path.getFileName.toString == "B.scala") Watch.CancelWatch
+  else Watch.Trigger
+}
+
+watchAntiEntropy := 0.millis
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/changes/A.scala b/sbt/src/sbt-test/watch/on-change/changes/A.scala
index e69de29bb..c0f3723e9 100644
--- a/sbt/src/sbt-test/watch/on-change/changes/A.scala
+++ b/sbt/src/sbt-test/watch/on-change/changes/A.scala
@@ -0,0 +1,3 @@
+package sbt.test
+
+class A
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/changes/B.scala b/sbt/src/sbt-test/watch/on-change/changes/B.scala
index e69de29bb..b09c653dd 100644
--- a/sbt/src/sbt-test/watch/on-change/changes/B.scala
+++ b/sbt/src/sbt-test/watch/on-change/changes/B.scala
@@ -0,0 +1,3 @@
+package sbt.test
+
+class B
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/changes/C.scala b/sbt/src/sbt-test/watch/on-change/changes/C.scala
index e69de29bb..c4f0b26b3 100644
--- a/sbt/src/sbt-test/watch/on-change/changes/C.scala
+++ b/sbt/src/sbt-test/watch/on-change/changes/C.scala
@@ -0,0 +1,3 @@
+package sbt.test
+
+class C
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/A.scala b/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/A.scala
index e69de29bb..8a803930e 100644
--- a/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/A.scala
+++ b/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/A.scala
@@ -0,0 +1,3 @@
+package sbt.test
+
+class A {
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/B.scala b/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/B.scala
index e69de29bb..dc7d2140d 100644
--- a/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/B.scala
+++ b/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/B.scala
@@ -0,0 +1,3 @@
+package sbt.test
+
+class B {
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/C.scala b/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/C.scala
index e69de29bb..c4f0b26b3 100644
--- a/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/C.scala
+++ b/sbt/src/sbt-test/watch/on-change/src/main/scala/sbt/test/C.scala
@@ -0,0 +1,3 @@
+package sbt.test
+
+class C
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-change/test b/sbt/src/sbt-test/watch/on-change/test
index e69de29bb..d2a71e6d9 100644
--- a/sbt/src/sbt-test/watch/on-change/test
+++ b/sbt/src/sbt-test/watch/on-change/test
@@ -0,0 +1,3 @@
+> ~compile
+
+> compile
\ No newline at end of file
diff --git a/sbt/src/sbt-test/watch/on-start-watch/build.sbt b/sbt/src/sbt-test/watch/on-start-watch/build.sbt
index b66bb6199..5fbef8dd3 100644
--- a/sbt/src/sbt-test/watch/on-start-watch/build.sbt
+++ b/sbt/src/sbt-test/watch/on-start-watch/build.sbt
@@ -19,6 +19,7 @@ failingTask := {
 Compile / compile := {
   Count.increment()
   // Trigger a new build by updating the last modified time
-  ((Compile / scalaSource).value / "A.scala").setLastModified(5000)
+  val file = (Compile / scalaSource).value / "A.scala"
+  IO.write(file, IO.read(file) + ("\n" * Count.get))
   (Compile / compile).value
 }