starred/pkl
1
0
Fork 0
mirror of https://github.com/apple/pkl.git synced 2026-05-18 04:46:59 +02:00
Code Issues 198 Packages Projects Releases 10 Wiki Activity

Fix race condition when concurrently downloading packages (#584)

Browse Source 
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 Dan Chao
parent 57df7995fd
commit 7c3787396e
2 changed files with 16 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkl-core/src/main/java/org/pkl/core/packages/PackageResolvers.java
+10 -5
View File
@@ -455,10 +455,9 @@ final class PackageResolvers {
private byte[] downloadUriToPathAndComputeChecksum(URI downloadUri, Path path) private byte[] downloadUriToPathAndComputeChecksum(URI downloadUri, Path path)
throws IOException, SecurityManagerException { throws IOException, SecurityManagerException {
Files.createDirectories(path.getParent());
var inputStream = openExternalUri(downloadUri); var inputStream = openExternalUri(downloadUri);
try (var digestInputStream = newDigestInputStream(inputStream)) { try (var digestInputStream = newDigestInputStream(inputStream)) {
Files.copy(digestInputStream, path); Files.copy(digestInputStream, path, StandardCopyOption.REPLACE_EXISTING);
return digestInputStream.getMessageDigest().digest(); return digestInputStream.getMessageDigest().digest();
} }
} }
@@ -472,7 +471,7 @@ final class PackageResolvers {
} }
try (var in = inputStream) { try (var in = inputStream) {
Files.createDirectories(path.getParent()); Files.createDirectories(path.getParent());
Files.copy(in, path); Files.copy(in, path, StandardCopyOption.REPLACE_EXISTING);
if (checksums != null) { if (checksums != null) {
var digestInputStream = (DigestInputStream) inputStream; var digestInputStream = (DigestInputStream) inputStream;
var checksumBytes = digestInputStream.getMessageDigest().digest(); var checksumBytes = digestInputStream.getMessageDigest().digest();
@@ -490,7 +489,10 @@ final class PackageResolvers {
if (Files.exists(cachePath)) { if (Files.exists(cachePath)) {
return cachePath; return cachePath;
} }
var tmpPath = tmpDir.resolve(metadataRelativePath); Files.createDirectories(tmpDir);
var tmpPath =
Files.createTempFile(
tmpDir, IoUtils.encodePath(packageUri.toString().replaceAll("/", "-")), ".json");
try { try {
downloadMetadata(packageUri, requestUri, tmpPath, checksums); downloadMetadata(packageUri, requestUri, tmpPath, checksums);
Files.createDirectories(cachePath.getParent()); Files.createDirectories(cachePath.getParent());
@@ -539,7 +541,10 @@ final class PackageResolvers {
if (Files.exists(cachePath)) { if (Files.exists(cachePath)) {
return cachePath; return cachePath;
} }
var tmpPath = tmpDir.resolve(relativePath); Files.createDirectories(tmpDir);
var tmpPath =
Files.createTempFile(
tmpDir, IoUtils.encodePath(packageUri.toString().replaceAll("/", "-")), ".zip");
try { try {
var checksumBytes = var checksumBytes =
downloadUriToPathAndComputeChecksum(dependencyMetadata.getPackageZipUrl(), tmpPath); downloadUriToPathAndComputeChecksum(dependencyMetadata.getPackageZipUrl(), tmpPath);
pkl-core/src/test/kotlin/org/pkl/core/packages/PackageResolversTest.kt
+6 -5
View File
@@ -2,10 +2,9 @@ package org.pkl.core.packages
import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatCode import org.assertj.core.api.Assertions.assertThatCode
import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.*
import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.parallel.Execution
import org.junit.jupiter.api.Test import org.junit.jupiter.api.parallel.ExecutionMode
import org.junit.jupiter.api.assertThrows
import org.pkl.commons.deleteRecursively import org.pkl.commons.deleteRecursively
import org.pkl.commons.readString import org.pkl.commons.readString
import org.pkl.commons.test.FileTestUtils import org.pkl.commons.test.FileTestUtils
@@ -44,7 +43,9 @@ class PackageResolversTest {
} }
} }
@Test // execute test 3 times to check concurrent writes
@RepeatedTest(3)
@Execution(ExecutionMode.CONCURRENT)
fun `get module bytes`() { fun `get module bytes`() {
val expectedBirdModule = packageRoot.resolve("birds@0.5.0/package/Bird.pkl").readString(StandardCharsets.UTF_8) val expectedBirdModule = packageRoot.resolve("birds@0.5.0/package/Bird.pkl").readString(StandardCharsets.UTF_8)
val assetUri = PackageAssetUri("package://localhost:0/birds@0.5.0#/Bird.pkl") val assetUri = PackageAssetUri("package://localhost:0/birds@0.5.0#/Bird.pkl")