mirror of https://github.com/sbt/sbt.git
[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)
This commit is contained in:
parent
7218b2a1ac
commit
eb70fce097
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue