Skip to content

Commit 0bdfe24

Browse files
authored
Merge pull request #450 from GeneralLibrary/fix/bsdiff-thread-safety
fix: make BsdiffDiffer thread-safe for concurrent pipeline use
2 parents 9d26981 + b99e554 commit 0bdfe24

3 files changed

Lines changed: 867 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: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ 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 safe for concurrent calls provided the
19+
/// configured <see cref="ICompressionProvider"/> is thread-safe.
20+
/// The built-in providers (<see cref="BZip2CompressionProvider"/>,
21+
/// <see cref="DeflateCompressionProvider"/>, <see cref="BrotliCompressionProvider"/>)
22+
/// are all safe for concurrent use.
2123
/// </remarks>
2224
public class BsdiffDiffer : IBinaryDiffer
2325
{
@@ -31,8 +33,6 @@ public class BsdiffDiffer : IBinaryDiffer
3133
private const int ExtendedHeaderSize = 33;
3234

3335
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;
3636

3737
#endregion Private Members
3838

@@ -84,15 +84,12 @@ public async Task Clean(string oldfilePath, string newfilePath, string patchPath
8484
{
8585
await Task.Run(() =>
8686
{
87-
_oldfilePath = oldfilePath;
88-
_newfilePath = newfilePath;
89-
_patchPath = patchPath;
90-
ValidationParameters();
87+
ValidationParameters(oldfilePath, newfilePath, patchPath);
9188

9289
using (var output = new FileStream(patchPath, FileMode.Create))
9390
{
94-
var oldBytes = File.ReadAllBytes(_oldfilePath);
95-
var newBytes = File.ReadAllBytes(_newfilePath);
91+
var oldBytes = File.ReadAllBytes(oldfilePath);
92+
var newBytes = File.ReadAllBytes(newfilePath);
9693

9794
// Header layout:
9895
// 0 8 "BSDIFF40" (magic)
@@ -270,13 +267,10 @@ public async Task Dirty(string oldfilePath, string newfilePath, string patchPath
270267
{
271268
await Task.Run(() =>
272269
{
273-
_oldfilePath = oldfilePath;
274-
_newfilePath = newfilePath;
275-
_patchPath = patchPath;
276-
ValidationParameters();
270+
ValidationParameters(oldfilePath, newfilePath, patchPath);
277271

278-
using (var input = new FileStream(_oldfilePath, FileMode.Open, FileAccess.Read, FileShare.Read))
279-
using (var output = new FileStream(_newfilePath, FileMode.Create))
272+
using (var input = new FileStream(oldfilePath, FileMode.Open, FileAccess.Read, FileShare.Read))
273+
using (var output = new FileStream(newfilePath, FileMode.Create))
280274
{
281275
long controlLength, diffLength, newSize;
282276
byte formatVersion;
@@ -432,7 +426,6 @@ await Task.Run(() =>
432426
}
433427
}
434428

435-
// The result is left at _newfilePath.
436429
// Atomic replacement (if needed) is handled by the caller
437430
// (e.g. DefaultDirtyStrategy.ApplyPatch).
438431
});
@@ -450,14 +443,14 @@ private static FileStream OpenPatchStream(string patchPath)
450443
return new FileStream(patchPath, FileMode.Open, FileAccess.Read, FileShare.Read);
451444
}
452445

453-
private void ValidationParameters()
446+
private static void ValidationParameters(string oldfilePath, string newfilePath, string patchPath)
454447
{
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.");
448+
if (string.IsNullOrWhiteSpace(oldfilePath))
449+
throw new ArgumentNullException(nameof(oldfilePath), "Old file path must not be empty.");
450+
if (string.IsNullOrWhiteSpace(newfilePath))
451+
throw new ArgumentNullException(nameof(newfilePath), "New file path must not be empty.");
452+
if (string.IsNullOrWhiteSpace(patchPath))
453+
throw new ArgumentNullException(nameof(patchPath), "Patch path must not be empty.");
461454
}
462455

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

0 commit comments

Comments
 (0)