From 0bea7e9e182329f888d7c9b2f5ce8c3fc034defe Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 5 Mar 2018 18:06:46 +1000 Subject: [PATCH 1/2] Fix race condition in non-forked, parallel tests. Non forked tests that are run in parallel groups can call into a single instance of TestStatusReporter concurrently. This seems to be limited to the startGroup/endGroup/testEvent methods called in: https://github.com/sbt/sbt/blob/a41727fb17bb923036f90ca2ca62897def1a893e/testing/src/main/scala/sbt/TestFramework.scala#L100-L124 Which itself is called within: https://github.com/sbt/sbt/blob/a41727fb17bb923036f90ca2ca62897def1a893e/testing/src/main/scala/sbt/TestFramework.scala#L203-L229 Creating the `runnables` that are run in parallel (in builds so configured): https://github.com/sbt/sbt/blob/a6eb1260c8162bfbfcbfb8656f152854a93d33ae/main-actions/src/main/scala/sbt/Tests.scala#L222-L230 We believe this to be the cause of the hang witnessed in the a suite of Scalacheck-framework tests in the Scala build: https://github.com/scala/scala-jenkins-infra/issues/249 This commit uses a concurrent map to support concurrent status updates. --- .../main/scala/sbt/internal/server/Server.scala | 3 ++- .../src/main/scala/sbt/TestStatusReporter.scala | 17 +++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/main-command/src/main/scala/sbt/internal/server/Server.scala b/main-command/src/main/scala/sbt/internal/server/Server.scala index 05b86c48a..c2bccb886 100644 --- a/main-command/src/main/scala/sbt/internal/server/Server.scala +++ b/main-command/src/main/scala/sbt/internal/server/Server.scala @@ -63,7 +63,8 @@ private[sbt] object Server { val maxSocketLength = new UnixDomainSocketLibrary.SockaddrUn().sunPath.length - 1 val path = socketfile.getAbsolutePath if (path.length > maxSocketLength) - sys.error("socket file absolute path too long; " + + sys.error( + "socket file absolute path too long; " + "either switch to another connection type " + "or define a short \"SBT_GLOBAL_SERVER_DIR\" value. " + s"Current path: ${path}") diff --git a/testing/src/main/scala/sbt/TestStatusReporter.scala b/testing/src/main/scala/sbt/TestStatusReporter.scala index 732e3257c..a99d084fe 100644 --- a/testing/src/main/scala/sbt/TestStatusReporter.scala +++ b/testing/src/main/scala/sbt/TestStatusReporter.scala @@ -8,14 +8,16 @@ package sbt import java.io.File -import sbt.io.IO -import scala.collection.mutable.Map +import sbt.io.IO import sbt.protocol.testing.TestResult +import java.util.concurrent.ConcurrentHashMap + +import scala.collection.concurrent // Assumes exclusive ownership of the file. private[sbt] class TestStatusReporter(f: File) extends TestsListener { - private lazy val succeeded = TestStatus.read(f) + private lazy val succeeded: concurrent.Map[String, Long] = TestStatus.read(f) def doInit = () def startGroup(name: String): Unit = { succeeded remove name } @@ -32,13 +34,16 @@ private[sbt] class TestStatusReporter(f: File) extends TestsListener { private[sbt] object TestStatus { import java.util.Properties - def read(f: File): Map[String, Long] = { + def read(f: File): concurrent.Map[String, Long] = { import scala.collection.JavaConverters._ val properties = new Properties IO.load(properties, f) - properties.asScala map { case (k, v) => (k, v.toLong) } + val result = new ConcurrentHashMap[String, Long]() + properties.asScala.iterator.foreach { case (k, v) => result.put(k, v.toLong) } + result.asScala } - def write(map: Map[String, Long], label: String, f: File): Unit = { + + def write(map: collection.Map[String, Long], label: String, f: File): Unit = { val properties = new Properties for ((test, lastSuccessTime) <- map) properties.setProperty(test, lastSuccessTime.toString) From 49b4c3a3e0c46266401ac1dd7e050f2f9b06cbc8 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Wed, 7 Mar 2018 15:31:24 -0500 Subject: [PATCH 2/2] mima --- build.sbt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build.sbt b/build.sbt index bb6df2c9d..85e006027 100644 --- a/build.sbt +++ b/build.sbt @@ -203,6 +203,11 @@ lazy val testingProj = (project in file("testing")) sourceManaged in (Compile, generateContrabands) := baseDirectory.value / "src" / "main" / "contraband-scala", contrabandFormatsForType in generateContrabands in Compile := ContrabandConfig.getFormats, mimaSettings, + mimaBinaryIssueFilters ++= Seq( + // private[sbt] + ProblemFilters.exclude[IncompatibleMethTypeProblem]("sbt.TestStatus.write"), + ProblemFilters.exclude[IncompatibleResultTypeProblem]("sbt.TestStatus.read"), + ), ) .configure(addSbtIO, addSbtCompilerClasspath, addSbtUtilLogging)