Merge pull request #2715 from SixLabors/backport/v2-memlimit

antonfirsov · web-flow · commit f21d64188e59 · 2024-04-11T00:26:18.000+02:00
V2 - Limit all memory allocations in the MemoryAllocator layer
diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs
@@ -1504,6 +1504,9 @@ private void SkipChunkDataAndCrc(in PngChunk chunk)
         private IMemoryOwner<byte> 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<byte> buffer = this.Configuration.MemoryAllocator.Allocate<byte>(length, AllocationOptions.Clean);
 
             this.currentStream.Read(buffer.GetSpan(), 0, length);
diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs
@@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Memory.Internals
     internal struct UnmanagedMemoryHandle : IEquatable<UnmanagedMemoryHandle>
     {
         // 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;
diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs
@@ -3,6 +3,8 @@
 
 using System;
 using System.Buffers;
+using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 
 namespace SixLabors.ImageSharp.Memory
 {
@@ -11,6 +13,8 @@ namespace SixLabors.ImageSharp.Memory
     /// </summary>
     public abstract class MemoryAllocator
     {
+        private const int OneGigabyte = 1 << 30;
+
         /// <summary>
         /// Gets the default platform-specific global <see cref="MemoryAllocator"/> instance that
         /// serves as the default value for <see cref="Configuration.MemoryAllocator"/>.
@@ -21,6 +25,10 @@ public abstract class MemoryAllocator
         /// </summary>
         public static MemoryAllocator Default { get; } = Create();
 
+        internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? 4L * OneGigabyte : OneGigabyte;
+
+        internal int SingleBufferAllocationLimitBytes { get; private set; } = OneGigabyte;
+
         /// <summary>
         /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes.
         /// </summary>
@@ -31,16 +39,24 @@ public abstract class MemoryAllocator
         /// Creates a default instance of a <see cref="MemoryAllocator"/> optimized for the executing platform.
         /// </summary>
         /// <returns>The <see cref="MemoryAllocator"/>.</returns>
-        public static MemoryAllocator Create() =>
-            new UniformUnmanagedMemoryPoolMemoryAllocator(null);
+        public static MemoryAllocator Create() => Create(default);
 
         /// <summary>
         /// Creates the default <see cref="MemoryAllocator"/> using the provided options.
         /// </summary>
         /// <param name="options">The <see cref="MemoryAllocatorOptions"/>.</param>
         /// <returns>The <see cref="MemoryAllocator"/>.</returns>
-        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;
+        }
 
         /// <summary>
         /// Allocates an <see cref="IMemoryOwner{T}" />, holding a <see cref="Memory{T}"/> of length <paramref name="length"/>.
@@ -65,16 +81,35 @@ public virtual void ReleaseRetainedResources()
         /// <summary>
         /// Allocates a <see cref="MemoryGroup{T}"/>.
         /// </summary>
+        /// <typeparam name="T">The type of element to allocate.</typeparam>
         /// <param name="totalLength">The total length of the buffer.</param>
         /// <param name="bufferAlignment">The expected alignment (eg. to make sure image rows fit into single buffers).</param>
         /// <param name="options">The <see cref="AllocationOptions"/>.</param>
         /// <returns>A new <see cref="MemoryGroup{T}"/>.</returns>
         /// <exception cref="InvalidMemoryOperationException">Thrown when 'blockAlignment' converted to bytes is greater than the buffer capacity of the allocator.</exception>
-        internal virtual MemoryGroup<T> AllocateGroup<T>(
+        internal MemoryGroup<T> AllocateGroup<T>(
             long totalLength,
             int bufferAlignment,
             AllocationOptions options = AllocationOptions.None)
             where T : struct
-            => MemoryGroup<T>.Allocate(this, totalLength, bufferAlignment, options);
+        {
+            if (totalLength < 0)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={totalLength}.");
+            }
+
+            ulong totalLengthInBytes = (ulong)totalLength * (ulong)Unsafe.SizeOf<T>();
+            if (totalLengthInBytes > (ulong)this.MemoryGroupAllocationLimitBytes)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={totalLengthInBytes} that exceeded the limit {this.MemoryGroupAllocationLimitBytes}.");
+            }
+
+            // Cast to long is safe because we already checked that the total length is within the limit.
+            return this.AllocateGroupCore<T>(totalLength, (long)totalLengthInBytes, bufferAlignment, options);
+        }
+
+        internal virtual MemoryGroup<T> AllocateGroupCore<T>(long totalLengthInElements, long totalLengthInBytes, int bufferAlignment, AllocationOptions options)
+            where T : struct
+            => MemoryGroup<T>.Allocate(this, totalLengthInElements, bufferAlignment, options);
     }
 }
