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.
This commit is contained in:
Ethan Atkins 2019-09-10 11:11:36 -07:00
parent 5e44479dfb
commit af16d33700
1 changed files with 14 additions and 1 deletions

View File

@ -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