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] 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 2a5c917b..9ee22c05 100755 --- a/src/SharpCompress/IO/BufferedSubStream.cs +++ b/src/SharpCompress/IO/BufferedSubStream.cs @@ -29,17 +29,25 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack #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 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack 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 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack 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 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack } 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 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack } } - return _cache[_cacheOffset++]; + return _cache![_cacheOffset++]; } public override async Task ReadAsync( @@ -159,7 +167,7 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack } 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 @@ internal class BufferedSubStream : SharpCompressStream, IStreamStack } 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 44051226..f74a6670 100644 --- a/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs +++ b/tests/SharpCompress.Test/Streams/SharpCompressStreamTest.cs @@ -97,4 +97,25 @@ public class SharpCompressStreamTests } } } + + [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); + } }