Skip to content

Commit b9a672f

Browse files
feat: handle parallel chunk upload assembly race in Local and S3 devices
1 parent 52d1f89 commit b9a672f

4 files changed

Lines changed: 47 additions & 1 deletion

File tree

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
.phpunit.result.cache
44
tests/chunk.php
55
.idea/
6-
.env
6+
.env
7+
.DS_Store

src/Storage/Device/Local.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,10 @@ private function countChunks(string $tmp, string $path): int
164164

165165
private function joinChunks(string $path, int $chunks): void
166166
{
167+
if (\file_exists($path)) {
168+
return;
169+
}
170+
167171
$tmp = \dirname($path).DIRECTORY_SEPARATOR.'tmp_'.\basename($path);
168172
$tmpAssemble = \dirname($path).DIRECTORY_SEPARATOR.'tmp_assemble_'.\basename($path);
169173

@@ -195,6 +199,11 @@ private function joinChunks(string $path, int $chunks): void
195199
\fclose($dest);
196200

197201
if (! \rename($tmpAssemble, $path)) {
202+
if (\file_exists($path)) {
203+
\unlink($tmpAssemble);
204+
205+
return;
206+
}
198207
\unlink($tmpAssemble);
199208
throw new Exception('Failed to finalize assembled file '.$path);
200209
}

src/Storage/Device/S3.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,9 @@ public function uploadData(string $data, string $path, string $contentType, int
203203
}
204204
$metadata['parts'][$chunk] = $etag;
205205
if ($metadata['chunks'] == $chunks) {
206+
if ($this->exists($path)) {
207+
return $metadata['chunks'];
208+
}
206209
$this->completeMultipartUpload($path, $uploadId, $metadata['parts']);
207210
}
208211

tests/Storage/Device/LocalTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,4 +577,37 @@ public function testOutOfOrderUploadWithRetry(): void
577577

578578
$storage->delete($storage->getRoot(), true);
579579
}
580+
581+
public function testParallelChunkUpload(): void
582+
{
583+
$storage = $this->makeJoinTestStorage();
584+
$dest = $storage->getRoot().DIRECTORY_SEPARATOR.'parallel.dat';
585+
586+
// Upload chunk 1 (creates temp directory)
587+
$storage->uploadData('AAAA', $dest, 'application/octet-stream', 1, 2);
588+
589+
// Upload chunk 2 (assembles the file)
590+
$storage->uploadData('BBBB', $dest, 'application/octet-stream', 2, 2);
591+
592+
// Verify file exists and is correct
593+
$this->assertTrue(\file_exists($dest));
594+
$this->assertSame('AAAABBBB', \file_get_contents($dest));
595+
596+
// Simulate the race where another request already assembled the file
597+
// by calling joinChunks directly when the file already exists
598+
$reflection = new \ReflectionClass($storage);
599+
$method = $reflection->getMethod('joinChunks');
600+
$method->setAccessible(true);
601+
602+
try {
603+
$method->invoke($storage, $dest, 2);
604+
} catch (\Exception $e) {
605+
$this->fail('Duplicate assembly should not throw: '.$e->getMessage());
606+
}
607+
608+
$this->assertTrue(\file_exists($dest), 'File should still exist after duplicate assembly attempt');
609+
$this->assertSame('AAAABBBB', \file_get_contents($dest), 'File content must not be corrupted');
610+
611+
$storage->delete($storage->getRoot(), true);
612+
}
580613
}

0 commit comments

Comments
 (0)