From 8c5c1e6b6dbcd3c1030283b71136c9d6d2315a19 Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Sun, 17 May 2026 00:54:58 -0400 Subject: [PATCH] [2.0.x] fix: Ensure resources are copied atomically (#9234) Bumps io to released 1.12.0 which contains the atomic-write changes to IO.copyFile, IO.transfer(InputStream, File), and IO.jar/zip. ActionCacheStore.putBlob now uses IO.transfer instead of duplicating staging logic. Co-authored-by: Anatolii Kmetiuk --- main-actions/src/main/scala/sbt/Pkg.scala | 4 +- .../scala/sbt/util/ActionCacheStore.scala | 29 ++++++++------ .../test/scala/sbt/util/ActionCacheTest.scala | 39 +++++++++++++++++++ 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/main-actions/src/main/scala/sbt/Pkg.scala b/main-actions/src/main/scala/sbt/Pkg.scala index fad3e2c19..11d088ec0 100644 --- a/main-actions/src/main/scala/sbt/Pkg.scala +++ b/main-actions/src/main/scala/sbt/Pkg.scala @@ -226,9 +226,7 @@ object Pkg: val path = jar.getAbsolutePath log.debug("Packaging " + path + " ...") if (jar.exists) - if (jar.isFile) - IO.delete(jar) - else + if (!jar.isFile) sys.error(path + " exists, but is not a regular file") log.debug(sourcesDebugString(sources)) IO.jar(sources, jar, manifest, time) diff --git a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala index ccb6546ce..e551900f1 100644 --- a/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala +++ b/util-cache/src/main/scala/sbt/util/ActionCacheStore.scala @@ -1,6 +1,6 @@ package sbt.util -import java.io.{ IOException, RandomAccessFile } +import java.io.{ ByteArrayInputStream, IOException } import java.nio.ByteBuffer import java.nio.file.{ Files, @@ -240,23 +240,30 @@ class DiskActionCacheStore(base: Path, converter: FileConverter) extends Abstrac def putBlob(input: InputStream, digest: Digest): Path = val casFile = toCasFile(digest) - IO.transfer(input, casFile.toFile()) - casFile + if isCompleteBlob(casFile, digest) then casFile + else + IO.transfer(input, casFile.toFile()) + casFile def putBlob(input: ByteBuffer, digest: Digest): Path = val casFile = toCasFile(digest) - input.flip() - val file = RandomAccessFile(casFile.toFile(), "rw") - try - file.getChannel().write(input) + if isCompleteBlob(casFile, digest) then casFile + else + input.flip() + val bytes = new Array[Byte](input.remaining()) + input.get(bytes) + IO.transfer(new ByteArrayInputStream(bytes), casFile.toFile()) casFile - finally file.close() + + private def isCompleteBlob(casFile: Path, digest: Digest): Boolean = + try Files.exists(casFile) && Digest.sameDigest(casFile, digest) + catch case _: NoSuchFileException => false private def getBlobs(refs: Seq[HashedVirtualFileRef]): Seq[VirtualFile] = refs.flatMap: r => try val casFile = toCasFile(Digest(r)) - if casFile.toFile().exists then + if isCompleteBlob(casFile, Digest(r)) then r match case p: PathBasedFile => Some(p) case _ => @@ -270,7 +277,7 @@ class DiskActionCacheStore(base: Path, converter: FileConverter) extends Abstrac refs.flatMap: r => try val casFile = toCasFile(Digest(r)) - if casFile.toFile().exists then + if isCompleteBlob(casFile, Digest(r)) then // println(s"syncBlobs: $casFile exists for $r") Some(syncFile(r, casFile, outputDirectory)) else None @@ -384,7 +391,7 @@ class DiskActionCacheStore(base: Path, converter: FileConverter) extends Abstrac refs.flatMap: r => try val casFile = toCasFile(Digest(r)) - if casFile.toFile().exists then Some(r) + if isCompleteBlob(casFile, Digest(r)) then Some(r) else None // Digest(r) can throw NoSuchFileException catch case _: NoSuchFileException => None diff --git a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala index 7d387b71b..79bcc5831 100644 --- a/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala +++ b/util-cache/src/test/scala/sbt/util/ActionCacheTest.scala @@ -1,5 +1,8 @@ package sbt.util +import java.io.{ IOException, InputStream } +import java.nio.charset.StandardCharsets + import sbt.internal.util.CacheEventLog import sbt.internal.util.StringVirtualFile1 import sbt.io.IO @@ -27,6 +30,32 @@ object ActionCacheTest extends BasicTestSuite: test("Disk cache can hold a blob"): withDiskCache(testHoldBlob) + test("Disk cache rejects truncated blobs"): + withDiskCache: cache => + val blob = StringVirtualFile1("a.txt", "hello") + val digest = Digest(blob) + val ref: HashedVirtualFileRef = blob + val casFile = cache.toCasFile(digest) + Files.writeString(casFile, "he", StandardCharsets.UTF_8) + + assert(cache.findBlobs(Seq(ref)).isEmpty) + cache.putBlobs(Seq(blob)) + assert(cache.findBlobs(Seq(ref)) == Seq(ref)) + assert(Files.readString(casFile, StandardCharsets.UTF_8) == "hello") + + test("Disk cache removes staged blobs when writes fail"): + withDiskCache: cache => + val blob = StringVirtualFile1("a.txt", "hello") + val digest = Digest(blob) + val casFile = cache.toCasFile(digest) + try + cache.putBlob(FailingInputStream("hello".getBytes(StandardCharsets.UTF_8), 2), digest) + assert(false, "expected blob write to fail") + catch case _: IOException => () + + assert(!Files.exists(casFile)) + assert(Files.list(cache.casBase).toArray.isEmpty) + def testHoldBlob(cache: ActionCacheStore): Unit = IO.withTemporaryDirectory: tempDir => val in = StringVirtualFile1(s"$tempDir/a.txt", "foo") @@ -35,6 +64,16 @@ object ActionCacheTest extends BasicTestSuite: val actual = cache.syncBlobs(hashRefs, tempDir.toPath()).head assert(actual.getFileName().toString() == "a.txt") + final class FailingInputStream(bytes: Array[Byte], failAt: Int) extends InputStream: + private var index = 0 + override def read(): Int = + if index == failAt then throw IOException("simulated interrupted write") + else if index >= bytes.length then -1 + else + val b = bytes(index) & 0xff + index += 1 + b + test("In-memory cache can hold action value"): withInMemoryCache(testActionCacheBasic)