From af16d33700ed6ec0128559ac4e4d7edb10628c9c Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 10 Sep 2019 11:11:36 -0700 Subject: [PATCH] Add per file stream locks We have seen failures in scripted to create the output file for streams and it has also been reported in https://github.com/sbt/sbt/issues/5067. I believe this may caused by the same stream output being initialized by multiple tasks. To fix this, I add locking on a per-file basis. There was (and is) additional synchronization on the Streams _instance_, but the per-file locks are stored in the Streams companion object so the locking should be honored no matter which Streams instance calls make. I also skip the call to IO.touch if the file already exists. I believe that this is the common case and since IO.touch was being called with setModified = false, it should be fine to skip the touch when the file exists. Prior to this change, I was able to induce the issue in #5067 in roughly 1/50 of calls to `scripted actions/cross-multiproject`. I wasn't able to reproduce after this change. --- .../src/main/scala/sbt/std/Streams.scala | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tasks-standard/src/main/scala/sbt/std/Streams.scala b/tasks-standard/src/main/scala/sbt/std/Streams.scala index 4fcf6050a..b71c8567c 100644 --- a/tasks-standard/src/main/scala/sbt/std/Streams.scala +++ b/tasks-standard/src/main/scala/sbt/std/Streams.scala @@ -9,6 +9,7 @@ package sbt package std import java.io.{ File => _, _ } +import java.util.concurrent.ConcurrentHashMap import sbt.internal.io.DeferredWriter import sbt.internal.util.ManagedLogger @@ -98,6 +99,7 @@ object Streams { try { c.close() } catch { case _: IOException => () } + private[this] val streamLocks = new ConcurrentHashMap[File, AnyRef]() def closeable[Key](delegate: Streams[Key]): CloseableStreams[Key] = new CloseableStreams[Key] { private[this] val streams = new collection.mutable.HashMap[Key, ManagedStreams[Key]] @@ -184,7 +186,18 @@ object Streams { def make[T <: Closeable](a: Key, sid: String)(f: File => T): T = synchronized { checkOpen() val file = taskDirectory(a) / sid - IO.touch(file, false) + val parent = file.getParentFile + val newLock = new AnyRef + val lock = streamLocks.putIfAbsent(parent, newLock) match { + case null => newLock + case l => l + } + try lock.synchronized { + if (!file.exists) IO.touch(file, setModified = false) + } finally { + streamLocks.remove(parent) + () + } val t = f(file) opened ::= t t