From 92b82779ac8e7a032989533bb9a01ef092186b2e Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Thu, 28 Mar 2024 02:36:45 +0100 Subject: [PATCH 1/6] Limit all allocations --- .../Memory/Allocators/MemoryAllocator.cs | 51 ++++++++++++++++--- .../Allocators/MemoryAllocatorOptions.cs | 21 +++++++- .../Allocators/SimpleGcMemoryAllocator.cs | 10 +++- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 25 ++++----- .../Memory/InvalidMemoryOperationException.cs | 6 +++ 5 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 2bd9cb5eef..194449cfc1 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -2,6 +2,8 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory; @@ -10,6 +12,8 @@ namespace SixLabors.ImageSharp.Memory; /// public abstract class MemoryAllocator { + private const int OneGigabyte = 1 << 30; + /// /// Gets the default platform-specific global instance that /// serves as the default value for . @@ -20,6 +24,12 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); + internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? + 4L * OneGigabyte : + OneGigabyte; + + internal int SingleBufferAllocationLimitBytes { get; private set; } = OneGigabyte; + /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. /// @@ -30,16 +40,24 @@ public abstract class MemoryAllocator /// Creates a default instance of a optimized for the executing platform. /// /// The . - public static MemoryAllocator Create() => - new UniformUnmanagedMemoryPoolMemoryAllocator(null); + public static MemoryAllocator Create() => Create(default); /// /// Creates the default using the provided options. /// /// The . /// The . - public static MemoryAllocator Create(MemoryAllocatorOptions options) => - new UniformUnmanagedMemoryPoolMemoryAllocator(options.MaximumPoolSizeMegabytes); + public static MemoryAllocator Create(MemoryAllocatorOptions options) + { + UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(options.MaximumPoolSizeMegabytes); + if (options.AllocationLimitMegabytes.HasValue) + { + allocator.MemoryGroupAllocationLimitBytes = options.AllocationLimitMegabytes.Value * 1024 * 1024; + allocator.SingleBufferAllocationLimitBytes = (int)Math.Min(allocator.SingleBufferAllocationLimitBytes, allocator.MemoryGroupAllocationLimitBytes); + } + + return allocator; + } /// /// Allocates an , holding a of length . @@ -69,10 +87,31 @@ public virtual void ReleaseRetainedResources() /// The . /// A new . /// Thrown when 'blockAlignment' converted to bytes is greater than the buffer capacity of the allocator. - internal virtual MemoryGroup AllocateGroup( + internal MemoryGroup AllocateGroup( long totalLength, int bufferAlignment, AllocationOptions options = AllocationOptions.None) where T : struct - => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); + { + long totalLengthInBytes = totalLength * Unsafe.SizeOf(); + if (totalLengthInBytes < 0) + { + ThrowNotRepresentable(); + } + + if (totalLengthInBytes > this.MemoryGroupAllocationLimitBytes) + { + InvalidMemoryOperationException.ThrowAllocationOverLimitException(totalLengthInBytes, this.MemoryGroupAllocationLimitBytes); + } + + return this.AllocateGroupCore(totalLengthInBytes, totalLength, bufferAlignment, options); + + [DoesNotReturn] + static void ThrowNotRepresentable() => + throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); + } + + internal virtual MemoryGroup AllocateGroupCore(long totalLengthInElements, long totalLengthInBytes, int bufferAlignment, AllocationOptions options) + where T : struct + => MemoryGroup.Allocate(this, totalLengthInElements, bufferAlignment, options); } diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index 5a821fd04a..d9ba62c1ef 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. namespace SixLabors.ImageSharp.Memory; @@ -9,6 +9,7 @@ namespace SixLabors.ImageSharp.Memory; public struct MemoryAllocatorOptions { private int? maximumPoolSizeMegabytes; + private int? allocationLimitMegabytes; /// /// Gets or sets a value defining the maximum size of the 's internal memory pool @@ -27,4 +28,22 @@ public int? MaximumPoolSizeMegabytes this.maximumPoolSizeMegabytes = value; } } + + /// + /// Gets or sets a value defining the maximum (discontiguous) buffer size that can be allocated by the allocator in Megabytes. + /// means platform default: 1GB on 32-bit processes, 4GB on 64-bit processes. + /// + public int? AllocationLimitMegabytes + { + get => this.allocationLimitMegabytes; + set + { + if (value.HasValue) + { + Guard.MustBeGreaterThan(value.Value, 0, nameof(this.AllocationLimitMegabytes)); + } + + this.allocationLimitMegabytes = value; + } + } } diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 41730d9678..e604621954 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -1,7 +1,8 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. using System.Buffers; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; namespace SixLabors.ImageSharp.Memory; @@ -19,6 +20,13 @@ public override IMemoryOwner Allocate(int length, AllocationOptions option { Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + int lengthInBytes = length * Unsafe.SizeOf(); + + if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + { + InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); + } + return new BasicArrayBuffer(new T[length]); } } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 0bae193632..585d4717ac 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -2,6 +2,7 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; @@ -86,6 +87,11 @@ public override IMemoryOwner Allocate( Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); + if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + { + InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); + } + if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) { var buffer = new SharedArrayPoolBuffer(length); @@ -111,20 +117,15 @@ public override IMemoryOwner Allocate( } /// - internal override MemoryGroup AllocateGroup( - long totalLength, + internal override MemoryGroup AllocateGroupCore( + long totalLengthInElements, + long totalLengthInBytes, int bufferAlignment, AllocationOptions options = AllocationOptions.None) { - long totalLengthInBytes = totalLength * Unsafe.SizeOf(); - if (totalLengthInBytes < 0) - { - throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); - } - if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer((int)totalLength); + var buffer = new SharedArrayPoolBuffer((int)totalLengthInElements); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } @@ -134,18 +135,18 @@ internal override MemoryGroup AllocateGroup( UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) { - UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLength, options.Has(AllocationOptions.Clean)); + UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLengthInElements, options.Has(AllocationOptions.Clean)); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } } // 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, totalLengthInElements, bufferAlignment, options, out MemoryGroup? poolGroup)) { return poolGroup; } - return MemoryGroup.Allocate(this.nonPoolAllocator, totalLength, bufferAlignment, options); + return MemoryGroup.Allocate(this.nonPoolAllocator, totalLengthInElements, bufferAlignment, options); } public override void ReleaseRetainedResources() => this.pool.Release(); diff --git a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs index 6a55472236..0dc8106b6b 100644 --- a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs +++ b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Diagnostics.CodeAnalysis; + namespace SixLabors.ImageSharp.Memory; /// @@ -24,4 +26,8 @@ public InvalidMemoryOperationException(string message) public InvalidMemoryOperationException() { } + + [DoesNotReturn] + internal static void ThrowAllocationOverLimitException(long length, long limit) => + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={length} that exceeded the limit {limit}."); } From 7dd3c43a00fffc7c9e70b6ed53e24c936f4b38a8 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Thu, 28 Mar 2024 02:40:52 +0100 Subject: [PATCH 2/6] reduce UnmanagedMemoryHandle.MaxAllocationAttempts --- .../Memory/Allocators/Internals/UnmanagedMemoryHandle.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs index c13fa754e2..6b31cadf4f 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs @@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Memory.Internals; internal struct UnmanagedMemoryHandle : IEquatable { // Number of allocation re-attempts when detecting OutOfMemoryException. - private const int MaxAllocationAttempts = 1000; + private const int MaxAllocationAttempts = 10; // Track allocations for testing purposes: private static int totalOutstandingHandles; From 63d4b2028a370debe463077fb7fa7fe8469676f7 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 3 Apr 2024 15:24:05 +1000 Subject: [PATCH 3/6] Fix args and add tests --- .../ComponentProcessors/ComponentProcessor.cs | 2 +- .../Components/Encoder/ComponentProcessor.cs | 2 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++ src/ImageSharp/Formats/Tga/TgaDecoderCore.cs | 2 +- .../Memory/Allocators/MemoryAllocator.cs | 21 +++++--------- .../Allocators/MemoryAllocatorOptions.cs | 4 +-- .../Allocators/SimpleGcMemoryAllocator.cs | 10 ++++--- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 14 +++++---- .../DiscontiguousBuffers/MemoryGroup{T}.cs | 13 +++++---- .../Memory/InvalidMemoryOperationException.cs | 11 ++++++- .../Memory/MemoryAllocatorExtensions.cs | 14 ++++----- .../Transforms/Resize/ResizeKernelMap.cs | 2 +- .../Transforms/Resize/ResizeWorker.cs | 2 +- .../Formats/Bmp/BmpDecoderTests.cs | 9 ++++++ .../Formats/Jpg/JpegDecoderTests.cs | 18 +++--------- .../SimpleGcMemoryAllocatorTests.cs | 16 ++++++---- ...niformUnmanagedPoolMemoryAllocatorTests.cs | 13 ++++++++- .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 29 +++++++++++++++++-- .../MemoryGroupTests.Allocate.cs | 12 ++++++-- tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Bmp/issue-2696.bmp | 3 ++ 21 files changed, 131 insertions(+), 71 deletions(-) create mode 100644 tests/Images/Input/Bmp/issue-2696.bmp diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs index f0cc4d5e82..3bdaf2dc27 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs @@ -16,7 +16,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, JpegFrame frame, Size this.Component = component; this.BlockAreaSize = component.SubSamplingDivisors * blockSize; - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, this.BlockAreaSize.Height); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs index c33a8a1968..9346b11e7b 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs @@ -28,7 +28,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, Component component, this.blockAreaSize = component.SubSamplingDivisors * 8; // alignment of 8 so each block stride can be sampled from a single 'ref pointer' - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, 8, diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a96c53c104..abd5cadeb1 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1968,6 +1968,9 @@ private IMemoryOwner ReadChunkData(int length) } // We rent the buffer here to return it afterwards in Decode() + // We don't want to throw a degenerated memory exception here as we want to allow partial decoding + // so limit the length. + length = (int)Math.Min(length, this.currentStream.Length - this.currentStream.Position); IMemoryOwner buffer = this.configuration.MemoryAllocator.Allocate(length, AllocationOptions.Clean); this.currentStream.Read(buffer.GetSpan(), 0, length); diff --git a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs index 26e057bff9..34f4c2bcf1 100644 --- a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs +++ b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs @@ -84,7 +84,7 @@ public Image Decode(BufferedReadStream stream, CancellationToken throw new UnknownImageFormatException("Width or height cannot be 0"); } - Image image = Image.CreateUninitialized(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); + Image image = new(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); if (this.fileHeader.ColorMapType == 1) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 194449cfc1..b56bedd045 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Buffers; -using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory; @@ -24,9 +23,7 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); - internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? - 4L * OneGigabyte : - OneGigabyte; + internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? 4L * OneGigabyte : OneGigabyte; internal int SingleBufferAllocationLimitBytes { get; private set; } = OneGigabyte; @@ -82,6 +79,7 @@ public virtual void ReleaseRetainedResources() /// /// Allocates a . /// + /// The type of element to allocate. /// The total length of the buffer. /// The expected alignment (eg. to make sure image rows fit into single buffers). /// The . @@ -93,22 +91,19 @@ internal MemoryGroup AllocateGroup( AllocationOptions options = AllocationOptions.None) where T : struct { - long totalLengthInBytes = totalLength * Unsafe.SizeOf(); - if (totalLengthInBytes < 0) + if (totalLength < 0) { - ThrowNotRepresentable(); + InvalidMemoryOperationException.ThrowNegativeAllocationException(totalLength); } - if (totalLengthInBytes > this.MemoryGroupAllocationLimitBytes) + ulong totalLengthInBytes = (ulong)totalLength * (ulong)Unsafe.SizeOf(); + if (totalLengthInBytes > (ulong)this.MemoryGroupAllocationLimitBytes) { InvalidMemoryOperationException.ThrowAllocationOverLimitException(totalLengthInBytes, this.MemoryGroupAllocationLimitBytes); } - return this.AllocateGroupCore(totalLengthInBytes, totalLength, bufferAlignment, options); - - [DoesNotReturn] - static void ThrowNotRepresentable() => - throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); + // Cast to long is safe because we already checked that the total length is within the limit. + return this.AllocateGroupCore(totalLength, (long)totalLengthInBytes, bufferAlignment, options); } internal virtual MemoryGroup AllocateGroupCore(long totalLengthInElements, long totalLengthInBytes, int bufferAlignment, AllocationOptions options) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index d9ba62c1ef..9f25816609 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -17,7 +17,7 @@ public struct MemoryAllocatorOptions /// public int? MaximumPoolSizeMegabytes { - get => this.maximumPoolSizeMegabytes; + readonly get => this.maximumPoolSizeMegabytes; set { if (value.HasValue) @@ -35,7 +35,7 @@ public int? MaximumPoolSizeMegabytes /// public int? AllocationLimitMegabytes { - get => this.allocationLimitMegabytes; + readonly get => this.allocationLimitMegabytes; set { if (value.HasValue) diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index e604621954..675afe8b9f 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -18,11 +18,13 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); - - int lengthInBytes = length * Unsafe.SizeOf(); + if (length < 0) + { + InvalidMemoryOperationException.ThrowNegativeAllocationException(length); + } - if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf(); + if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes) { InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 585d4717ac..621073a3db 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Buffers; -using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; @@ -84,15 +83,18 @@ public override IMemoryOwner Allocate( int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); - int lengthInBytes = length * Unsafe.SizeOf(); + if (length < 0) + { + InvalidMemoryOperationException.ThrowNegativeAllocationException(length); + } - if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf(); + if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes) { InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); } - if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) + if (lengthInBytes <= (ulong)this.sharedArrayPoolThresholdInBytes) { var buffer = new SharedArrayPoolBuffer(length); if (options.Has(AllocationOptions.Clean)) @@ -103,7 +105,7 @@ public override IMemoryOwner Allocate( return buffer; } - if (lengthInBytes <= this.poolBufferSizeInBytes) + if (lengthInBytes <= (ulong)this.poolBufferSizeInBytes) { UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 9695c7f98f..03c29a7231 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -83,15 +83,16 @@ public static MemoryGroup Allocate( { int bufferCapacityInBytes = allocator.GetBufferCapacityInBytes(); Guard.NotNull(allocator, nameof(allocator)); - Guard.MustBeGreaterThanOrEqualTo(totalLengthInElements, 0, nameof(totalLengthInElements)); - Guard.MustBeGreaterThanOrEqualTo(bufferAlignmentInElements, 0, nameof(bufferAlignmentInElements)); - int blockCapacityInElements = bufferCapacityInBytes / ElementSize; + if (totalLengthInElements < 0) + { + InvalidMemoryOperationException.ThrowNegativeAllocationException(totalLengthInElements); + } - if (bufferAlignmentInElements > blockCapacityInElements) + int blockCapacityInElements = bufferCapacityInBytes / ElementSize; + if (bufferAlignmentInElements < 0 || bufferAlignmentInElements > blockCapacityInElements) { - throw new InvalidMemoryOperationException( - $"The buffer capacity of the provided MemoryAllocator is insufficient for the requested buffer alignment: {bufferAlignmentInElements}."); + InvalidMemoryOperationException.ThrowInvalidAlignmentException(bufferAlignmentInElements); } if (totalLengthInElements == 0) diff --git a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs index 0dc8106b6b..81210f13db 100644 --- a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs +++ b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs @@ -28,6 +28,15 @@ public InvalidMemoryOperationException() } [DoesNotReturn] - internal static void ThrowAllocationOverLimitException(long length, long limit) => + internal static void ThrowNegativeAllocationException(long length) => + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}."); + + [DoesNotReturn] + internal static void ThrowInvalidAlignmentException(long alignment) => + throw new InvalidMemoryOperationException( + $"The buffer capacity of the provided MemoryAllocator is insufficient for the requested buffer alignment: {alignment}."); + + [DoesNotReturn] + internal static void ThrowAllocationOverLimitException(ulong length, long limit) => throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={length} that exceeded the limit {limit}."); } diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index ff306e1e45..e0dc8aab36 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -18,20 +18,20 @@ public static class MemoryAllocatorExtensions /// The memory allocator. /// The buffer width. /// The buffer height. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, int width, int height, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct { long groupLength = (long)width * height; MemoryGroup memoryGroup; - if (preferContiguosImageBuffers && groupLength < int.MaxValue) + if (preferContiguousImageBuffers && groupLength < int.MaxValue) { IMemoryOwner buffer = memoryAllocator.Allocate((int)groupLength, options); memoryGroup = MemoryGroup.CreateContiguous(buffer, false); @@ -69,16 +69,16 @@ public static Buffer2D Allocate2D( /// The type of buffer items to allocate. /// The memory allocator. /// The buffer size. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, Size size, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct => - Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguosImageBuffers, options); + Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguousImageBuffers, options); /// /// Allocates a buffer of value type objects interpreted as a 2D region @@ -96,7 +96,7 @@ public static Buffer2D Allocate2D( where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, false, options); - internal static Buffer2D Allocate2DOveraligned( + internal static Buffer2D Allocate2DOverAligned( this MemoryAllocator memoryAllocator, int width, int height, diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index c1907bb520..da26cbefcd 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -50,7 +50,7 @@ private ResizeKernelMap( this.sourceLength = sourceLength; this.DestinationLength = destinationLength; this.MaxDiameter = (radius * 2) + 1; - this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguosImageBuffers: true, AllocationOptions.Clean); + this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguousImageBuffers: true, AllocationOptions.Clean); this.pinHandle = this.data.DangerousGetSingleMemory().Pin(); this.kernels = new ResizeKernel[destinationLength]; this.tempValues = new double[this.MaxDiameter]; diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index cce27a401c..fd13d7b5a6 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -83,7 +83,7 @@ public ResizeWorker( this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, targetWorkingRect.Width, - preferContiguosImageBuffers: true, + preferContiguousImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index e76f21d0e9..be2fa4b685 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -558,4 +558,13 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder.Instance); + }); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 6a94a98ac6..2fe4282607 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -338,21 +338,11 @@ public void Issue2564_DecodeWorks(TestImageProvider provider) } [Theory] - [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)] - public void DecodeHang(TestImageProvider provider) + [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.Rgb24)] + public void DecodeHang_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel - { - if (TestEnvironment.IsWindows && - TestEnvironment.RunsOnCI) - { - // Windows CI runs consistently fail with OOM. - return; - } - - using Image image = provider.GetImage(JpegDecoder.Instance); - Assert.Equal(65503, image.Width); - Assert.Equal(65503, image.Height); - } + => Assert.Throws( + () => { using Image image = provider.GetImage(JpegDecoder.Instance); }); // https://github.com/SixLabors/ImageSharp/issues/2517 [Theory] diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 780ba7f20e..665f34a342 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -19,13 +19,17 @@ public BufferTests() protected SimpleGcMemoryAllocator MemoryAllocator { get; } = new SimpleGcMemoryAllocator(); - [Theory] - [InlineData(-1)] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + public static TheoryData InvalidLengths { get; set; } = new() { - ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); - Assert.Equal("length", ex.ParamName); - } + { -1 }, + { (1 << 30) + 1 } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length) + => Assert.Throws( + () => this.MemoryAllocator.Allocate(length)); [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 7e7c136635..6187b536ed 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -111,9 +111,20 @@ public void AllocateGroup_MultipleTimes_ExceedPoolLimit() public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperationException() { var allocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null); - Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); + Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); } + public static TheoryData InvalidLengths { get; set; } = new() + { + { -1 }, + { (1 << 30) + 1 } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length) + => Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); + [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() { diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 5364de0652..3ef4fc540c 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -4,6 +4,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Memory; @@ -72,11 +73,11 @@ public void Construct_PreferContiguousImageBuffers_AllocatesContiguousRegardless using Buffer2D buffer = useSizeOverload ? this.MemoryAllocator.Allocate2D( new Size(200, 200), - preferContiguosImageBuffers: true) : + preferContiguousImageBuffers: true) : this.MemoryAllocator.Allocate2D( 200, 200, - preferContiguosImageBuffers: true); + preferContiguousImageBuffers: true); Assert.Equal(1, buffer.FastMemoryGroup.Count); Assert.Equal(200 * 200, buffer.FastMemoryGroup.TotalLength); } @@ -87,7 +88,7 @@ public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int { this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -337,4 +338,26 @@ public void PublicMemoryGroup_IsMemoryGroupView() Assert.False(mgBefore.IsValid); Assert.NotSame(mgBefore, buffer1.MemoryGroup); } + + public static TheoryData InvalidLengths { get; set; } = new() + { + { new(-1, -1) }, + { new(32768, 32769) }, + { new(32769, 32768) } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(size.Width, size.Height)); + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_Size(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(size))); + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs index 936e482cb3..4c7de5412c 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. using System.Runtime.CompilerServices; @@ -219,11 +219,17 @@ public void MemoryAllocatorIsUtilizedCorrectly(AllocationOptions allocationOptio [StructLayout(LayoutKind.Sequential, Size = 5)] internal struct S5 { - public override string ToString() => "S5"; + public override readonly string ToString() => nameof(S5); } [StructLayout(LayoutKind.Sequential, Size = 4)] internal struct S4 { - public override string ToString() => "S4"; + public override readonly string ToString() => nameof(S4); +} + +[StructLayout(LayoutKind.Explicit, Size = 512)] +internal struct S512 +{ + public override readonly string ToString() => nameof(S512); } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fe633fddc3..42778e6f11 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -439,6 +439,8 @@ public static class Bmp public const string Rgba321010102 = "Bmp/rgba32-1010102.bmp"; public const string RgbaAlphaBitfields = "Bmp/rgba32abf.bmp"; + public const string Issue2696 = "Bmp/issue-2696.bmp"; + public const string BlackWhitePalletDataMatrix = "Bmp/bit1datamatrix.bmp"; public static readonly string[] BitFields = diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp new file mode 100644 index 0000000000..4a42e456cb --- /dev/null +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:de7e7ec0454a55f6a76c859f356b240a3d4cb56ca50dfa209a1813dd09e12076 +size 143 From da497882fed5080ef78fd65364e14d34be6b9a22 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Apr 2024 09:18:23 +1000 Subject: [PATCH 4/6] Revert additional changes --- .../ComponentProcessors/ComponentProcessor.cs | 2 +- .../Jpeg/Components/Encoder/ComponentProcessor.cs | 2 +- src/ImageSharp/Formats/Tga/TgaDecoderCore.cs | 2 +- .../Memory/Allocators/MemoryAllocatorOptions.cs | 4 ++-- src/ImageSharp/Memory/MemoryAllocatorExtensions.cs | 14 +++++++------- .../Transforms/Resize/ResizeKernelMap.cs | 2 +- .../Processors/Transforms/Resize/ResizeWorker.cs | 2 +- tests/ImageSharp.Tests/Memory/Buffer2DTests.cs | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs index 3bdaf2dc27..f0cc4d5e82 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs @@ -16,7 +16,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, JpegFrame frame, Size this.Component = component; this.BlockAreaSize = component.SubSamplingDivisors * blockSize; - this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( + this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, this.BlockAreaSize.Height); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs index 9346b11e7b..c33a8a1968 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs @@ -28,7 +28,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, Component component, this.blockAreaSize = component.SubSamplingDivisors * 8; // alignment of 8 so each block stride can be sampled from a single 'ref pointer' - this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( + this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, 8, diff --git a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs index 34f4c2bcf1..26e057bff9 100644 --- a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs +++ b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs @@ -84,7 +84,7 @@ public Image Decode(BufferedReadStream stream, CancellationToken throw new UnknownImageFormatException("Width or height cannot be 0"); } - Image image = new(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); + Image image = Image.CreateUninitialized(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); if (this.fileHeader.ColorMapType == 1) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index 9f25816609..d9ba62c1ef 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -17,7 +17,7 @@ public struct MemoryAllocatorOptions /// public int? MaximumPoolSizeMegabytes { - readonly get => this.maximumPoolSizeMegabytes; + get => this.maximumPoolSizeMegabytes; set { if (value.HasValue) @@ -35,7 +35,7 @@ public int? MaximumPoolSizeMegabytes /// public int? AllocationLimitMegabytes { - readonly get => this.allocationLimitMegabytes; + get => this.allocationLimitMegabytes; set { if (value.HasValue) diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index e0dc8aab36..ff306e1e45 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -18,20 +18,20 @@ public static class MemoryAllocatorExtensions /// The memory allocator. /// The buffer width. /// The buffer height. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, int width, int height, - bool preferContiguousImageBuffers, + bool preferContiguosImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct { long groupLength = (long)width * height; MemoryGroup memoryGroup; - if (preferContiguousImageBuffers && groupLength < int.MaxValue) + if (preferContiguosImageBuffers && groupLength < int.MaxValue) { IMemoryOwner buffer = memoryAllocator.Allocate((int)groupLength, options); memoryGroup = MemoryGroup.CreateContiguous(buffer, false); @@ -69,16 +69,16 @@ public static Buffer2D Allocate2D( /// The type of buffer items to allocate. /// The memory allocator. /// The buffer size. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, Size size, - bool preferContiguousImageBuffers, + bool preferContiguosImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct => - Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguousImageBuffers, options); + Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguosImageBuffers, options); /// /// Allocates a buffer of value type objects interpreted as a 2D region @@ -96,7 +96,7 @@ public static Buffer2D Allocate2D( where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, false, options); - internal static Buffer2D Allocate2DOverAligned( + internal static Buffer2D Allocate2DOveraligned( this MemoryAllocator memoryAllocator, int width, int height, diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index da26cbefcd..c1907bb520 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -50,7 +50,7 @@ private ResizeKernelMap( this.sourceLength = sourceLength; this.DestinationLength = destinationLength; this.MaxDiameter = (radius * 2) + 1; - this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguousImageBuffers: true, AllocationOptions.Clean); + this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguosImageBuffers: true, AllocationOptions.Clean); this.pinHandle = this.data.DangerousGetSingleMemory().Pin(); this.kernels = new ResizeKernel[destinationLength]; this.tempValues = new double[this.MaxDiameter]; diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index fd13d7b5a6..cce27a401c 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -83,7 +83,7 @@ public ResizeWorker( this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, targetWorkingRect.Width, - preferContiguousImageBuffers: true, + preferContiguosImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 3ef4fc540c..8ba3bf70a2 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -73,11 +73,11 @@ public void Construct_PreferContiguousImageBuffers_AllocatesContiguousRegardless using Buffer2D buffer = useSizeOverload ? this.MemoryAllocator.Allocate2D( new Size(200, 200), - preferContiguousImageBuffers: true) : + preferContiguosImageBuffers: true) : this.MemoryAllocator.Allocate2D( 200, 200, - preferContiguousImageBuffers: true); + preferContiguosImageBuffers: true); Assert.Equal(1, buffer.FastMemoryGroup.Count); Assert.Equal(200 * 200, buffer.FastMemoryGroup.TotalLength); } @@ -88,7 +88,7 @@ public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int { this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -359,5 +359,5 @@ public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationExcepti [Theory] [MemberData(nameof(InvalidLengths))] public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) - => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOveraligned(size.Width, size.Height, 1)); } From d1cc651e19def81df66779985a50f9372db332bc Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 10 Apr 2024 23:18:45 +0200 Subject: [PATCH 5/6] fix BmpDecoder_ThrowsException_Issue2696 --- tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs | 11 +++++++---- tests/Images/Input/Bmp/issue-2696.bmp | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index be2fa4b685..1ce794e44b 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -563,8 +563,11 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel - => Assert.Throws(() => - { - using Image image = provider.GetImage(BmpDecoder.Instance); - }); + { + InvalidImageContentException ex = Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder.Instance); + }); + Assert.IsType(ex.InnerException); + } } diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp index 4a42e456cb..6770dd9469 100644 --- a/tests/Images/Input/Bmp/issue-2696.bmp +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:de7e7ec0454a55f6a76c859f356b240a3d4cb56ca50dfa209a1813dd09e12076 -size 143 +oid sha256:bc42cda9bac8fc73351ad03bf55214069bb8d31ea5bdd806321a8cc8b56c282e +size 126 From 572366e07806bfb6a269c9a132c9afbd2a9191d0 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 10 Apr 2024 23:33:34 +0200 Subject: [PATCH 6/6] test allocation limits --- ...niformUnmanagedPoolMemoryAllocatorTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 6187b536ed..077d0afed8 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -418,4 +418,28 @@ static void RunTest() _ = MemoryAllocator.Create(); } } + + [Fact] + public void Allocate_OverLimit_ThrowsInvalidMemoryOperationException() + { + MemoryAllocator allocator = MemoryAllocator.Create(new MemoryAllocatorOptions() + { + AllocationLimitMegabytes = 4 + }); + const int oneMb = 1 << 20; + allocator.Allocate(4 * oneMb).Dispose(); // Should work + Assert.Throws(() => allocator.Allocate(5 * oneMb)); + } + + [Fact] + public void AllocateGroup_OverLimit_ThrowsInvalidMemoryOperationException() + { + MemoryAllocator allocator = MemoryAllocator.Create(new MemoryAllocatorOptions() + { + AllocationLimitMegabytes = 4 + }); + const int oneMb = 1 << 20; + allocator.AllocateGroup(4 * oneMb, 1024).Dispose(); // Should work + Assert.Throws(() => allocator.AllocateGroup(5 * oneMb, 1024)); + } }