From 44e4b1804ec2c64f865eca8547c5c81879a82282 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 26 Jan 2026 09:41:13 +0000 Subject: [PATCH 1/7] add check to see if we need to seek before hand --- src/SharpCompress/IO/BufferedSubStream.cs | 16 ++++++++++++++-- src/SharpCompress/IO/SharpCompressStream.cs | 1 - 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index f9216216e..7e71207ba 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -64,7 +64,14 @@ private void RefillCache() _cacheLength = 0; return; } - Stream.Position = origin; + + // 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) + { + Stream.Position = origin; + } + _cacheLength = Stream.Read(_cache, 0, count); origin += _cacheLength; BytesLeftToRead -= _cacheLength; @@ -79,7 +86,12 @@ private async ValueTask RefillCacheAsync(CancellationToken cancellationToken) _cacheLength = 0; return; } - Stream.Position = origin; + // 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) + { + Stream.Position = origin; + } _cacheLength = await Stream .ReadAsync(_cache, 0, count, cancellationToken) .ConfigureAwait(false); diff --git a/src/SharpCompress/IO/SharpCompressStream.cs b/src/SharpCompress/IO/SharpCompressStream.cs index 8261505f7..101f7b163 100644 --- a/src/SharpCompress/IO/SharpCompressStream.cs +++ b/src/SharpCompress/IO/SharpCompressStream.cs @@ -257,7 +257,6 @@ public override long Seek(long offset, SeekOrigin origin) ValidateBufferState(); } - long orig = _internalPosition; long targetPos; // Calculate the absolute target position based on origin switch (origin) From a82fda98d789a708f8429fbc6b8acbebd8ea3823 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 26 Jan 2026 11:45:25 +0000 Subject: [PATCH 2/7] more testing and add pooling to cache --- src/SharpCompress/IO/BufferedSubStream.cs | 8 ++++++-- .../Streams/SharpCompressStreamTest.cs | 10 +++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index 7e71207ba..2a5c917b7 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -28,13 +29,16 @@ protected override void Dispose(bool disposing) #if DEBUG_STREAMS this.DebugDispose(typeof(BufferedSubStream)); #endif - if (disposing) { } + if (disposing) + { + ArrayPool.Shared.Return(_cache); + } base.Dispose(disposing); } private int _cacheOffset; private int _cacheLength; - private readonly byte[] _cache = new byte[32 << 10]; + private readonly byte[] _cache = ArrayPool.Shared.Rent(81920); private long origin; private long BytesLeftToRead { get; set; } diff --git a/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs b/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs index 1c4a8b571..440512265 100644 --- a/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs +++ b/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs @@ -5,6 +5,7 @@ using System.Text; using SharpCompress.Compressors.LZMA; using SharpCompress.IO; +using SharpCompress.Test.Mocks; using Xunit; namespace SharpCompress.Test.Streams; @@ -64,7 +65,14 @@ public void BufferReadAndSeekTest() { createData(ms); - using (SharpCompressStream scs = new SharpCompressStream(ms, true, false, 0x10000)) + using ( + SharpCompressStream scs = new SharpCompressStream( + new ForwardOnlyStream(ms), + true, + false, + 0x10000 + ) + ) { IStreamStack stack = (IStreamStack)scs; From ddf37e82c2798ed5052bd8ea5574f3319d0617fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 26 Jan 2026 12:06:38 +0000 Subject: [PATCH 3/7] Initial plan From 8c95f863cb8e39e1a52eebed187e1ba4f69c4042 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Mon, 26 Jan 2026 12:06:57 +0000 Subject: [PATCH 4/7] do CanSeek first --- src/SharpCompress/IO/BufferedSubStream.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index 2a5c917b7..a504b7efc 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -71,7 +71,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; } @@ -92,7 +92,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; } From 875c2d76943dd5ca2d14c13d2b787fe1edb104b6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 26 Jan 2026 12:10:19 +0000 Subject: [PATCH 5/7] Fix BufferedSubStream double-dispose issue with ArrayPool Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/IO/BufferedSubStream.cs | 24 ++++++++++++------- .../Streams/SharpCompressStreamTest.cs | 21 ++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index 2a5c917b7..9ee22c059 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -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.Shared.Return(_cache); + _cache = null; } base.Dispose(disposing); } private int _cacheOffset; private int _cacheLength; - private readonly byte[] _cache = ArrayPool.Shared.Rent(81920); + private byte[]? _cache = ArrayPool.Shared.Rent(81920); private long origin; + private bool _isDisposed; private long BytesLeftToRead { get; set; } @@ -61,7 +69,7 @@ public override long Position private void RefillCache() { - var count = (int)Math.Min(BytesLeftToRead, _cache.Length); + var count = (int)Math.Min(BytesLeftToRead, _cache!.Length); _cacheOffset = 0; if (count == 0) { @@ -83,7 +91,7 @@ private void RefillCache() private async ValueTask RefillCacheAsync(CancellationToken cancellationToken) { - var count = (int)Math.Min(BytesLeftToRead, _cache.Length); + var count = (int)Math.Min(BytesLeftToRead, _cache!.Length); _cacheOffset = 0; if (count == 0) { @@ -118,7 +126,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; } @@ -136,7 +144,7 @@ public override int ReadByte() } } - return _cache[_cacheOffset++]; + return _cache![_cacheOffset++]; } public override async Task ReadAsync( @@ -159,7 +167,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; } @@ -186,7 +194,7 @@ public override async ValueTask ReadAsync( } count = Math.Min(count, _cacheLength - _cacheOffset); - _cache.AsSpan(_cacheOffset, count).CopyTo(buffer.Span); + _cache!.AsSpan(_cacheOffset, count).CopyTo(buffer.Span); _cacheOffset += count; } diff --git a/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs b/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs index 440512265..f74a6670e 100644 --- a/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs +++ b/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs @@ -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); + } } From a0af0604d11ab1144e4be00ec73033e66013ab16 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 26 Jan 2026 12:11:16 +0000 Subject: [PATCH 6/7] Add disposal checks to RefillCache methods Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/IO/BufferedSubStream.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index 9ee22c059..d2302b3a9 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -69,6 +69,11 @@ public override long Position private void RefillCache() { + if (_isDisposed) + { + throw new ObjectDisposedException(nameof(BufferedSubStream)); + } + var count = (int)Math.Min(BytesLeftToRead, _cache!.Length); _cacheOffset = 0; if (count == 0) @@ -91,6 +96,11 @@ private void RefillCache() private async ValueTask RefillCacheAsync(CancellationToken cancellationToken) { + if (_isDisposed) + { + throw new ObjectDisposedException(nameof(BufferedSubStream)); + } + var count = (int)Math.Min(BytesLeftToRead, _cache!.Length); _cacheOffset = 0; if (count == 0) From 56d3091688a459229a64735b1c580e909e5ece2d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 26 Jan 2026 12:12:08 +0000 Subject: [PATCH 7/7] Fix condition order to check CanSeek before Position Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/IO/BufferedSubStream.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SharpCompress/IO/BufferedSubStream.cs b/src/SharpCompress/IO/BufferedSubStream.cs index d2302b3a9..6bb3fa1d0 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -84,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; } @@ -110,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; }