Skip to content

Commit 877e806

Browse files
JusterZhuclaude
andcommitted
fix: make BsdiffDiffer thread-safe for concurrent pipeline use
Remove per-call mutable instance fields (_oldfilePath, _newfilePath, _patchPath) that caused race conditions when a single BsdiffDiffer instance was shared across parallel DiffPipeline tasks. The Task.Run lambdas now capture parameters directly via closure, making each invocation independently thread-safe. Also add missing GeneralUpdate.Core project reference to DifferentialTest, and add comprehensive single/multi-file differential integration tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 9d26981 commit 877e806

3 files changed

Lines changed: 864 additions & 25 deletions

File tree

src/c#/DifferentialTest/DifferentialTest.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
<ItemGroup>
1313
<ProjectReference Include="..\GeneralUpdate.Differential\GeneralUpdate.Differential.csproj" />
14+
<ProjectReference Include="..\GeneralUpdate.Core\GeneralUpdate.Core.csproj" />
1415
</ItemGroup>
1516

1617
<ItemGroup>

src/c#/GeneralUpdate.Differential/Differ/BsdiffDiffer.cs

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ namespace GeneralUpdate.Differential.Differ
1515
/// Default: <see cref="BZip2CompressionProvider"/> (backward compatible).
1616
/// Use <see cref="DeflateCompressionProvider"/> (0x01) for faster decompression.
1717
///
18-
/// Thread-safety: this class stores per-call state in instance fields.
19-
/// A single instance MUST NOT be used concurrently by multiple callers.
20-
/// Create separate instances for concurrent operations.
18+
/// Thread-safety: this class is stateless beyond the compression provider.
19+
/// A single instance is safe for concurrent calls.
2120
/// </remarks>
2221
public class BsdiffDiffer : IBinaryDiffer
2322
{
@@ -31,8 +30,6 @@ public class BsdiffDiffer : IBinaryDiffer
3130
private const int ExtendedHeaderSize = 33;
3231

3332
private readonly ICompressionProvider _compressionProvider;
34-
// Per-call mutable state. NOT thread-safe for concurrent calls on the same instance.
35-
private string _oldfilePath, _newfilePath, _patchPath;
3633

3734
#endregion Private Members
3835

@@ -84,15 +81,12 @@ public async Task Clean(string oldfilePath, string newfilePath, string patchPath
8481
{
8582
await Task.Run(() =>
8683
{
87-
_oldfilePath = oldfilePath;
88-
_newfilePath = newfilePath;
89-
_patchPath = patchPath;
90-
ValidationParameters();
84+
ValidationParameters(oldfilePath, newfilePath, patchPath);
9185

9286
using (var output = new FileStream(patchPath, FileMode.Create))
9387
{
94-
var oldBytes = File.ReadAllBytes(_oldfilePath);
95-
var newBytes = File.ReadAllBytes(_newfilePath);
88+
var oldBytes = File.ReadAllBytes(oldfilePath);
89+
var newBytes = File.ReadAllBytes(newfilePath);
9690

9791
// Header layout:
9892
// 0 8 "BSDIFF40" (magic)
@@ -270,13 +264,10 @@ public async Task Dirty(string oldfilePath, string newfilePath, string patchPath
270264
{
271265
await Task.Run(() =>
272266
{
273-
_oldfilePath = oldfilePath;
274-
_newfilePath = newfilePath;
275-
_patchPath = patchPath;
276-
ValidationParameters();
267+
ValidationParameters(oldfilePath, newfilePath, patchPath);
277268

278-
using (var input = new FileStream(_oldfilePath, FileMode.Open, FileAccess.Read, FileShare.Read))
279-
using (var output = new FileStream(_newfilePath, FileMode.Create))
269+
using (var input = new FileStream(oldfilePath, FileMode.Open, FileAccess.Read, FileShare.Read))
270+
using (var output = new FileStream(newfilePath, FileMode.Create))
280271
{
281272
long controlLength, diffLength, newSize;
282273
byte formatVersion;
@@ -432,7 +423,6 @@ await Task.Run(() =>
432423
}
433424
}
434425

435-
// The result is left at _newfilePath.
436426
// Atomic replacement (if needed) is handled by the caller
437427
// (e.g. DefaultDirtyStrategy.ApplyPatch).
438428
});
@@ -450,14 +440,14 @@ private static FileStream OpenPatchStream(string patchPath)
450440
return new FileStream(patchPath, FileMode.Open, FileAccess.Read, FileShare.Read);
451441
}
452442

453-
private void ValidationParameters()
443+
private static void ValidationParameters(string oldfilePath, string newfilePath, string patchPath)
454444
{
455-
if (string.IsNullOrWhiteSpace(_oldfilePath))
456-
throw new ArgumentNullException(nameof(_oldfilePath), "Old file path must not be empty.");
457-
if (string.IsNullOrWhiteSpace(_newfilePath))
458-
throw new ArgumentNullException(nameof(_newfilePath), "New file path must not be empty.");
459-
if (string.IsNullOrWhiteSpace(_patchPath))
460-
throw new ArgumentNullException(nameof(_patchPath), "Patch path must not be empty.");
445+
if (string.IsNullOrWhiteSpace(oldfilePath))
446+
throw new ArgumentNullException(nameof(oldfilePath), "Old file path must not be empty.");
447+
if (string.IsNullOrWhiteSpace(newfilePath))
448+
throw new ArgumentNullException(nameof(newfilePath), "New file path must not be empty.");
449+
if (string.IsNullOrWhiteSpace(patchPath))
450+
throw new ArgumentNullException(nameof(patchPath), "Patch path must not be empty.");
461451
}
462452

463453
private static int CompareBytes(byte[] left, int leftOffset, byte[] right, int rightOffset)

0 commit comments

Comments
 (0)