diff --git 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 Apache License, Version 2.0.
 
 namespace SixLabors.ImageSharp.Memory
@@ -9,6 +9,7 @@ namespace SixLabors.ImageSharp.Memory
     public struct MemoryAllocatorOptions
     {
         private int? maximumPoolSizeMegabytes;
+        private int? allocationLimitMegabytes;
 
         /// <summary>
         /// Gets or sets a value defining the maximum size of the <see cref="MemoryAllocator"/>'s internal memory pool
@@ -27,5 +28,23 @@ public int? MaximumPoolSizeMegabytes
                 this.maximumPoolSizeMegabytes = value;
             }
         }
+
+        /// <summary>
+        /// Gets or sets a value defining the maximum (discontiguous) buffer size that can be allocated by the allocator in Megabytes.
+        /// <see langword="null"/> means platform default: 1GB on 32-bit processes, 4GB on 64-bit processes.
+        /// </summary>
+        public int? AllocationLimitMegabytes
+        {
+            readonly 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
@@ -1,7 +1,8 @@
-﻿// Copyright (c) Six Labors.
+// Copyright (c) Six Labors.
 // Licensed under the Apache License, Version 2.0.
 
 using System.Buffers;
+using System.Runtime.CompilerServices;
 using SixLabors.ImageSharp.Memory.Internals;
 
 namespace SixLabors.ImageSharp.Memory
@@ -17,7 +18,16 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator
         /// <inheritdoc />
         public override IMemoryOwner<T> Allocate<T>(int length, AllocationOptions options = AllocationOptions.None)
         {
-            Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length));
+            if (length < 0)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}.");
+            }
+
+            ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf<T>();
+            if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={lengthInBytes} that exceeded the limit {this.SingleBufferAllocationLimitBytes}.");
+            }
 
             return new BasicArrayBuffer<T>(new T[length]);
         }
diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs
@@ -87,10 +87,18 @@ public override IMemoryOwner<T> Allocate<T>(
             int length,
             AllocationOptions options = AllocationOptions.None)
         {
-            Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length));
-            int lengthInBytes = length * Unsafe.SizeOf<T>();
+            if (length < 0)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}.");
+            }
+
+            ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf<T>();
+            if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={lengthInBytes} that exceeded the limit {this.SingleBufferAllocationLimitBytes}.");
+            }
 
