From 03ba4c6e77969a7cdad56e5115b8f8de30650ba4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 15:37:50 +1000 Subject: [PATCH 1/3] Refactor Image.Dispose --- src/ImageSharp/IImage.cs | 4 ++-- src/ImageSharp/Image.cs | 27 +++++++++++-------------- src/ImageSharp/ImageExtensions.cs | 13 +----------- src/ImageSharp/ImageFrame.cs | 12 ++++++++++- src/ImageSharp/ImageFrame{TPixel}.cs | 16 +++++++-------- src/ImageSharp/Image{TPixel}.cs | 30 +++++++++++++++++++++++++--- 6 files changed, 61 insertions(+), 41 deletions(-) diff --git a/src/ImageSharp/IImage.cs b/src/ImageSharp/IImage.cs index b9e2cee616..0d4dc3c9d9 100644 --- a/src/ImageSharp/IImage.cs +++ b/src/ImageSharp/IImage.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -11,4 +11,4 @@ namespace SixLabors.ImageSharp public interface IImage : IImageInfo, IDisposable { } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 57f60f2e75..696f836628 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; using SixLabors.ImageSharp.Advanced; @@ -80,21 +81,11 @@ internal Image( /// Configuration IConfigurable.Configuration => this.Configuration; - /// - /// Gets a value indicating whether the image instance is disposed. - /// - public bool IsDisposed { get; private set; } - /// public void Dispose() { - if (this.IsDisposed) - { - return; - } - - this.IsDisposed = true; - this.DisposeImpl(); + this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -109,7 +100,7 @@ public void Save(Stream stream, IImageEncoder encoder) Guard.NotNull(encoder, nameof(encoder)); this.EnsureNotDisposed(); - EncodeVisitor visitor = new EncodeVisitor(encoder, stream); + var visitor = new EncodeVisitor(encoder, stream); this.AcceptVisitor(visitor); } @@ -144,9 +135,15 @@ public abstract Image CloneAs(Configuration configuration) protected void UpdateSize(Size size) => this.size = size; /// - /// Implements the Dispose logic. + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// Whether to dispose of managed and unmanaged objects. + protected abstract void Dispose(bool disposing); + + /// + /// Throws if the image is disposed. /// - protected abstract void DisposeImpl(); + internal abstract void EnsureNotDisposed(); private class EncodeVisitor : IImageVisitor { diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index 6ea2b234c5..6cdc948d40 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -119,16 +119,5 @@ public static string ToBase64String(this Image source, IImageFor 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."); - } - } } } diff --git a/src/ImageSharp/ImageFrame.cs b/src/ImageSharp/ImageFrame.cs index f3fe1ed8d4..91872b21d6 100644 --- a/src/ImageSharp/ImageFrame.cs +++ b/src/ImageSharp/ImageFrame.cs @@ -74,7 +74,17 @@ protected ImageFrame(Configuration configuration, int width, int height, ImageFr public Rectangle Bounds() => new Rectangle(0, 0, this.Width, this.Height); /// - public abstract void Dispose(); + public void Dispose() + { + this.Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// Whether to dispose of managed and unmanaged objects. + protected abstract void Dispose(bool disposing); internal abstract void CopyPixelsTo(Span destination) where TDestinationPixel : struct, IPixel; diff --git a/src/ImageSharp/ImageFrame{TPixel}.cs b/src/ImageSharp/ImageFrame{TPixel}.cs index 5c9ff489e1..0436eb9d2b 100644 --- a/src/ImageSharp/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/ImageFrame{TPixel}.cs @@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp /// In all other cases it is the only frame of the image. /// /// The pixel format. - public sealed class ImageFrame : ImageFrame, IPixelSource, IDisposable + public sealed class ImageFrame : ImageFrame, IPixelSource where TPixel : struct, IPixel { private bool isDisposed; @@ -196,20 +196,20 @@ internal void SwapOrCopyPixelsBufferFrom(ImageFrame pixelSource) this.UpdateSize(this.PixelBuffer.Size()); } - /// - /// Disposes the object and frees resources for the Garbage Collector. - /// - public override void Dispose() + /// + protected override void Dispose(bool disposing) { if (this.isDisposed) { return; } - this.PixelBuffer?.Dispose(); - this.PixelBuffer = null; + if (disposing) + { + this.PixelBuffer?.Dispose(); + this.PixelBuffer = null; + } - // Note disposing is done. this.isDisposed = true; } diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index a7ea58652c..6dbfde20c6 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -18,9 +18,11 @@ namespace SixLabors.ImageSharp /// For generic -s the pixel type is known at compile time. /// /// The pixel format. - public sealed class Image : Image + public class Image : Image where TPixel : struct, IPixel { + private bool isDisposed; + /// /// Initializes a new instance of the class /// with the height and the width of the image. @@ -185,7 +187,29 @@ public override Image CloneAs(Configuration configuration) } /// - protected override void DisposeImpl() => this.Frames.Dispose(); + protected override void Dispose(bool disposing) + { + if (this.isDisposed) + { + return; + } + + if (disposing) + { + this.Frames.Dispose(); + } + + this.isDisposed = true; + } + + /// + internal override void EnsureNotDisposed() + { + if (this.isDisposed) + { + throw new ObjectDisposedException("Trying to execute an operation on a disposed image."); + } + } /// internal override void AcceptVisitor(IImageVisitor visitor) From 76708ef2d605c7468d02332b96f08c77783906f2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 15:45:58 +1000 Subject: [PATCH 2/3] Refactor IImageProcessor --- .../Processors/CloningImageProcessor{TPixel}.cs | 7 +------ .../Processing/Processors/ImageProcessor{TPixel}.cs | 8 ++------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs index 8b3e1eb965..c2c8690529 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class CloningImageProcessor : ICloningImageProcessor where TPixel : struct, IPixel { - private bool isDisposed; - /// /// Initializes a new instance of the class. /// @@ -104,6 +102,7 @@ public void Apply() public void Dispose() { this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -160,10 +159,6 @@ protected virtual void AfterImageApply(Image destination) /// Whether to dispose managed and unmanaged objects. protected virtual void Dispose(bool disposing) { - if (!this.isDisposed) - { - this.isDisposed = true; - } } } } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs index 8ac8cd67bc..55b4d3dc73 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class ImageProcessor : IImageProcessor where TPixel : struct, IPixel { - private bool isDisposed; - /// /// Initializes a new instance of the class. /// @@ -97,6 +95,8 @@ public void Apply(ImageFrame source) /// public virtual void Dispose() { + this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -142,10 +142,6 @@ protected virtual void AfterImageApply() /// Whether to dispose managed and unmanaged objects. protected virtual void Dispose(bool disposing) { - if (!this.isDisposed) - { - this.isDisposed = true; - } } } } From b0ef8a10d1fac2d9bb0cb7fb75a926b6a76cf587 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 18 Sep 2019 08:36:04 +1000 Subject: [PATCH 3/3] reseal Image{TPixel} --- src/ImageSharp/Image{TPixel}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 6dbfde20c6..181e818ee7 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp /// For generic -s the pixel type is known at compile time. /// /// The pixel format. - public class Image : Image + public sealed class Image : Image where TPixel : struct, IPixel { private bool isDisposed;