From eb70fce097424b89fa7bdb8b1ac8a6576b678e0e Mon Sep 17 00:00:00 2001 From: bitloi Date: Thu, 9 Apr 2026 09:01:01 +0200 Subject: [PATCH] [2.x] fix: Fix FileAlreadyExistsException in packageDirectory **Problem** Intermittent java.nio.file.FileAlreadyExistsException when publishing classes.sbtdir.zip during action-cache packaging under parallel tasks (#9043). Copying from a temp directory straight into the final path races on the fixed destination name. **Solution** Stage the built zip next to the destination with a unique temp file, then replace the final path via Files.move with REPLACE_EXISTING and ATOMIC_MOVE, falling back to a non-atomic move when needed. Add a concurrent packageDirectory test. Closes #9043 Generated-by: Cursor (AI-assisted) --- .../src/main/scala/sbt/util/ActionCache.scala | 34 +++++++++++++- .../test/scala/sbt/util/ActionCacheTest.scala | 44 ++++++++++++++++--- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/util-cache/src/main/scala/sbt/util/ActionCache.scala b/util-cache/src/main/scala/sbt/util/ActionCache.scala index 04c62f8f9..56551b42c 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCache.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCache.scala @@ -10,7 +10,7 @@ package sbt.util import java.io.{ File, IOException } import java.nio.charset.StandardCharsets -import java.nio.file.{ Files, Path, Paths, StandardCopyOption } +import java.nio.file.{ AtomicMoveNotSupportedException, Files, Path, Paths, StandardCopyOption } import sbt.internal.util.{ ActionCacheEvent, CacheEventLog, StringVirtualFile1 } import sbt.io.syntax.* import sbt.io.IO @@ -270,6 +270,36 @@ object ActionCache: private val default2010Timestamp: Long = 1262304000000L + /** + * Publishes `builtZip` as `destZip` by staging next to the destination and renaming into place. + * Avoids races from a direct `Files.copy` into `destZip` under parallel task execution. + */ + private def installPackagedZip(builtZip: Path, destZip: Path, fallbackStagingDir: Path): Unit = + val stagingDir = Option(destZip.getParent) match + case Some(parent) => + Files.createDirectories(parent) + parent + case None => fallbackStagingDir + + val staging = Files.createTempFile( + stagingDir, + destZip.getFileName.toString + ".", + dirZipExt + ".tmp", + ) + try + Files.copy(builtZip, staging, StandardCopyOption.REPLACE_EXISTING) + try + Files.move( + staging, + destZip, + StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE, + ) + catch + case _: AtomicMoveNotSupportedException => + Files.move(staging, destZip, StandardCopyOption.REPLACE_EXISTING) + finally Files.deleteIfExists(staging) + def packageDirectory( dir: VirtualFileRef, conv: FileConverter, @@ -311,7 +341,7 @@ object ActionCache: tempZipPath.toFile(), Some(default2010Timestamp) ) - Files.copy(tempZipPath, zipPath, StandardCopyOption.REPLACE_EXISTING) + installPackagedZip(tempZipPath, zipPath, tempDir.toPath()) conv.toVirtualFile(zipPath) diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index 708e9a646..5054218f3 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -1,5 +1,10 @@ package sbt.util +import java.nio.charset.StandardCharsets +import java.nio.file.{ Files, Path, Paths } +import java.util.Optional +import java.util.concurrent.{ CyclicBarrier, ExecutorService, Executors, TimeUnit } + import sbt.internal.util.CacheEventLog import sbt.internal.util.StringVirtualFile1 import sbt.io.IO @@ -7,18 +12,15 @@ import sbt.io.syntax.* import verify.BasicTestSuite import xsbti.{ CompileFailed, + FileConverter, HashedVirtualFileRef, Problem, Position, Severity, VirtualFile, - FileConverter, - VirtualFileRef + VirtualFileRef, } -import java.nio.file.Path -import java.nio.file.Paths -import java.nio.file.Files -import java.util.Optional + import ActionCache.InternalActionResult object ActionCacheTest extends BasicTestSuite: @@ -259,6 +261,36 @@ object ActionCacheTest extends BasicTestSuite: assert(v2 == 42) assert(called == 2) + test("packageDirectory is safe when many threads package the same directory concurrently"): + IO.withTemporaryDirectory: tmp => + val root = tmp.toPath + val classesDir = root.resolve("classes") + Files.createDirectories(classesDir) + Files.writeString(classesDir.resolve("A.class"), "compiled") + val classesPathStr = classesDir.toString + val dirRef = VirtualFileRef.of(classesPathStr) + val conv = new FileConverter: + override def toPath(ref: VirtualFileRef): Path = Paths.get(ref.id) + override def toVirtualFile(path: Path): VirtualFile = + val content = + if Files.isRegularFile(path) then + new String(Files.readAllBytes(path), StandardCharsets.UTF_8) + else "" + StringVirtualFile1(path.toString, content) + val threadCount = 64 + val barrier = new CyclicBarrier(threadCount) + val pool: ExecutorService = Executors.newFixedThreadPool(threadCount) + try + val tasks = + for _ <- 1 to threadCount yield pool.submit: () => + barrier.await(30, TimeUnit.SECONDS) + ActionCache.packageDirectory(dirRef, conv, root) + tasks.foreach(_.get(60, TimeUnit.SECONDS)) + val zipPath = Paths.get(classesPathStr + ActionCache.dirZipExt) + assert(Files.isRegularFile(zipPath)) + assert(Files.size(zipPath) > 0L) + finally pool.shutdown() + test("Changing cacheVersion invalidates the cache"): withDiskCache(testCacheVersionInvalidation)