-            if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes)
+            if (lengthInBytes <= (ulong)this.sharedArrayPoolThresholdInBytes)
             {
                 var buffer = new SharedArrayPoolBuffer<T>(length);
                 if (options.Has(AllocationOptions.Clean))
@@ -101,7 +109,7 @@ public override IMemoryOwner<T> Allocate<T>(
                 return buffer;
             }
 
-            if (lengthInBytes <= this.poolBufferSizeInBytes)
+            if (lengthInBytes <= (ulong)this.poolBufferSizeInBytes)
             {
                 UnmanagedMemoryHandle mem = this.pool.Rent();
                 if (mem.IsValid)
@@ -115,20 +123,15 @@ public override IMemoryOwner<T> Allocate<T>(
         }
 
         /// <inheritdoc />
-        internal override MemoryGroup<T> AllocateGroup<T>(
-            long totalLength,
+        internal override MemoryGroup<T> AllocateGroupCore<T>(
+            long totalLengthInElements,
+            long totalLengthInBytes,
             int bufferAlignment,
             AllocationOptions options = AllocationOptions.None)
         {
-            long totalLengthInBytes = totalLength * Unsafe.SizeOf<T>();
-            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<T>((int)totalLength);
+                var buffer = new SharedArrayPoolBuffer<T>((int)totalLengthInElements);
                 return MemoryGroup<T>.CreateContiguous(buffer, options.Has(AllocationOptions.Clean));
             }
 
@@ -138,18 +141,18 @@ internal override MemoryGroup<T> AllocateGroup<T>(
                 UnmanagedMemoryHandle mem = this.pool.Rent();
                 if (mem.IsValid)
                 {
-                    UnmanagedBuffer<T> buffer = this.pool.CreateGuardedBuffer<T>(mem, (int)totalLength, options.Has(AllocationOptions.Clean));
+                    UnmanagedBuffer<T> buffer = this.pool.CreateGuardedBuffer<T>(mem, (int)totalLengthInElements, options.Has(AllocationOptions.Clean));
                     return MemoryGroup<T>.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<T>.TryAllocate(this.pool, totalLength, bufferAlignment, options, out MemoryGroup<T> poolGroup))
+            if (MemoryGroup<T>.TryAllocate(this.pool, totalLengthInElements, bufferAlignment, options, out MemoryGroup<T>? poolGroup))
             {
                 return poolGroup;
             }
 
-            return MemoryGroup<T>.Allocate(this.nonPoolAllocator, totalLength, bufferAlignment, options);
+            return MemoryGroup<T>.Allocate(this.nonPoolAllocator, totalLengthInElements, bufferAlignment, options);
         }
 
         public override void ReleaseRetainedResources() => this.pool.Release();
diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs
@@ -20,7 +20,7 @@ namespace SixLabors.ImageSharp.Memory
     /// <typeparam name="T">The element type.</typeparam>
     internal abstract partial class MemoryGroup<T> : IMemoryGroup<T>, IDisposable
         where T : struct
-    {
+    {    
         private static readonly int ElementSize = Unsafe.SizeOf<T>();
 
         private MemoryGroupSpanCache memoryGroupSpanCache;
@@ -43,7 +43,7 @@ private MemoryGroup(int bufferLength, long totalLength)
         /// <inheritdoc />
         public bool IsValid { get; private set; } = true;
 
-        public MemoryGroupView<T> View { get; private set; }
+        public MemoryGroupView<T> View { get; private set; } = null!;
 
         /// <inheritdoc />
         public abstract Memory<T> this[int index] { get; }
@@ -85,12 +85,14 @@ public static MemoryGroup<T> 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)
+            {
+                throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={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}.");
diff --git a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0.
 
 using System;
+using System.Diagnostics.CodeAnalysis;
 
 namespace SixLabors.ImageSharp.Memory
 {
diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs
@@ -619,5 +619,18 @@ public void BmpDecoder_CanDecode_Os2BitmapArray<TPixel>(TestImageProvider<TPixel
                 // image.CompareToOriginal(provider);
             }
         }
+
+        [Theory]
+        [WithFile(Issue2696, PixelTypes.Rgba32)]
+        public void BmpDecoder_ThrowsException_Issue2696<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : unmanaged, IPixel<TPixel>
+        {
+            // On V2 this is throwing InvalidOperationException,
+            // because of the validation logic in BmpInfoHeader.VerifyDimensions().
+            Assert.Throws<InvalidOperationException>(() =>
+                {
+                    using Image<TPixel> image = provider.GetImage(BmpDecoder);
+                });
+        }
     }
 }
diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs
@@ -21,13 +21,17 @@ public BufferTests()
 
         protected SimpleGcMemoryAllocator MemoryAllocator { get; } = new SimpleGcMemoryAllocator();
 
-        [Theory]
-        [InlineData(-1)]
-        public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length)
+        public static TheoryData<int> InvalidLengths { get; set; } = new()
         {
-            ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(() => this.MemoryAllocator.Allocate<BigStruct>(length));
-            Assert.Equal("length", ex.ParamName);
-        }
+            { -1 },
+            { (1 << 30) + 1 }
+        };
+
+        [Theory]
+        [MemberData(nameof(InvalidLengths))]
+        public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length)
+            => Assert.Throws<InvalidMemoryOperationException>(
+                () => this.MemoryAllocator.Allocate<BigStruct>(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
@@ -118,6 +118,17 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati
             Assert.Throws<InvalidMemoryOperationException>(() => allocator.AllocateGroup<S4>(int.MaxValue * (long)int.MaxValue, int.MaxValue));
         }
 
+        public static TheoryData<int> InvalidLengths { get; set; } = new()
+        {
+            { -1 },
+            { (1 << 30) + 1 }
+        };
+
+        [Theory]
+        [MemberData(nameof(InvalidLengths))]
+        public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length)
+            => Assert.Throws<InvalidMemoryOperationException>(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate<S512>(length));
+
         [Fact]
         public unsafe void Allocate_MemoryIsPinnableMultipleTimes()
         {
@@ -387,6 +398,30 @@ private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllo
             }
         }
 
