Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions src/SharpCompress/IO/BufferedSubStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,25 @@ protected override void Dispose(bool disposing)
#if DEBUG_STREAMS
this.DebugDispose(typeof(BufferedSubStream));
#endif
if (disposing)
if (_isDisposed)
{
return;
}
_isDisposed = true;

if (disposing && _cache is not null)
{
ArrayPool<byte>.Shared.Return(_cache);
_cache = null;
}
base.Dispose(disposing);
}

private int _cacheOffset;
private int _cacheLength;
private readonly byte[] _cache = ArrayPool<byte>.Shared.Rent(81920);
private byte[]? _cache = ArrayPool<byte>.Shared.Rent(81920);
private long origin;
private bool _isDisposed;

private long BytesLeftToRead { get; set; }

Expand All @@ -61,7 +69,12 @@ public override long Position

private void RefillCache()
{
var count = (int)Math.Min(BytesLeftToRead, _cache.Length);
if (_isDisposed)
{
throw new ObjectDisposedException(nameof(BufferedSubStream));
}

var count = (int)Math.Min(BytesLeftToRead, _cache!.Length);
_cacheOffset = 0;
if (count == 0)
{
Expand All @@ -71,7 +84,7 @@ private void RefillCache()

// Only seek if we're not already at the correct position
// This avoids expensive seek operations when reading sequentially
if (Stream.Position != origin && Stream.CanSeek)
if (Stream.CanSeek && Stream.Position != origin)
{
Stream.Position = origin;
}
Expand All @@ -83,7 +96,12 @@ private void RefillCache()

private async ValueTask RefillCacheAsync(CancellationToken cancellationToken)
{
var count = (int)Math.Min(BytesLeftToRead, _cache.Length);
if (_isDisposed)
{
throw new ObjectDisposedException(nameof(BufferedSubStream));
}

var count = (int)Math.Min(BytesLeftToRead, _cache!.Length);
_cacheOffset = 0;
if (count == 0)
{
Expand All @@ -92,7 +110,7 @@ private async ValueTask RefillCacheAsync(CancellationToken cancellationToken)
}
// Only seek if we're not already at the correct position
// This avoids expensive seek operations when reading sequentially
if (Stream.Position != origin && Stream.CanSeek)
if (Stream.CanSeek && Stream.Position != origin)
{
Stream.Position = origin;
}
Expand All @@ -118,7 +136,7 @@ public override int Read(byte[] buffer, int offset, int count)
}

count = Math.Min(count, _cacheLength - _cacheOffset);
Buffer.BlockCopy(_cache, _cacheOffset, buffer, offset, count);
Buffer.BlockCopy(_cache!, _cacheOffset, buffer, offset, count);
_cacheOffset += count;
}

Expand All @@ -136,7 +154,7 @@ public override int ReadByte()
}
}

return _cache[_cacheOffset++];
return _cache![_cacheOffset++];
}

public override async Task<int> ReadAsync(
Expand All @@ -159,7 +177,7 @@ CancellationToken cancellationToken
}

count = Math.Min(count, _cacheLength - _cacheOffset);
Buffer.BlockCopy(_cache, _cacheOffset, buffer, offset, count);
Buffer.BlockCopy(_cache!, _cacheOffset, buffer, offset, count);
_cacheOffset += count;
}

Expand All @@ -186,7 +204,7 @@ public override async ValueTask<int> ReadAsync(
}

count = Math.Min(count, _cacheLength - _cacheOffset);
_cache.AsSpan(_cacheOffset, count).CopyTo(buffer.Span);
_cache!.AsSpan(_cacheOffset, count).CopyTo(buffer.Span);
_cacheOffset += count;
}

Expand Down
21 changes: 21 additions & 0 deletions tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,25 @@ public void BufferReadAndSeekTest()
}
}
}

[Fact]
public void BufferedSubStream_DoubleDispose_DoesNotCorruptArrayPool()
{
// This test verifies that calling Dispose multiple times on BufferedSubStream
// doesn't return the same array to the pool twice, which would cause pool corruption
byte[] data = new byte[0x10000];
using (MemoryStream ms = new MemoryStream(data))
{
var stream = new BufferedSubStream(ms, 0, data.Length);

// First disposal
stream.Dispose();

// Second disposal should not throw or corrupt the pool
stream.Dispose();
}

// If we got here without an exception, the test passed
Assert.True(true);
}
}