diff --git a/src/main/scala/scoverage/IOUtils.scala b/src/main/scala/scoverage/IOUtils.scala index f384eb4c..f8609df9 100644 --- a/src/main/scala/scoverage/IOUtils.scala +++ b/src/main/scala/scoverage/IOUtils.scala @@ -7,12 +7,12 @@ object IOUtils { // loads all the invoked statement ids def invoked(dir: File): Seq[Int] = { - dir.listFiles.map(file => { + dir.listFiles.flatMap { file => val reader = new BufferedReader(new FileReader(file)) val line = reader.readLine() reader.close() line.split(";").filterNot(_.isEmpty).map(_.toInt) - }).flatten.toSeq + } } def serialize(coverage: Coverage, file: File) { diff --git a/src/main/scala/scoverage/Invoker.scala b/src/main/scala/scoverage/Invoker.scala index f8b8ffe9..4c0dc617 100644 --- a/src/main/scala/scoverage/Invoker.scala +++ b/src/main/scala/scoverage/Invoker.scala @@ -8,17 +8,19 @@ object Invoker { /** * We record that the given id has been invoked by appending its id to the coverage * data file. + * * This will happen concurrently on as many threads as the application is using, - * but appending small amounts of data to a file is atomic on both POSIX and Windows - * if it is a single write of a small enough string. + * so we use one file per thread, named for the thread id. * - * @see http://stackoverflow.com/questions/1154446/is-file-append-atomic-in-unix - * @see http://stackoverflow.com/questions/3032482/is-appending-to-a-file-atomic-with-windows-ntfs + * This method is not thread-safe if the threads are in different JVMs, because + * the thread IDs may collide. + * You may not use `scoverage` on multiple processes in parallel without risking + * corruption of the measurement file. */ def invoked(id: Int, path: String) = { - val dir = new File(path) - dir.mkdirs() - val file = new File(path + "/" + Thread.currentThread.getId) + // Each thread writes to a separate measurement file, to reduce contention + // and because file appends via FileWriter are not atomic on Windows. + val file = new File(path, Thread.currentThread.getId.toString) val writer = new FileWriter(file, true) writer.append(id.toString + ';') writer.close() diff --git a/src/main/scala/scoverage/plugin.scala b/src/main/scala/scoverage/plugin.scala index d1d03dce..35296392 100644 --- a/src/main/scala/scoverage/plugin.scala +++ b/src/main/scala/scoverage/plugin.scala @@ -76,7 +76,6 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) def safeSource(tree: Tree): Option[SourceFile] = if (tree.pos.isDefined) Some(tree.pos.source) else None def invokeCall(id: Int): Tree = { - val file = Env.measurementFile(options.dataDir).getAbsolutePath Apply( Select( Select( @@ -90,7 +89,7 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) Constant(id) ), Literal( - Constant(file) + Constant(getMeasurementFilesPath()) ) ) ) @@ -140,6 +139,20 @@ class ScoverageComponent(val global: Global, options: ScoverageOptions) } } + /** + * Get the configured dir in which the measurement files should be written + * (see [[Invoker.invoked()]]) + * + * This path will be hardcoded into the instrumented class files. + * + * We create the dir now (rather than at client runtime), for efficiency. + */ + def getMeasurementFilesPath(): String = { + val dir = Env.measurementFile(options.dataDir).getAbsoluteFile + dir.mkdirs() + dir.getPath + } + def isIncluded(t: Tree): Boolean = { new CoverageFilter(options.excludedPackages).isIncluded(t.symbol.fullNameString) } diff --git a/src/test/scala/scoverage/InvokerConcurrencyTest.scala b/src/test/scala/scoverage/InvokerConcurrencyTest.scala new file mode 100644 index 00000000..d95db422 --- /dev/null +++ b/src/test/scala/scoverage/InvokerConcurrencyTest.scala @@ -0,0 +1,51 @@ +package scoverage + +import org.scalatest.FunSuite +import org.scalatest.BeforeAndAfter +import java.io.File +import scala.concurrent._ +import scala.concurrent.duration._ +import ExecutionContext.Implicits.global +import scala.collection.breakOut + +/** + * Verify that [[Invoker.invoked()]] is thread-safe + */ +class InvokerConcurrencyTest extends FunSuite with BeforeAndAfter { + + val measurementFile = new File("invoker-test.measurement") + + before { + deleteMeasurementFiles() + } + + test("calling Invoker.invoked on multiple threads does not corrupt the measurement file") { + + val testIds: Set[Int] = (1 to 1000).toSet + + // Create 1k "invoked" calls on the common thread pool, to stress test + // the method + val futures: List[Future[Unit]] = testIds.map { i: Int => + future { + Invoker.invoked(i, measurementFile.toString) + } + }(breakOut) + + futures.foreach(Await.result(_, 1.second)) + + // Now verify that the measurement file is not corrupted by loading it + val idsFromFile = IOUtils.invoked(measurementFile).toSet + + idsFromFile === testIds + } + + after { + deleteMeasurementFiles() + } + + private def deleteMeasurementFiles(): Unit = { + measurementFile.getAbsoluteFile.getParentFile.listFiles() + .filter(_.getName.startsWith(measurementFile.getName)) + .foreach(_.delete()) + } +}