+        [Fact]
+        public void Allocate_OverLimit_ThrowsInvalidMemoryOperationException()
+        {
+            MemoryAllocator allocator = MemoryAllocator.Create(new MemoryAllocatorOptions()
+            {
+                AllocationLimitMegabytes = 4
+            });
+            const int oneMb = 1 << 20;
+            allocator.Allocate<byte>(4 * oneMb).Dispose(); // Should work
+            Assert.Throws<InvalidMemoryOperationException>(() => allocator.Allocate<byte>(5 * oneMb));
+        }
+
+        [Fact]
+        public void AllocateGroup_OverLimit_ThrowsInvalidMemoryOperationException()
+        {
+            MemoryAllocator allocator = MemoryAllocator.Create(new MemoryAllocatorOptions()
+            {
+                AllocationLimitMegabytes = 4
+            });
+            const int oneMb = 1 << 20;
+            allocator.AllocateGroup<byte>(4 * oneMb, 1024).Dispose(); // Should work
+            Assert.Throws<InvalidMemoryOperationException>(() => allocator.AllocateGroup<byte>(5 * oneMb, 1024));
+        }
+
 #if NETCOREAPP3_1_OR_GREATER
         [Fact]
         public void Issue2001_NegativeMemoryReportedByGc()
diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs
diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs
diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs
diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp

Original file line number	Diff line number	Diff line change
`@@ -1504,6 +1504,9 @@ private void SkipChunkDataAndCrc(in PngChunk chunk)`
`1504`	`1504`	`private IMemoryOwner<byte> ReadChunkData(int length)`
`1505`	`1505`	`{`
`1506`	`1506`	`// We rent the buffer here to return it afterwards in Decode()`
	`1507`	`+ // We don't want to throw a degenerated memory exception here as we want to allow partial decoding`
	`1508`	`+ // so limit the length.`
	`1509`	`+ length = (int)Math.Min(length, this.currentStream.Length - this.currentStream.Position);`
