Fix race condition when concurrently downloading packages (#584)

This fixes a possible race condition where multiple processes download
the same package into the same temp dir.
This commit is contained in:
Daniel Chao
2024-07-18 09:11:25 -07:00
committed by GitHub
parent 24cc95abcc
commit 176ede0002
2 changed files with 16 additions and 10 deletions

View File

@@ -455,10 +455,9 @@ final class PackageResolvers {
private byte[] downloadUriToPathAndComputeChecksum(URI downloadUri, Path path)
throws IOException, SecurityManagerException {
Files.createDirectories(path.getParent());
var inputStream = openExternalUri(downloadUri);
try (var digestInputStream = newDigestInputStream(inputStream)) {
Files.copy(digestInputStream, path);
Files.copy(digestInputStream, path, StandardCopyOption.REPLACE_EXISTING);
return digestInputStream.getMessageDigest().digest();
}
}
@@ -472,7 +471,7 @@ final class PackageResolvers {
}
try (var in = inputStream) {
Files.createDirectories(path.getParent());
Files.copy(in, path);
Files.copy(in, path, StandardCopyOption.REPLACE_EXISTING);
if (checksums != null) {
var digestInputStream = (DigestInputStream) inputStream;
var checksumBytes = digestInputStream.getMessageDigest().digest();
@@ -490,7 +489,10 @@ final class PackageResolvers {
if (Files.exists(cachePath)) {
return cachePath;
}
var tmpPath = tmpDir.resolve(metadataRelativePath);
Files.createDirectories(tmpDir);
var tmpPath =
Files.createTempFile(
tmpDir, IoUtils.encodePath(packageUri.toString().replaceAll("/", "-")), ".json");
try {
downloadMetadata(packageUri, requestUri, tmpPath, checksums);
Files.createDirectories(cachePath.getParent());
@@ -539,7 +541,10 @@ final class PackageResolvers {
if (Files.exists(cachePath)) {
return cachePath;
}
var tmpPath = tmpDir.resolve(relativePath);
Files.createDirectories(tmpDir);
var tmpPath =
Files.createTempFile(
tmpDir, IoUtils.encodePath(packageUri.toString().replaceAll("/", "-")), ".zip");
try {
var checksumBytes =
downloadUriToPathAndComputeChecksum(dependencyMetadata.getPackageZipUrl(), tmpPath);

View File

@@ -22,10 +22,9 @@ import kotlin.io.path.exists
import kotlin.io.path.readBytes
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatCode
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import org.junit.jupiter.api.*
import org.junit.jupiter.api.parallel.Execution
import org.junit.jupiter.api.parallel.ExecutionMode
import org.pkl.commons.deleteRecursively
import org.pkl.commons.readString
import org.pkl.commons.test.FileTestUtils
@@ -60,7 +59,9 @@ class PackageResolversTest {
}
}
@Test
// execute test 3 times to check concurrent writes
@RepeatedTest(3)
@Execution(ExecutionMode.CONCURRENT)
fun `get module bytes`() {
val expectedBirdModule =
packageRoot.resolve("birds@0.5.0/package/Bird.pkl").readString(StandardCharsets.UTF_8)