Make raw LZMA setup and disposal exception-safe

2026-11-15 17:02:03 +00:00 · 2026-10-13 10:29:07 +01:00
parent 38e3222dc2
commit bdb42df4dc
8 changed files with 291 additions and 75 deletions
--- a/docs/API.md
+++ b/docs/API.md
@@ -836,6 +836,8 @@ using (var reader = ReaderFactory.OpenReader(stream))

 `SharpCompressStream` rejects reading and starting recording after disposal. Its pooled ring buffer is released even when disposal of the underlying stream fails.

+`LzmaStream` also rejects reads and writes after disposal. Failed initialization releases internal buffers without closing the supplied stream; successful construction retains the specified `leaveOpen` semantics. Disposal releases internal buffers even if finalization or underlying stream disposal throws.
+
 `SharpCompress.Compressors.LZMA.Utilities.CrcCheckStream` validates its expected CRC on disposal. A mismatch throws `ArchiveOperationException`, but pooled buffers are still released. Repeated disposal is safe, including after failed CRC validation. Once disposed, `CanWrite` is `false`, and `Write` / `WriteAsync` throw `ObjectDisposedException`.

 ## Related Documentation
--- a/src/SharpCompress/Compressors/LZMA/LZ/LzOutWindow.Async.cs
+++ b/src/SharpCompress/Compressors/LZMA/LZ/LzOutWindow.Async.cs
@@ -24,13 +24,20 @@ internal partial class OutWindow : IAsyncDisposable

    public async ValueTask DisposeAsync()
    {
-        await ReleaseStreamAsync().ConfigureAwait(false);
-        if (_buffer is null)
+        try
        {
-            return;
+            await ReleaseStreamAsync().ConfigureAwait(false);
+        }
+        finally
+        {
+            var buffer = _buffer;
+            _buffer = null;
+            _stream = null;
+            if (buffer is not null)
+            {
+                _pool.Return(buffer);
+            }
        }
-        _pool.Return(_buffer);
-        _buffer = null;
    }

    private async ValueTask FlushAsync(CancellationToken cancellationToken = default)
--- a/src/SharpCompress/Compressors/LZMA/LZ/LzOutWindow.cs
+++ b/src/SharpCompress/Compressors/LZMA/LZ/LzOutWindow.cs
@@ -58,13 +58,20 @@ internal partial class OutWindow : IDisposable

    public void Dispose()
    {
-        ReleaseStream();
-        if (_buffer is null)
+        try
        {
-            return;
+            ReleaseStream();
+        }
+        finally
+        {
+            var buffer = _buffer;
+            _buffer = null;
+            _stream = null;
+            if (buffer is not null)
+            {
+                _pool.Return(buffer);
+            }
        }
-        _pool.Return(_buffer);
-        _buffer = null;
    }

    public void Reset()
--- a/src/SharpCompress/Compressors/LZMA/Lzma2EncoderStream.cs
+++ b/src/SharpCompress/Compressors/LZMA/Lzma2EncoderStream.cs
@@ -354,7 +354,7 @@ internal sealed class Lzma2EncoderStream : Stream
        props[3] = (byte)(_dictionarySize >> 16);
        props[4] = (byte)(_dictionarySize >> 24);

-        var decoder = new Decoder();
+        using var decoder = new Decoder();
        decoder.SetDecoderProperties(props);

        using var input = new MemoryStream(compressedData, 0, compressedSize, writable: false);
--- a/src/SharpCompress/Compressors/LZMA/LzmaStream.Async.cs
+++ b/src/SharpCompress/Compressors/LZMA/LzmaStream.Async.cs
@@ -43,24 +43,32 @@ public partial class LzmaStream
            isLzma2,
            leaveOpen
        );
-        if (!isLzma2)
+        try
        {
-            if (presetDictionary != null)
+            if (!isLzma2)
            {
-                await lzma._outWindow.TrainAsync(presetDictionary).ConfigureAwait(false);
-            }
+                if (presetDictionary != null)
+                {
+                    await lzma._outWindow.TrainAsync(presetDictionary).ConfigureAwait(false);
+                }

-            await lzma._rangeDecoder.InitAsync(inputStream).ConfigureAwait(false);
-        }
-        else
-        {
-            if (presetDictionary != null)
-            {
-                await lzma._outWindow.TrainAsync(presetDictionary).ConfigureAwait(false);
-                lzma._needDictReset = false;
+                await lzma._rangeDecoder.InitAsync(inputStream).ConfigureAwait(false);
            }
+            else
+            {
+                if (presetDictionary != null)
+                {
+                    await lzma._outWindow.TrainAsync(presetDictionary).ConfigureAwait(false);
+                    lzma._needDictReset = false;
+                }
+            }
+            return lzma;
+        }
+        catch
+        {
+            await lzma.ReleaseBuffersAsync().ConfigureAwait(false);
+            throw;
        }