`1507`	`1510`	`IMemoryOwner<byte> buffer = this.Configuration.MemoryAllocator.Allocate<byte>(length, AllocationOptions.Clean);`
`1508`	`1511`
`1509`	`1512`	`this.currentStream.Read(buffer.GetSpan(), 0, length);`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Memory.Internals`
`14`	`14`	`internal struct UnmanagedMemoryHandle : IEquatable<UnmanagedMemoryHandle>`
`15`	`15`	`{`
`16`	`16`	`// Number of allocation re-attempts when detecting OutOfMemoryException.`
`17`		`- private const int MaxAllocationAttempts = 1000;`
	`17`	`+ private const int MaxAllocationAttempts = 10;`
`18`	`18`
`19`	`19`	`// Track allocations for testing purposes:`
`20`	`20`	`private static int totalOutstandingHandles;`
Original file line number	Diff line number	Diff line change
`@@ -87,10 +87,18 @@ public override IMemoryOwner<T> Allocate<T>(`
`87`	`87`	`int length,`
`88`	`88`	`AllocationOptions options = AllocationOptions.None)`
`89`	`89`	`{`
`90`		`- Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length));`
`91`		`- int lengthInBytes = length * Unsafe.SizeOf<T>();`
	`90`	`+ if (length < 0)`
	`91`	`+ {`
	`92`	`+ throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}.");`
	`93`	`+ }`
	`94`	`+`
	`95`	`+ ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf<T>();`
	`96`	`+ if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes)`
	`97`	`+ {`
	`98`	`+ throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={lengthInBytes} that exceeded the limit {this.SingleBufferAllocationLimitBytes}.");`
	`99`	`+ }`
`92`	`100`
`93`		`- if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes)`
	`101`	`+ if (lengthInBytes <= (ulong)this.sharedArrayPoolThresholdInBytes)`
`94`	`102`	`{`
`95`	`103`	`var buffer = new SharedArrayPoolBuffer<T>(length);`
`96`	`104`	`if (options.Has(AllocationOptions.Clean))`
`@@ -101,7 +109,7 @@ public override IMemoryOwner<T> Allocate<T>(`
`101`	`109`	`return buffer;`
`102`	`110`	`}`
`103`	`111`
`104`		`- if (lengthInBytes <= this.poolBufferSizeInBytes)`
	`112`	`+ if (lengthInBytes <= (ulong)this.poolBufferSizeInBytes)`
`105`	`113`	`{`
`106`	`114`	`UnmanagedMemoryHandle mem = this.pool.Rent();`
`107`	`115`	`if (mem.IsValid)`
`@@ -115,20 +123,15 @@ public override IMemoryOwner<T> Allocate<T>(`
`115`	`123`	`}`
`116`	`124`
`117`	`125`	`/// <inheritdoc />`
`118`		`- internal override MemoryGroup<T> AllocateGroup<T>(`
`119`		`- long totalLength,`
	`126`	`+ internal override MemoryGroup<T> AllocateGroupCore<T>(`
	`127`	`+ long totalLengthInElements,`
	`128`	`+ long totalLengthInBytes,`
`120`	`129`	`int bufferAlignment,`
`121`	`130`	`AllocationOptions options = AllocationOptions.None)`
`122`	`131`	`{`
`123`		`- long totalLengthInBytes = totalLength * Unsafe.SizeOf<T>();`
`124`		`- if (totalLengthInBytes < 0)`
`125`		`- {`
`126`		`- throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable.");`
`127`		`- }`
`128`		`-`
`129`	`132`	`if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes)`
`130`	`133`	`{`
`131`		`- var buffer = new SharedArrayPoolBuffer<T>((int)totalLength);`
	`134`	`+ var buffer = new SharedArrayPoolBuffer<T>((int)totalLengthInElements);`
`132`	`135`	`return MemoryGroup<T>.CreateContiguous(buffer, options.Has(AllocationOptions.Clean));`
`133`	`136`	`}`
`134`	`137`
`@@ -138,18 +141,18 @@ internal override MemoryGroup<T> AllocateGroup<T>(`
`138`	`141`	`UnmanagedMemoryHandle mem = this.pool.Rent();`
`139`	`142`	`if (mem.IsValid)`
`140`	`143`	`{`
`141`		`- UnmanagedBuffer<T> buffer = this.pool.CreateGuardedBuffer<T>(mem, (int)totalLength, options.Has(AllocationOptions.Clean));`
	`144`	`+ UnmanagedBuffer<T> buffer = this.pool.CreateGuardedBuffer<T>(mem, (int)totalLengthInElements, options.Has(AllocationOptions.Clean));`
`142`	`145`	`return MemoryGroup<T>.CreateContiguous(buffer, options.Has(AllocationOptions.Clean));`
`143`	`146`	`}`
`144`	`147`	`}`
`145`	`148`
`146`	`149`	`// Attempt to rent the whole group from the pool, allocate a group of unmanaged buffers if the attempt fails:`
`147`		`- if (MemoryGroup<T>.TryAllocate(this.pool, totalLength, bufferAlignment, options, out MemoryGroup<T> poolGroup))`
	`150`	`+ if (MemoryGroup<T>.TryAllocate(this.pool, totalLengthInElements, bufferAlignment, options, out MemoryGroup<T>? poolGroup))`
`148`	`151`	`{`
`149`	`152`	`return poolGroup;`
`150`	`153`	`}`
`151`	`154`
`152`		`- return MemoryGroup<T>.Allocate(this.nonPoolAllocator, totalLength, bufferAlignment, options);`
	`155`	`+ return MemoryGroup<T>.Allocate(this.nonPoolAllocator, totalLengthInElements, bufferAlignment, options);`
`153`	`156`	`}`
`154`	`157`
`155`	`158`	`public override void ReleaseRetainedResources() => this.pool.Release();`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@`
`2`	`2`	`// Licensed under the Apache License, Version 2.0.`
`3`	`3`
`4`	`4`	`using System;`
	`5`	`+using System.Diagnostics.CodeAnalysis;`
`5`	`6`
`6`	`7`	`namespace SixLabors.ImageSharp.Memory`
`7`	`8`	`{`
Original file line number	Diff line number	Diff line change
`@@ -619,5 +619,18 @@ public void BmpDecoder_CanDecode_Os2BitmapArray<TPixel>(TestImageProvider<TPixel`
`619`	`619`	`// image.CompareToOriginal(provider);`
`620`	`620`	`}`
`621`	`621`	`}`
	`622`	`+`
	`623`	`+ [Theory]`
	`624`	`+ [WithFile(Issue2696, PixelTypes.Rgba32)]`
	`625`	`+ public void BmpDecoder_ThrowsException_Issue2696<TPixel>(TestImageProvider<TPixel> provider)`
	`626`	`+ where TPixel : unmanaged, IPixel<TPixel>`
	`627`	`+ {`
	`628`	`+ // On V2 this is throwing InvalidOperationException,`
	`629`	`+ // because of the validation logic in BmpInfoHeader.VerifyDimensions().`
	`630`	`+ Assert.Throws<InvalidOperationException>(() =>`
	`631`	`+ {`
	`632`	`+ using Image<TPixel> image = provider.GetImage(BmpDecoder);`
	`633`	`+ });`
	`634`	`+ }`
`622`	`635`	`}`
`623`	`636`	`}`