From 3c2885e382e9c5057d4e059ca6da4bdadb01f8a8 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 23 Jan 2023 23:13:53 +1000 Subject: [PATCH 01/12] Allow zero DPI and only throw at EOF when not enough data --- .../Jpeg/Components/Decoder/JFifMarker.cs | 22 ++++--------------- .../Components/Decoder/SpectralConverter.cs | 6 +++++ .../Decoder/SpectralConverter{TPixel}.cs | 6 +++++ .../Formats/Jpeg/JpegDecoderCore.cs | 12 ++++++++++ .../Formats/Jpg/JpegDecoderTests.cs | 11 ++++++++++ .../Formats/Jpg/SpectralJpegTests.cs | 2 ++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Jpg/issues/issue-2315.jpg | 3 +++ 8 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/issue-2315.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs index 42dcf7200a..e13b26f9a9 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs @@ -26,16 +26,6 @@ internal readonly struct JFifMarker : IEquatable /// The vertical pixel density. private JFifMarker(byte majorVersion, byte minorVersion, byte densityUnits, short xDensity, short yDensity) { - if (xDensity <= 0) - { - JpegThrowHelper.ThrowInvalidImageContentException($"X-Density {xDensity} must be greater than 0."); - } - - if (yDensity <= 0) - { - JpegThrowHelper.ThrowInvalidImageContentException($"Y-Density {yDensity} must be greater than 0."); - } - this.MajorVersion = majorVersion; this.MinorVersion = minorVersion; @@ -64,12 +54,12 @@ internal readonly struct JFifMarker : IEquatable public PixelResolutionUnit DensityUnits { get; } /// - /// Gets the horizontal pixel density. Must not be zero. + /// Gets the horizontal pixel density. /// public short XDensity { get; } /// - /// Gets the vertical pixel density. Must not be zero. + /// Gets the vertical pixel density. /// public short YDensity { get; } @@ -88,12 +78,8 @@ internal readonly struct JFifMarker : IEquatable byte densityUnits = bytes[7]; short xDensity = (short)((bytes[8] << 8) | bytes[9]); short yDensity = (short)((bytes[10] << 8) | bytes[11]); - - if (xDensity > 0 && yDensity > 0) - { - marker = new JFifMarker(majorVersion, minorVersion, densityUnits, xDensity, yDensity); - return true; - } + marker = new JFifMarker(majorVersion, minorVersion, densityUnits, xDensity, yDensity); + return true; } marker = default; diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs index 0eb484b94a..51d9bfbced 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs @@ -121,4 +121,10 @@ internal abstract class SpectralConverter return size; } + + /// + /// Gets a value indicating whether the converter has a pixel buffer. + /// + /// if the converter has a pixel buffer; otherwise, . + public abstract bool HasPixelBuffer(); } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 4bb58d8a12..d6250127f5 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -85,6 +85,12 @@ internal class SpectralConverter : SpectralConverter, IDisposable /// public Configuration Configuration { get; } + /// + /// Gets a value indicating whether the converter has a pixel buffer. + /// + /// if the converter has a pixel buffer; otherwise, . + public override bool HasPixelBuffer() => this.pixelBuffer is not null; + /// /// Gets converted pixel buffer. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 511ec1ba1d..51de9ffbd1 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -356,6 +356,18 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals // to uint to avoid sign extension. if (stream.RemainingBytes < (uint)markerContentByteSize) { + if (metadataOnly && this.Metadata != null && this.Frame != null) + { + // We have enough data to decode the image, so we can stop parsing. + return; + } + + if (this.Metadata != null && this.Frame != null && spectralConverter.HasPixelBuffer()) + { + // We have enough data to decode the image, so we can stop parsing. + return; + } + JpegThrowHelper.ThrowNotEnoughBytesForMarker(fileMarker.Marker); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 1d5a7e0ff8..425d12497c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -300,4 +300,15 @@ public partial class JpegDecoderTests image.DebugSave(provider); image.CompareToOriginal(provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2315 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2315_NotEnoughBytes, PixelTypes.Rgba32)] + public void Issue2315_DecodeWorks(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(JpegDecoder.Instance); + image.DebugSave(provider); + image.CompareToOriginal(provider); + } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs index 58347a0724..805ee586a8 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs @@ -187,6 +187,8 @@ public class SpectralJpegTests } } + public override bool HasPixelBuffer() => throw new NotImplementedException(); + public override void InjectFrameData(JpegFrame frame, IRawJpegData jpegData) { this.frame = frame; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 29223b1fe0..22a48ebee4 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -281,6 +281,7 @@ public static class TestImages public const string ValidExifArgumentNullExceptionOnEncode = "Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg"; public const string Issue2133_DeduceColorSpace = "Jpg/issues/Issue2133.jpg"; public const string Issue2136_ScanMarkerExtraneousBytes = "Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg"; + public const string Issue2315_NotEnoughBytes = "Jpg/issues/issue-2315.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/issue-2315.jpg b/tests/Images/Input/Jpg/issues/issue-2315.jpg new file mode 100644 index 0000000000..fe3001eddc --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue-2315.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:f129b057efb499d492e9afcffdd98de62aac1e04b97a09a75b4799ba498cd3c1 +size 319056 From c44cd6c244f4671fdb8b4c7fd426afc865e55084 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 23 Jan 2023 23:26:15 +1000 Subject: [PATCH 02/12] Update DecodeJpegParseStreamOnly.cs --- .../Codecs/Jpeg/DecodeJpegParseStreamOnly.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs index aa88242ce4..b5a7245292 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs @@ -54,6 +54,8 @@ public class DecodeJpegParseStreamOnly { } + public override bool HasPixelBuffer() => throw new NotImplementedException(); + public override void InjectFrameData(JpegFrame frame, IRawJpegData jpegData) { } From c224dece1765026ca0da5086af88e71ed1fb30fc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 24 Jan 2023 08:57:39 +1000 Subject: [PATCH 03/12] Update JFifMarkerTests.cs --- tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs index 3890c20e93..3b7e20eb4e 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs @@ -46,15 +46,6 @@ public class JFifMarkerTests Assert.Equal(default, marker); } - [Fact] - public void MarkerIgnoresCorrectHeaderButInvalidDensities() - { - bool isJFif = JFifMarker.TryParse(this.bytes3, out JFifMarker marker); - - Assert.False(isJFif); - Assert.Equal(default, marker); - } - [Fact] public void MarkerEqualityIsCorrect() { From b41611ae6d43ff2e534ed0148169cdb13d6fd9e7 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Tue, 24 Jan 2023 08:00:38 +0100 Subject: [PATCH 04/12] Remove nullable disable from zlib #2231 --- .../Compression/Zlib/DeflateThrowHelper.cs | 10 +++++++++ src/ImageSharp/Compression/Zlib/Deflater.cs | 2 -- .../Compression/Zlib/DeflaterEngine.cs | 12 ++++------ .../Compression/Zlib/DeflaterHuffman.cs | 9 -------- .../Compression/Zlib/DeflaterOutputStream.cs | 3 --- .../Compression/Zlib/DeflaterPendingBuffer.cs | 2 -- .../Compression/Zlib/ZlibDeflateStream.cs | 3 --- .../Compression/Zlib/ZlibInflateStream.cs | 6 +++-- .../Diagnostics/MemoryDiagnostics.cs | 7 +++--- .../Decompressors/DeflateTiffCompression.cs | 22 ++++++++++--------- 10 files changed, 33 insertions(+), 43 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/DeflateThrowHelper.cs b/src/ImageSharp/Compression/Zlib/DeflateThrowHelper.cs index ba22d5d85e..30761328f3 100644 --- a/src/ImageSharp/Compression/Zlib/DeflateThrowHelper.cs +++ b/src/ImageSharp/Compression/Zlib/DeflateThrowHelper.cs @@ -1,23 +1,33 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Diagnostics.CodeAnalysis; + namespace SixLabors.ImageSharp.Compression.Zlib; internal static class DeflateThrowHelper { + [DoesNotReturn] public static void ThrowAlreadyFinished() => throw new InvalidOperationException("Finish() already called."); + [DoesNotReturn] public static void ThrowAlreadyClosed() => throw new InvalidOperationException("Deflator already closed."); + [DoesNotReturn] public static void ThrowUnknownCompression() => throw new InvalidOperationException("Unknown compression function."); + [DoesNotReturn] public static void ThrowNotProcessed() => throw new InvalidOperationException("Old input was not completely processed."); + [DoesNotReturn] public static void ThrowNull(string name) => throw new ArgumentNullException(name); + [DoesNotReturn] public static void ThrowOutOfRange(string name) => throw new ArgumentOutOfRangeException(name); + [DoesNotReturn] public static void ThrowHeapViolated() => throw new InvalidOperationException("Huffman heap invariant violated."); + [DoesNotReturn] public static void ThrowNoDeflate() => throw new ImageFormatException("Cannot deflate all input."); } diff --git a/src/ImageSharp/Compression/Zlib/Deflater.cs b/src/ImageSharp/Compression/Zlib/Deflater.cs index f001b8a67c..f642ec85a7 100644 --- a/src/ImageSharp/Compression/Zlib/Deflater.cs +++ b/src/ImageSharp/Compression/Zlib/Deflater.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory; @@ -285,7 +284,6 @@ internal sealed class Deflater : IDisposable if (!this.isDisposed) { this.engine.Dispose(); - this.engine = null; this.isDisposed = true; } } diff --git a/src/ImageSharp/Compression/Zlib/DeflaterEngine.cs b/src/ImageSharp/Compression/Zlib/DeflaterEngine.cs index 47bc43f52e..31fa0238bf 100644 --- a/src/ImageSharp/Compression/Zlib/DeflaterEngine.cs +++ b/src/ImageSharp/Compression/Zlib/DeflaterEngine.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Runtime.CompilerServices; @@ -87,7 +86,7 @@ internal sealed unsafe class DeflaterEngine : IDisposable /// /// The input data for compression. /// - private byte[] inputBuf; + private byte[]? inputBuf; /// /// The offset into inputBuf, where input data starts. @@ -222,7 +221,7 @@ internal sealed unsafe class DeflaterEngine : IDisposable /// The buffer containing input data. /// The offset of the first byte of data. /// The number of bytes of data to use as input. - public void SetInput(byte[] buffer, int offset, int count) + public void SetInput(byte[]? buffer, int offset, int count) { if (buffer is null) { @@ -362,6 +361,8 @@ internal sealed unsafe class DeflaterEngine : IDisposable more = this.inputEnd - this.inputOff; } + ArgumentNullException.ThrowIfNull(this.inputBuf); + Unsafe.CopyBlockUnaligned( ref this.window.Span[this.strstart + this.lookahead], ref this.inputBuf[this.inputOff], @@ -393,11 +394,6 @@ internal sealed unsafe class DeflaterEngine : IDisposable this.prevMemoryHandle.Dispose(); this.prevMemoryOwner.Dispose(); - this.windowMemoryOwner = null; - this.headMemoryOwner = null; - this.prevMemoryOwner = null; - this.huffman = null; - this.isDisposed = true; } } diff --git a/src/ImageSharp/Compression/Zlib/DeflaterHuffman.cs b/src/ImageSharp/Compression/Zlib/DeflaterHuffman.cs index 8c67699085..dc11de4259 100644 --- a/src/ImageSharp/Compression/Zlib/DeflaterHuffman.cs +++ b/src/ImageSharp/Compression/Zlib/DeflaterHuffman.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Runtime.CompilerServices; @@ -427,10 +426,6 @@ internal sealed unsafe class DeflaterHuffman : IDisposable this.blTree.Dispose(); this.distTree.Dispose(); - this.Pending = null; - this.literalTree = null; - this.blTree = null; - this.distTree = null; this.isDisposed = true; } } @@ -977,10 +972,6 @@ internal sealed unsafe class DeflaterHuffman : IDisposable this.codesMemoryHandle.Dispose(); this.codesMemoryOwner.Dispose(); - this.frequenciesMemoryOwner = null; - this.lengthsMemoryOwner = null; - this.codesMemoryOwner = null; - this.isDisposed = true; } } diff --git a/src/ImageSharp/Compression/Zlib/DeflaterOutputStream.cs b/src/ImageSharp/Compression/Zlib/DeflaterOutputStream.cs index cea2fd2721..de818fd8f5 100644 --- a/src/ImageSharp/Compression/Zlib/DeflaterOutputStream.cs +++ b/src/ImageSharp/Compression/Zlib/DeflaterOutputStream.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using SixLabors.ImageSharp.Memory; @@ -137,8 +136,6 @@ internal sealed class DeflaterOutputStream : Stream this.memoryOwner.Dispose(); } - this.deflater = null; - this.memoryOwner = null; this.isDisposed = true; base.Dispose(disposing); } diff --git a/src/ImageSharp/Compression/Zlib/DeflaterPendingBuffer.cs b/src/ImageSharp/Compression/Zlib/DeflaterPendingBuffer.cs index 28febdc118..37e7404e40 100644 --- a/src/ImageSharp/Compression/Zlib/DeflaterPendingBuffer.cs +++ b/src/ImageSharp/Compression/Zlib/DeflaterPendingBuffer.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Runtime.CompilerServices; @@ -180,7 +179,6 @@ internal sealed unsafe class DeflaterPendingBuffer : IDisposable { this.bufferMemoryHandle.Dispose(); this.bufferMemoryOwner.Dispose(); - this.bufferMemoryOwner = null; this.isDisposed = true; } } diff --git a/src/ImageSharp/Compression/Zlib/ZlibDeflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibDeflateStream.cs index 00c4aed3b1..2e52f84d7b 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibDeflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibDeflateStream.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Formats.Png; @@ -172,8 +171,6 @@ internal sealed class ZlibDeflateStream : Stream this.rawStream.WriteByte((byte)(crc & 0xFF)); } - this.deflateStream = null; - base.Dispose(disposing); this.isDisposed = true; } diff --git a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs index f9e678605c..06a7c3928c 100644 --- a/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Compression/Zlib/ZlibInflateStream.cs @@ -1,7 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable +using System.Diagnostics.CodeAnalysis; using System.IO.Compression; using SixLabors.ImageSharp.IO; @@ -90,7 +90,7 @@ internal sealed class ZlibInflateStream : Stream /// /// Gets the compressed stream over the deframed inner stream. /// - public DeflateStream CompressedStream { get; private set; } + public DeflateStream? CompressedStream { get; private set; } /// /// Adds new bytes from a frame found in the original stream. @@ -98,6 +98,7 @@ internal sealed class ZlibInflateStream : Stream /// The current remaining data according to the chunk length. /// Whether the chunk to be inflated is a critical chunk. /// The . + [MemberNotNullWhen(true, nameof(CompressedStream))] public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { this.currentDataRemaining = bytes; @@ -210,6 +211,7 @@ internal sealed class ZlibInflateStream : Stream this.isDisposed = true; } + [MemberNotNullWhen(true, nameof(CompressedStream))] private bool InitializeInflateStream(bool isCriticalChunk) { // Read the zlib header : http://tools.ietf.org/html/rfc1950 diff --git a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs index f00ee1b2a8..8327daf23b 100644 --- a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs +++ b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable namespace SixLabors.ImageSharp.Diagnostics; @@ -17,7 +16,7 @@ public static class MemoryDiagnostics { private static int totalUndisposedAllocationCount; - private static UndisposedAllocationDelegate undisposedAllocation; + private static UndisposedAllocationDelegate? undisposedAllocation; private static int undisposedAllocationSubscriptionCounter; private static readonly object SyncRoot = new(); @@ -50,12 +49,12 @@ public static class MemoryDiagnostics /// /// Fires when ImageSharp allocates memory from a MemoryAllocator /// - internal static event Action MemoryAllocated; + internal static event Action? MemoryAllocated; /// /// Fires when ImageSharp releases memory allocated from a MemoryAllocator /// - internal static event Action MemoryReleased; + internal static event Action? MemoryReleased; /// /// Gets a value indicating the total number of memory resource objects leaked to the finalizer. diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs index 4c377c8783..27c311009c 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs @@ -50,19 +50,21 @@ internal sealed class DeflateTiffCompression : TiffBaseDecompressor return left > 0 ? left : 0; })) { - deframeStream.AllocateNewBytes(byteCount, true); - DeflateStream dataStream = deframeStream.CompressedStream; - - int totalRead = 0; - while (totalRead < buffer.Length) + if (deframeStream.AllocateNewBytes(byteCount, true)) { - int bytesRead = dataStream.Read(buffer, totalRead, buffer.Length - totalRead); - if (bytesRead <= 0) + DeflateStream? dataStream = deframeStream.CompressedStream; + + int totalRead = 0; + while (totalRead < buffer.Length) { - break; - } + int bytesRead = dataStream.Read(buffer, totalRead, buffer.Length - totalRead); + if (bytesRead <= 0) + { + break; + } - totalRead += bytesRead; + totalRead += bytesRead; + } } } From e70e71b16c665e98cb48f464fdc3ab37c614f916 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 10:57:29 +0100 Subject: [PATCH 05/12] Remove nullable disable from Memory.Allocators #2231 --- .../Memory/Allocators/Internals/Gen2GcCallback.cs | 11 +++++------ .../Allocators/Internals/ManagedBufferBase.cs | 2 +- .../Internals/RefCountedMemoryLifetimeGuard.cs | 3 +-- .../Internals/SharedArrayPoolBuffer{T}.cs | 6 ++---- .../Internals/UniformUnmanagedMemoryPool.cs | 7 +++---- .../Allocators/Internals/UnmanagedMemoryHandle.cs | 11 +++++------ .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 2 +- src/ImageSharp/Memory/Buffer2DExtensions.cs | 2 +- src/ImageSharp/Memory/Buffer2D{T}.cs | 4 ++-- .../DiscontiguousBuffers/MemoryGroupExtensions.cs | 2 +- .../DiscontiguousBuffers/MemoryGroupSpanCache.cs | 2 +- .../DiscontiguousBuffers/MemoryGroupView{T}.cs | 6 ++++-- .../MemoryGroup{T}.Consumed.cs | 2 +- .../DiscontiguousBuffers/MemoryGroup{T}.Owned.cs | 14 ++++++++------ .../Memory/DiscontiguousBuffers/MemoryGroup{T}.cs | 8 ++++---- 15 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs b/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs index 547f67d6a7..c0737a1140 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable // Port of BCL internal utility: // https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/System.Private.CoreLib/src/System/Gen2GcCallback.cs @@ -15,8 +14,8 @@ namespace SixLabors.ImageSharp.Memory.Internals; /// internal sealed class Gen2GcCallback : CriticalFinalizerObject { - private readonly Func callback0; - private readonly Func callback1; + private readonly Func? callback0; + private readonly Func? callback1; private GCHandle weakTargetObj; private Gen2GcCallback(Func callback) => this.callback0 = callback; @@ -32,7 +31,7 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject if (this.weakTargetObj.IsAllocated) { // Check to see if the target object is still alive. - object targetObj = this.weakTargetObj.Target; + object? targetObj = this.weakTargetObj.Target; if (targetObj == null) { // The target object is dead, so this callback object is no longer needed. @@ -43,7 +42,7 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject // Execute the callback method. try { - if (!this.callback1(targetObj)) + if (this.callback1 is not null && !this.callback1(targetObj)) { // If the callback returns false, this callback object is no longer needed. this.weakTargetObj.Free(); @@ -64,7 +63,7 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject // Execute the callback method. try { - if (!this.callback0()) + if (this.callback0 is not null && !this.callback0()) { // If the callback returns false, this callback object is no longer needed. return; diff --git a/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs b/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs index a18bd34474..be4cb8c8e4 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs @@ -43,5 +43,5 @@ internal abstract class ManagedBufferBase : MemoryManager /// Gets the object that should be pinned. /// /// The pinnable . - protected abstract object GetPinnableObject(); + protected abstract object? GetPinnableObject(); } diff --git a/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs index aed5d03e4b..4a202a96c3 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using SixLabors.ImageSharp.Diagnostics; @@ -15,7 +14,7 @@ internal abstract class RefCountedMemoryLifetimeGuard : IDisposable private int refCount = 1; private int disposed; private int released; - private string allocationStackTrace; + private string? allocationStackTrace; protected RefCountedMemoryLifetimeGuard() { diff --git a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs index 166992266c..1d73b2ddf2 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Diagnostics; @@ -22,7 +21,7 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted this.lifetimeGuard = new LifetimeGuard(this.Array); } - public byte[] Array { get; private set; } + public byte[]? Array { get; private set; } protected override void Dispose(bool disposing) { @@ -41,7 +40,7 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted return MemoryMarshal.Cast(this.Array.AsSpan(0, this.lengthInBytes)); } - protected override object GetPinnableObject() => this.Array; + protected override object? GetPinnableObject() => this.Array; public void AddRef() { @@ -74,7 +73,6 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted // meaning likely a different bucket than it was rented from, // but this is PROBABLY better than not returning the arrays at all. ArrayPool.Shared.Return(this.array); - this.array = null; } } } diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs index 464bc21650..aa8bcd3859 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Diagnostics; @@ -13,7 +12,7 @@ internal partial class UniformUnmanagedMemoryPool : System.Runtime.ConstrainedEx { private static int minTrimPeriodMilliseconds = int.MaxValue; private static readonly List> AllPools = new(); - private static Timer trimTimer; + private static Timer? trimTimer; private static readonly Stopwatch Stopwatch = Stopwatch.StartNew(); @@ -97,7 +96,7 @@ internal partial class UniformUnmanagedMemoryPool : System.Runtime.ConstrainedEx /// /// Rent buffers or return 'null' if the pool is full. /// - public UnmanagedMemoryHandle[] Rent(int bufferCount) + public UnmanagedMemoryHandle[]? Rent(int bufferCount) { UnmanagedMemoryHandle[] buffersLocal = this.buffers; @@ -248,7 +247,7 @@ internal partial class UniformUnmanagedMemoryPool : System.Runtime.ConstrainedEx foreach (WeakReference weakPoolRef in AllPools) { - if (weakPoolRef.TryGetTarget(out UniformUnmanagedMemoryPool pool)) + if (weakPoolRef.TryGetTarget(out UniformUnmanagedMemoryPool? pool)) { pool.Trim(); } diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs index a6da445b65..c13fa754e2 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs @@ -1,6 +1,5 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Runtime.InteropServices; @@ -20,7 +19,7 @@ internal struct UnmanagedMemoryHandle : IEquatable private static long totalOomRetries; // A Monitor to wait/signal when we are low on memory. - private static object lowMemoryMonitor; + private static object? lowMemoryMonitor; public static readonly UnmanagedMemoryHandle NullHandle; @@ -114,9 +113,9 @@ internal struct UnmanagedMemoryHandle : IEquatable if (Volatile.Read(ref lowMemoryMonitor) != null) { // We are low on memory. Signal all threads waiting in AllocateHandle(). - Monitor.Enter(lowMemoryMonitor); - Monitor.PulseAll(lowMemoryMonitor); - Monitor.Exit(lowMemoryMonitor); + Monitor.Enter(lowMemoryMonitor!); + Monitor.PulseAll(lowMemoryMonitor!); + Monitor.Exit(lowMemoryMonitor!); } this.lengthInBytes = 0; @@ -124,7 +123,7 @@ internal struct UnmanagedMemoryHandle : IEquatable public bool Equals(UnmanagedMemoryHandle other) => this.handle.Equals(other.handle); - public override bool Equals(object obj) => obj is UnmanagedMemoryHandle other && this.Equals(other); + public override bool Equals(object? obj) => obj is UnmanagedMemoryHandle other && this.Equals(other); public override int GetHashCode() => this.handle.GetHashCode(); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index bf75a716dd..2cb4421d5e 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -135,7 +135,7 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato } // Attempt to rent the whole group from the pool, allocate a group of unmanaged buffers if the attempt fails: - if (MemoryGroup.TryAllocate(this.pool, totalLength, bufferAlignment, options, out MemoryGroup poolGroup)) + if (MemoryGroup.TryAllocate(this.pool, totalLength, bufferAlignment, options, out MemoryGroup? poolGroup)) { return poolGroup; } diff --git a/src/ImageSharp/Memory/Buffer2DExtensions.cs b/src/ImageSharp/Memory/Buffer2DExtensions.cs index 2eb05ea935..e7ac64eda0 100644 --- a/src/ImageSharp/Memory/Buffer2DExtensions.cs +++ b/src/ImageSharp/Memory/Buffer2DExtensions.cs @@ -22,7 +22,7 @@ public static class Buffer2DExtensions where T : struct { Guard.NotNull(buffer, nameof(buffer)); - return buffer.FastMemoryGroup.View; + return buffer.FastMemoryGroup.View!; } /// diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index 8d6465389f..a7957a9871 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -43,7 +43,7 @@ public sealed class Buffer2D : IDisposable /// Gets the backing . /// /// The MemoryGroup. - public IMemoryGroup MemoryGroup => this.FastMemoryGroup.View; + public IMemoryGroup? MemoryGroup => this.FastMemoryGroup.View; /// /// Gets the backing without the view abstraction. @@ -138,7 +138,7 @@ public sealed class Buffer2D : IDisposable { DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y)); DebugGuard.MustBeLessThan(y, this.Height, nameof(y)); - return this.FastMemoryGroup.View.GetBoundedMemorySlice(y * (long)this.Width, this.Width); + return this.FastMemoryGroup.View!.GetBoundedMemorySlice(y * (long)this.Width, this.Width); } /// diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs index 4656e6ef95..b4b1ffc6f4 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupExtensions.cs @@ -106,7 +106,7 @@ internal static class MemoryGroupExtensions } } - internal static void CopyTo(this IMemoryGroup source, IMemoryGroup target) + internal static void CopyTo(this IMemoryGroup? source, IMemoryGroup? target) where T : struct { Guard.NotNull(source, nameof(source)); diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupSpanCache.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupSpanCache.cs index 36abfba886..dd3e67c371 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupSpanCache.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupSpanCache.cs @@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp.Memory; internal unsafe struct MemoryGroupSpanCache { public SpanCacheMode Mode; - public byte[] SingleArray; + public byte[]? SingleArray; public void* SinglePointer; public void*[] MultiPointer; diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs index 54e347e463..76371e6744 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroupView{T}.cs @@ -1,9 +1,9 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Collections; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory; @@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp.Memory; internal class MemoryGroupView : IMemoryGroup where T : struct { - private MemoryGroup owner; + private MemoryGroup? owner; private readonly MemoryOwnerWrapper[] memoryWrappers; public MemoryGroupView(MemoryGroup owner) @@ -63,6 +63,7 @@ internal class MemoryGroupView : IMemoryGroup } } + [MemberNotNullWhen(true, nameof(owner))] public bool IsValid => this.owner != null; public Memory this[int index] @@ -99,6 +100,7 @@ internal class MemoryGroupView : IMemoryGroup this.owner = null; } + [MemberNotNull(nameof(owner))] private void EnsureIsValid() { if (!this.IsValid) diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs index 950e2a019e..03a2fb78f7 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs @@ -48,6 +48,6 @@ internal abstract partial class MemoryGroup return ((IList>)this.source).GetEnumerator(); } - public override void Dispose() => this.View.Invalidate(); + public override void Dispose() => this.View?.Invalidate(); } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index e5bc91c983..664c137c13 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -1,8 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; @@ -15,8 +15,8 @@ internal abstract partial class MemoryGroup /// public sealed class Owned : MemoryGroup, IEnumerable> { - private IMemoryOwner[] memoryOwners; - private RefCountedMemoryLifetimeGuard groupLifetimeGuard; + private IMemoryOwner[]? memoryOwners; + private RefCountedMemoryLifetimeGuard? groupLifetimeGuard; public Owned(IMemoryOwner[] memoryOwners, int bufferLength, long totalLength, bool swappable) : base(bufferLength, totalLength) @@ -123,7 +123,7 @@ internal abstract partial class MemoryGroup public override void RecreateViewAfterSwap() { - this.View.Invalidate(); + this.View?.Invalidate(); this.View = new MemoryGroupView(this); } @@ -141,7 +141,7 @@ internal abstract partial class MemoryGroup return; } - this.View.Invalidate(); + this.View?.Invalidate(); if (this.groupLifetimeGuard != null) { @@ -149,7 +149,7 @@ internal abstract partial class MemoryGroup } else { - foreach (IMemoryOwner memoryOwner in this.memoryOwners) + foreach (IMemoryOwner memoryOwner in this.memoryOwners!) { memoryOwner.Dispose(); } @@ -161,6 +161,7 @@ internal abstract partial class MemoryGroup } [MethodImpl(InliningOptions.ShortMethod)] + [MemberNotNull(nameof(memoryOwners))] private void EnsureNotDisposed() { if (this.memoryOwners is null) @@ -170,6 +171,7 @@ internal abstract partial class MemoryGroup } [MethodImpl(MethodImplOptions.NoInlining)] + [DoesNotReturn] private static void ThrowObjectDisposedException() => throw new ObjectDisposedException(nameof(MemoryGroup)); // When the MemoryGroup points to multiple buffers via `groupLifetimeGuard`, diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 1ef30e354c..237cbad7a7 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -1,9 +1,9 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -#nullable disable using System.Buffers; using System.Collections; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory.Internals; @@ -41,7 +41,7 @@ internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable /// public bool IsValid { get; private set; } = true; - public MemoryGroupView View { get; private set; } + public MemoryGroupView? View { get; private set; } /// public abstract Memory this[int index] { get; } @@ -150,7 +150,7 @@ internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable long totalLengthInElements, int bufferAlignmentInElements, AllocationOptions options, - out MemoryGroup memoryGroup) + [NotNullWhen(true)] out MemoryGroup? memoryGroup) { Guard.NotNull(pool, nameof(pool)); Guard.MustBeGreaterThanOrEqualTo(totalLengthInElements, 0, nameof(totalLengthInElements)); @@ -188,7 +188,7 @@ internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable bufferCount++; } - UnmanagedMemoryHandle[] arrays = pool.Rent(bufferCount); + UnmanagedMemoryHandle[]? arrays = pool.Rent(bufferCount); if (arrays == null) { From dfde8d8bf521681b8638980253f0c4ae5b88bb4f Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 17:11:25 +0100 Subject: [PATCH 06/12] Resolve PR comments --- .../Memory/Allocators/Internals/Gen2GcCallback.cs | 4 ++-- .../Memory/Allocators/Internals/ManagedBufferBase.cs | 2 +- .../Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs | 6 +++++- src/ImageSharp/Memory/Buffer2D{T}.cs | 2 +- .../Memory/DiscontiguousBuffers/MemoryGroup{T}.cs | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs b/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs index c0737a1140..6f48c56696 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/Gen2GcCallback.cs @@ -42,7 +42,7 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject // Execute the callback method. try { - if (this.callback1 is not null && !this.callback1(targetObj)) + if (!this.callback1!(targetObj)) { // If the callback returns false, this callback object is no longer needed. this.weakTargetObj.Free(); @@ -63,7 +63,7 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject // Execute the callback method. try { - if (this.callback0 is not null && !this.callback0()) + if (!this.callback0!()) { // If the callback returns false, this callback object is no longer needed. return; diff --git a/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs b/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs index be4cb8c8e4..a18bd34474 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/ManagedBufferBase.cs @@ -43,5 +43,5 @@ internal abstract class ManagedBufferBase : MemoryManager /// Gets the object that should be pinned. /// /// The pinnable . - protected abstract object? GetPinnableObject(); + protected abstract object GetPinnableObject(); } diff --git a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs index 1d73b2ddf2..4952c6b5e4 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs @@ -40,7 +40,11 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted return MemoryMarshal.Cast(this.Array.AsSpan(0, this.lengthInBytes)); } - protected override object? GetPinnableObject() => this.Array; + protected override object GetPinnableObject() + { + Guard.NotNull(this.Array); + return this.Array; + } public void AddRef() { diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index a7957a9871..8c9093a35b 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -43,7 +43,7 @@ public sealed class Buffer2D : IDisposable /// Gets the backing . /// /// The MemoryGroup. - public IMemoryGroup? MemoryGroup => this.FastMemoryGroup.View; + public IMemoryGroup MemoryGroup => this.FastMemoryGroup.View; /// /// Gets the backing without the view abstraction. diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 237cbad7a7..d63fd2076c 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -41,7 +41,7 @@ internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable /// public bool IsValid { get; private set; } = true; - public MemoryGroupView? View { get; private set; } + public MemoryGroupView View { get; private set; } = null!; /// public abstract Memory this[int index] { get; } From 12cf563c5c54dd6feca7d3acaa0c72d994ae65d1 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 17:15:42 +0100 Subject: [PATCH 07/12] Use CheckDisposed --- .../Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs index 4952c6b5e4..609bb34a56 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs @@ -3,6 +3,7 @@ using System.Buffers; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -42,7 +43,7 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted protected override object GetPinnableObject() { - Guard.NotNull(this.Array); + this.CheckDisposed(); return this.Array; } @@ -55,6 +56,7 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted public void ReleaseRef() => this.lifetimeGuard.ReleaseRef(); [Conditional("DEBUG")] + [MemberNotNull(nameof(Array))] private void CheckDisposed() { if (this.Array == null) From 4efd49295a227ac57509db045774c07ee4283825 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 17:56:21 +0100 Subject: [PATCH 08/12] Resolve PR issues --- .../Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs | 5 +++-- .../Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs index 609bb34a56..f9434ee941 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs @@ -67,7 +67,7 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted private sealed class LifetimeGuard : RefCountedMemoryLifetimeGuard { - private byte[] array; + private byte[]? array; public LifetimeGuard(byte[] array) => this.array = array; @@ -78,7 +78,8 @@ internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted // This is not ideal, but subsequent leaks will end up returning arrays to per-cpu buckets, // meaning likely a different bucket than it was rented from, // but this is PROBABLY better than not returning the arrays at all. - ArrayPool.Shared.Return(this.array); + ArrayPool.Shared.Return(this.array!); + this.array = null; } } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs index 03a2fb78f7..950e2a019e 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Consumed.cs @@ -48,6 +48,6 @@ internal abstract partial class MemoryGroup return ((IList>)this.source).GetEnumerator(); } - public override void Dispose() => this.View?.Invalidate(); + public override void Dispose() => this.View.Invalidate(); } } From 12421030c2846ceee2e886f397b9c84d7bc9cb1a Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 19:42:05 +0100 Subject: [PATCH 09/12] Update src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs Co-authored-by: Anton Firszov --- .../Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index 664c137c13..eab90ee5a7 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -123,7 +123,7 @@ internal abstract partial class MemoryGroup public override void RecreateViewAfterSwap() { - this.View?.Invalidate(); + this.View.Invalidate(); this.View = new MemoryGroupView(this); } From 8f18e3596089a237d7a9bf59cbb9997b4219af5a Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 19:42:13 +0100 Subject: [PATCH 10/12] Update src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs Co-authored-by: Anton Firszov --- .../Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index eab90ee5a7..9da0139e6e 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -141,7 +141,7 @@ internal abstract partial class MemoryGroup return; } - this.View?.Invalidate(); + this.View.Invalidate(); if (this.groupLifetimeGuard != null) { From 6e70477cf2e477c1fbc981a1a31e68c6ff9399e2 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 19:42:21 +0100 Subject: [PATCH 11/12] Update src/ImageSharp/Memory/Buffer2D{T}.cs Co-authored-by: Anton Firszov --- src/ImageSharp/Memory/Buffer2D{T}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index 8c9093a35b..8d6465389f 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -138,7 +138,7 @@ public sealed class Buffer2D : IDisposable { DebugGuard.MustBeGreaterThanOrEqualTo(y, 0, nameof(y)); DebugGuard.MustBeLessThan(y, this.Height, nameof(y)); - return this.FastMemoryGroup.View!.GetBoundedMemorySlice(y * (long)this.Width, this.Width); + return this.FastMemoryGroup.View.GetBoundedMemorySlice(y * (long)this.Width, this.Width); } /// From 9c3d68f1f55f87d44822f91a98e053847218e674 Mon Sep 17 00:00:00 2001 From: Stefan Nikolei Date: Wed, 25 Jan 2023 19:42:28 +0100 Subject: [PATCH 12/12] Update src/ImageSharp/Memory/Buffer2DExtensions.cs Co-authored-by: Anton Firszov --- src/ImageSharp/Memory/Buffer2DExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/Buffer2DExtensions.cs b/src/ImageSharp/Memory/Buffer2DExtensions.cs index e7ac64eda0..2eb05ea935 100644 --- a/src/ImageSharp/Memory/Buffer2DExtensions.cs +++ b/src/ImageSharp/Memory/Buffer2DExtensions.cs @@ -22,7 +22,7 @@ public static class Buffer2DExtensions where T : struct { Guard.NotNull(buffer, nameof(buffer)); - return buffer.FastMemoryGroup.View!; + return buffer.FastMemoryGroup.View; } ///