-        return lzma;
    }

    /*public static async ValueTask<LzmaStream> CreateAsync(
@@ -175,6 +183,7 @@ public partial class LzmaStream
        CancellationToken cancellationToken
    )
    {
+        ThrowIfDisposed();
        if (_endReached)
        {
            return 0;
@@ -289,6 +298,7 @@ public partial class LzmaStream
        CancellationToken cancellationToken = default
    )
    {
+        ThrowIfDisposed();
        if (_endReached)
        {
            return 0;
@@ -407,6 +417,7 @@ public partial class LzmaStream
        CancellationToken cancellationToken
    )
    {
+        ThrowIfDisposed();
        cancellationToken.ThrowIfCancellationRequested();
        if (_encoder != null)
        {
@@ -422,6 +433,7 @@ public partial class LzmaStream
        CancellationToken cancellationToken = default
    )
    {
+        ThrowIfDisposed();
        cancellationToken.ThrowIfCancellationRequested();
        if (_encoder != null)
        {
@@ -443,30 +455,59 @@ public partial class LzmaStream
            return;
        }
        _isDisposed = true;
-
-        if (_encoder != null)
+        try
        {
-            _position = await _encoder.CodeAsync(null, true).ConfigureAwait(false);
-            _encoder.Dispose();
-        }
-
-        if (!_leaveOpen)
-        {
-            if (_inputStream is IAsyncDisposable asyncDisposableInputStream)
+            if (_encoder != null)
            {
-                await asyncDisposableInputStream.DisposeAsync().ConfigureAwait(false);
-            }
-            else
-            {
-                _inputStream?.Dispose();
+                _position = await _encoder.CodeAsync(null, true).ConfigureAwait(false);
+            }
+        }
+        finally
+        {
+            try
+            {
+                await ReleaseBuffersAsync().ConfigureAwait(false);
+            }
+            finally
+            {
+                if (!_leaveOpen)
+                {
+                    if (_inputStream is IAsyncDisposable asyncDisposableInputStream)
+                    {
+                        await asyncDisposableInputStream.DisposeAsync().ConfigureAwait(false);
+                    }
+                    else
+                    {
+                        _inputStream?.Dispose();
+                    }
+                }
            }
        }
-        await _outWindow.DisposeAsync().ConfigureAwait(false);
-        ReturnAsyncHeaderBuffer();

 #if !LEGACY_DOTNET || NETSTANDARD2_1
        await base.DisposeAsync().ConfigureAwait(false);
 #endif
        GC.SuppressFinalize(this);
    }
+
+    private async ValueTask ReleaseBuffersAsync()
+    {
+        try
+        {
+#pragma warning disable VSTHRD103 // Encoder.Dispose only returns buffers; finalization is separate.
+            _encoder?.Dispose();
+#pragma warning restore VSTHRD103
+        }
+        finally
+        {
+            try
+            {
+                await _outWindow.DisposeAsync().ConfigureAwait(false);
+            }
+            finally
+            {
+                ReturnAsyncHeaderBuffer();
+            }
+        }
+    }
 }
--- a/src/SharpCompress/Compressors/LZMA/LzmaStream.cs
+++ b/src/SharpCompress/Compressors/LZMA/LzmaStream.cs
@@ -54,24 +54,23 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable
        if (!isLzma2)
        {
            _dictionarySize = BinaryPrimitives.ReadInt32LittleEndian(properties.AsSpan(1));
-            _outWindow.Create(_dictionarySize);
-
            _decoder = new Decoder();
            _decoder.SetDecoderProperties(properties);
            Properties = properties;

            _availableBytes = outputSize < 0 ? long.MaxValue : outputSize;
            _rangeDecoderLimit = inputSize;
+            // Validate decoder properties before acquiring the dictionary buffer.
+            _outWindow.Create(_dictionarySize);
        }
        else
        {
            _dictionarySize = 2 | (properties[0] & 1);
            _dictionarySize <<= (properties[0] >> 1) + 11;

-            _outWindow.Create(_dictionarySize);
-
            Properties = new byte[1];
            _availableBytes = 0;
+            _outWindow.Create(_dictionarySize);
        }
    }

@@ -123,24 +122,32 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable
            isLzma2,
            leaveOpen
        );
-        if (!isLzma2)
+        try
        {
-            if (presetDictionary != null)
+            if (!isLzma2)
            {
-                lzma._outWindow.Train(presetDictionary);
-            }
+                if (presetDictionary != null)
+                {
+                    lzma._outWindow.Train(presetDictionary);
+                }

-            lzma._rangeDecoder.Init(inputStream);
-        }
-        else
-        {
-            if (presetDictionary != null)
-            {
-                lzma._outWindow.Train(presetDictionary);
-                lzma._needDictReset = false;
+                lzma._rangeDecoder.Init(inputStream);
            }
+            else
+            {
+                if (presetDictionary != null)
+                {
+                    lzma._outWindow.Train(presetDictionary);
+                    lzma._needDictReset = false;
+                }
+            }
+            return lzma;
+        }
+        catch
+        {
+            lzma.ReleaseBuffers();
+            throw;
        }
-        return lzma;
    }

    private LzmaStream(LzmaEncoderProperties properties, bool isLzma2)
@@ -175,21 +182,28 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable
    )
    {
        var lzma = new LzmaStream(properties, isLzma2);
-
-        lzma._encoder!.SetStreams(null, outputStream, -1, -1);
-
-        if (presetDictionary != null)
+        try
        {
-            lzma._encoder.Train(presetDictionary);
+            lzma._encoder!.SetStreams(null, outputStream, -1, -1);
+
+            if (presetDictionary != null)
+            {
+                lzma._encoder.Train(presetDictionary);
+            }
+            return lzma;
+        }
+        catch
+        {
+            lzma.ReleaseBuffers();
+            throw;
        }
-        return lzma;
    }

-    public override bool CanRead => _encoder == null;
+    public override bool CanRead => !_isDisposed && _encoder == null;

    public override bool CanSeek => false;

-    public override bool CanWrite => _encoder != null;
+    public override bool CanWrite => !_isDisposed && _encoder != null;

    public override void Flush() { }

@@ -202,21 +216,58 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable
            return;
        }
        _isDisposed = true;
-        if (disposing)
+        try
        {
-            if (_encoder != null)
+            if (disposing && _encoder != null)
            {
                _position = _encoder.Code(null, true);
-                _encoder.Dispose();
            }
-            if (!_leaveOpen)
-            {
-                _inputStream?.Dispose();
-            }
-            _outWindow.Dispose();
-            ReturnAsyncHeaderBuffer();
        }
-        base.Dispose(disposing);
+        finally
+        {
+            if (disposing)
+            {
+                try
+                {
+                    ReleaseBuffers();
+                }
+                finally
+                {
+                    if (!_leaveOpen)
+                    {
+                        _inputStream?.Dispose();
+                    }
+                }
+            }
+            base.Dispose(disposing);
+        }
+    }
+
+    private void ReleaseBuffers()
+    {
+        try
+        {
+            _encoder?.Dispose();
+        }
+        finally
+        {
+            try
+            {
+                _outWindow.Dispose();
+            }
+            finally
+            {
+                ReturnAsyncHeaderBuffer();
+            }
+        }
+    }
+
+    private void ThrowIfDisposed()
+    {
+        if (_isDisposed)
+        {
+            throw new ObjectDisposedException(nameof(LzmaStream));
+        }
    }

    private byte[] GetAsyncHeaderBuffer() => _asyncHeaderBuffer ??= ArrayPool<byte>.Shared.Rent(6);
@@ -242,6 +293,7 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable

    public override int Read(byte[] buffer, int offset, int count)
    {
+        ThrowIfDisposed();
        if (_endReached)
        {
            return 0;
@@ -337,6 +389,7 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable

    public override int ReadByte()
    {
+        ThrowIfDisposed();
        if (_endReached)
        {
            return -1;
@@ -506,6 +559,7 @@ public partial class LzmaStream : Stream, IStreamStack, IAsyncDisposable

    public override void Write(byte[] buffer, int offset, int count)
    {
+        ThrowIfDisposed();
        if (_encoder != null)
        {
            _position = _encoder.Code(new MemoryStream(buffer, offset, count), false);
--- a/tests/SharpCompress.Test/Streams/BufferPoolLifetimeTests.cs
+++ b/tests/SharpCompress.Test/Streams/BufferPoolLifetimeTests.cs
@@ -16,6 +16,31 @@ namespace SharpCompress.Test.Streams;

 public class BufferPoolLifetimeTests
 {
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task OutWindow_FailedFlush_ReturnsBuffer(bool asyncDispose)
+    {
+        var pool = new TrackingPool<byte>();
+        using var output = new ThrowingOutputStream();
+        var window = new OutWindow(pool);
+        window.Create(32);
+        window.Init(output);
+        window.SetLimit(1);
+        window.PutByte(42);
+        if (asyncDispose)
+        {
+            await Assert.ThrowsAsync<IOException>(async () => await window.DisposeAsync());
+        }
+        else
+        {
+            Assert.Throws<IOException>(() => window.Dispose());
+        }
+        window.Dispose();
+        Assert.Equal(1, pool.ReturnCount);
+        Assert.Empty(pool.Active);
+    }
+
    [Theory]
    [InlineData(false)]
    [InlineData(true)]
--- a/tests/SharpCompress.Test/Streams/LzmaLifetimeTests.cs
+++ b/tests/SharpCompress.Test/Streams/LzmaLifetimeTests.cs
@@ -0,0 +1,80 @@
+using System;
+using System.IO;
+using System.Reflection;
+using System.Threading;
+using System.Threading.Tasks;
+using SharpCompress.Compressors.LZMA;
+using Xunit;
+
+namespace SharpCompress.Test.Streams;
+
+public class LzmaLifetimeTests
+{
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task FailedFinalizationReleasesEncoderBuffers(bool asyncDispose)
+    {
+        using var output = new ThrowingOutput();
+        var stream = LzmaStream.Create(new LzmaEncoderProperties(), false, output);
+        var encoder = GetField(stream, "_encoder")!;
+        if (asyncDispose)
+        {
+            await Assert.ThrowsAsync<IOException>(async () => await stream.DisposeAsync());
+        }
+        else
+        {
+            Assert.Throws<IOException>(() => stream.Dispose());
+        }
+        Assert.Null(GetField(encoder, "_matchFinder"));
+        stream.Dispose();
+        Assert.False(stream.CanWrite);
+        Assert.Throws<ObjectDisposedException>(() => stream.Write(new byte[1], 0, 1));
+        await Assert.ThrowsAsync<ObjectDisposedException>(() =>
+            stream.WriteAsync(new byte[1], 0, 1, CancellationToken.None)
+        );
+    }
+
+    [Fact]
+    public async Task DisposedLzma2ReaderCannotRentHeaderBuffer()
+    {
+        using var input = new MemoryStream(new byte[] { 0 });
+        var stream = LzmaStream.Create(new byte[] { 0 }, input, leaveOpen: true);
+        stream.Dispose();
+        await Assert.ThrowsAsync<ObjectDisposedException>(() =>
+            stream.ReadAsync(new byte[1], 0, 1, CancellationToken.None)
+        );
+        Assert.Null(GetField(stream, "_asyncHeaderBuffer"));
+        Assert.False(stream.CanRead);
+        Assert.True(input.CanRead);
+    }
+
+    [Fact]
+    public void InvalidPropertiesLeaveInputOpen()
+    {
+        using var input = new MemoryStream();
+        Assert.Throws<InvalidParamException>(() =>
+            LzmaStream.Create(new byte[] { 225, 0, 0, 16, 0 }, input)
+        );
+        Assert.True(input.CanRead);
+    }
+
+    private static object? GetField(object instance, string name) =>
+        instance
+            .GetType()
+            .GetField(name, BindingFlags.Instance | BindingFlags.NonPublic)!
+            .GetValue(instance);
+
+    private sealed class ThrowingOutput : MemoryStream
+    {
+        public override void WriteByte(byte value) =>
+            throw new IOException("Injected output failure");
+
+        public override Task WriteAsync(
+            byte[] buffer,
+            int offset,
+            int count,
+            CancellationToken cancellationToken
+        ) => Task.FromException(new IOException("Injected output failure"));
+    }
+}