diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 86fef715d..57f60f2e7 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -80,8 +80,22 @@ namespace SixLabors.ImageSharp /// Configuration IConfigurable.Configuration => this.Configuration; + /// + /// Gets a value indicating whether the image instance is disposed. + /// + public bool IsDisposed { get; private set; } + /// - public abstract void Dispose(); + public void Dispose() + { + if (this.IsDisposed) + { + return; + } + + this.IsDisposed = true; + this.DisposeImpl(); + } /// /// Saves the image to the given stream using the given image encoder. @@ -93,6 +107,7 @@ namespace SixLabors.ImageSharp { Guard.NotNull(stream, nameof(stream)); Guard.NotNull(encoder, nameof(encoder)); + this.EnsureNotDisposed(); EncodeVisitor visitor = new EncodeVisitor(encoder, stream); this.AcceptVisitor(visitor); @@ -128,6 +143,11 @@ namespace SixLabors.ImageSharp /// The . protected void UpdateSize(Size size) => this.size = size; + /// + /// Implements the Dispose logic. + /// + protected abstract void DisposeImpl(); + private class EncodeVisitor : IImageVisitor { private readonly IImageEncoder encoder; diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index ec4c364d8..6ea2b234c 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -119,5 +119,16 @@ namespace SixLabors.ImageSharp return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(stream.ToArray())}"; } } + + /// + /// Throws if the image is disposed. + /// + internal static void EnsureNotDisposed(this Image image) + { + if (image.IsDisposed) + { + throw new ObjectDisposedException(nameof(image), "Trying to execute an operation on a disposed image."); + } + } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 5d0364dd8..a7ea58652 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -162,6 +162,8 @@ namespace SixLabors.ImageSharp /// Returns a new with all the same pixel data as the original. public Image Clone(Configuration configuration) { + this.EnsureNotDisposed(); + IEnumerable> clonedFrames = this.Frames.Select, ImageFrame>(x => x.Clone(configuration)); return new Image(configuration, this.Metadata.DeepClone(), clonedFrames); @@ -175,17 +177,21 @@ namespace SixLabors.ImageSharp /// The . public override Image CloneAs(Configuration configuration) { + this.EnsureNotDisposed(); + IEnumerable> clonedFrames = this.Frames.Select, ImageFrame>(x => x.CloneAs(configuration)); return new Image(configuration, this.Metadata.DeepClone(), clonedFrames); } /// - public override void Dispose() => this.Frames.Dispose(); + protected override void DisposeImpl() => this.Frames.Dispose(); /// internal override void AcceptVisitor(IImageVisitor visitor) { + this.EnsureNotDisposed(); + visitor.Visit(this); } diff --git a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs index 2304a44c3..cb3f8fa57 100644 --- a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs @@ -21,6 +21,10 @@ namespace SixLabors.ImageSharp.Processing /// The operation to perform on the source. public static void Mutate(this Image source, Action operation) { + Guard.NotNull(source, nameof(source)); + Guard.NotNull(operation, nameof(operation)); + source.EnsureNotDisposed(); + ProcessingVisitor visitor = new ProcessingVisitor(operation, true); source.AcceptVisitor(visitor); } @@ -34,8 +38,9 @@ namespace SixLabors.ImageSharp.Processing public static void Mutate(this Image source, Action operation) where TPixel : struct, IPixel { - Guard.NotNull(operation, nameof(operation)); Guard.NotNull(source, nameof(source)); + Guard.NotNull(operation, nameof(operation)); + source.EnsureNotDisposed(); IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider .CreateImageProcessingContext(source, true); @@ -51,8 +56,9 @@ namespace SixLabors.ImageSharp.Processing public static void Mutate(this Image source, params IImageProcessor[] operations) where TPixel : struct, IPixel { - Guard.NotNull(operations, nameof(operations)); Guard.NotNull(source, nameof(source)); + Guard.NotNull(operations, nameof(operations)); + source.EnsureNotDisposed(); IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider .CreateImageProcessingContext(source, true); @@ -67,6 +73,10 @@ namespace SixLabors.ImageSharp.Processing /// The new . public static Image Clone(this Image source, Action operation) { + Guard.NotNull(source, nameof(source)); + Guard.NotNull(operation, nameof(operation)); + source.EnsureNotDisposed(); + ProcessingVisitor visitor = new ProcessingVisitor(operation, false); source.AcceptVisitor(visitor); return visitor.ResultImage; @@ -82,8 +92,9 @@ namespace SixLabors.ImageSharp.Processing public static Image Clone(this Image source, Action operation) where TPixel : struct, IPixel { - Guard.NotNull(operation, nameof(operation)); Guard.NotNull(source, nameof(source)); + Guard.NotNull(operation, nameof(operation)); + source.EnsureNotDisposed(); IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider .CreateImageProcessingContext(source, false); @@ -101,8 +112,9 @@ namespace SixLabors.ImageSharp.Processing public static Image Clone(this Image source, params IImageProcessor[] operations) where TPixel : struct, IPixel { - Guard.NotNull(operations, nameof(operations)); Guard.NotNull(source, nameof(source)); + Guard.NotNull(operations, nameof(operations)); + source.EnsureNotDisposed(); IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider .CreateImageProcessingContext(source, false); @@ -152,4 +164,4 @@ namespace SixLabors.ImageSharp.Processing } } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Image/ImageCloneTests.cs b/tests/ImageSharp.Tests/Image/ImageCloneTests.cs index 82864f156..66081348a 100644 --- a/tests/ImageSharp.Tests/Image/ImageCloneTests.cs +++ b/tests/ImageSharp.Tests/Image/ImageCloneTests.cs @@ -7,6 +7,24 @@ namespace SixLabors.ImageSharp.Tests { public class ImageCloneTests { + [Fact] + public void CloneAs_WhenDisposed_Throws() + { + Image image = new Image(5, 5); + image.Dispose(); + + Assert.Throws(() => image.CloneAs()); + } + + [Fact] + public void Clone_WhenDisposed_Throws() + { + Image image = new Image(5, 5); + image.Dispose(); + + Assert.Throws(() => image.Clone()); + } + [Theory] [WithTestPatternImages(9, 9, PixelTypes.Rgba32)] public void CloneAs_ToBgra32(TestImageProvider provider) @@ -109,4 +127,4 @@ namespace SixLabors.ImageSharp.Tests } } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Image/ImageTests.Save.cs b/tests/ImageSharp.Tests/Image/ImageTests.Save.cs index 66b0dd21d..61f694518 100644 --- a/tests/ImageSharp.Tests/Image/ImageTests.Save.cs +++ b/tests/ImageSharp.Tests/Image/ImageTests.Save.cs @@ -4,6 +4,10 @@ // ReSharper disable InconsistentNaming using System; +using System.IO; + +using Moq; + using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.PixelFormats; using Xunit; @@ -65,6 +69,18 @@ namespace SixLabors.ImageSharp.Tests Assert.Equal("image/png", mime.DefaultMimeType); } } + + [Fact] + public void ThrowsWhenDisposed() + { + var image = new Image(5, 5); + image.Dispose(); + IImageEncoder encoder = Mock.Of(); + using (MemoryStream stream = new MemoryStream()) + { + Assert.Throws(() => image.Save(stream, encoder)); + } + } } } } diff --git a/tests/ImageSharp.Tests/ImageOperationTests.cs b/tests/ImageSharp.Tests/ImageOperationTests.cs index c997f9bd0..20c3b728f 100644 --- a/tests/ImageSharp.Tests/ImageOperationTests.cs +++ b/tests/ImageSharp.Tests/ImageOperationTests.cs @@ -23,19 +23,22 @@ namespace SixLabors.ImageSharp.Tests private readonly FakeImageOperationsProvider provider; private readonly IImageProcessor processorDefinition; + private static readonly string ExpectedExceptionMessage = GetExpectedExceptionText(); + public ImageOperationTests() { this.provider = new FakeImageOperationsProvider(); Mock processorMock = new Mock(); this.processorDefinition = processorMock.Object; - + this.image = new Image(new Configuration { ImageOperationsProvider = this.provider }, 1, 1); } + [Fact] public void MutateCallsImageOperationsProvider_Func_OriginalImage() { @@ -109,5 +112,65 @@ namespace SixLabors.ImageSharp.Tests } public void Dispose() => this.image.Dispose(); + + [Fact] + public void GenericMutate_WhenDisposed_Throws() + { + this.image.Dispose(); + + CheckThrowsCorrectObjectDisposedException( + () => this.image.Mutate(x => x.ApplyProcessor(this.processorDefinition))); + } + + [Fact] + public void GenericClone_WhenDisposed_Throws() + { + this.image.Dispose(); + + CheckThrowsCorrectObjectDisposedException( + () => this.image.Clone(x => x.ApplyProcessor(this.processorDefinition))); + } + + [Fact] + public void AgnosticMutate_WhenDisposed_Throws() + { + this.image.Dispose(); + Image img = this.image; + + CheckThrowsCorrectObjectDisposedException( + () => img.Mutate(x => x.ApplyProcessor(this.processorDefinition))); + } + + [Fact] + public void AgnosticClone_WhenDisposed_Throws() + { + this.image.Dispose(); + Image img = this.image; + + CheckThrowsCorrectObjectDisposedException( + () => img.Clone(x => x.ApplyProcessor(this.processorDefinition))); + } + + private static string GetExpectedExceptionText() + { + try + { + Image img = new Image(1, 1); + img.Dispose(); + img.EnsureNotDisposed(); + } + catch (ObjectDisposedException ex) + { + return ex.Message; + } + + return "?"; + } + + private static void CheckThrowsCorrectObjectDisposedException(Action action) + { + var ex = Assert.Throws(action); + Assert.Equal(ExpectedExceptionMessage, ex.Message); + } } }