Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge latest release from v3 #2720

Merged
merged 34 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
58e8efa
Ensure VP8X alpha flag is updated correctly.
JimBobSquarePants Mar 15, 2024
a197f22
Update WebpVp8X.cs
JimBobSquarePants Mar 17, 2024
623ad5d
Only exit after multiple EOF hits
JimBobSquarePants Mar 18, 2024
4f289f9
Remove unused code.
JimBobSquarePants Mar 18, 2024
ea12c0d
Update comment
JimBobSquarePants Mar 18, 2024
a29c6fd
Merge pull request #2702 from SixLabors/backport/2638
JimBobSquarePants Mar 19, 2024
92b8277
Limit all allocations
antonfirsov Mar 28, 2024
7dd3c43
reduce UnmanagedMemoryHandle.MaxAllocationAttempts
antonfirsov Mar 28, 2024
326f76d
Merge pull request #2699 from SixLabors/js/fix-webp-alpha-flags
JimBobSquarePants Apr 1, 2024
417667a
Add test
SpaceCheetah Mar 28, 2024
d933db7
Fix ProcessInterlacedPaletteScanline not obeying frameControl.XOffset
SpaceCheetah Mar 28, 2024
63b7ecd
Fix frame dispose operation handling
SpaceCheetah Mar 29, 2024
d15a2e7
Add test for default image not animated
SpaceCheetah Mar 29, 2024
9b8ef10
Fix handling of case where default image isn't animated
SpaceCheetah Mar 29, 2024
85fd431
Fix PngMetadata copy
SpaceCheetah Mar 29, 2024
addec72
Make PngEncoder respect DefaultImageAnimated
SpaceCheetah Mar 29, 2024
9ae8be4
Add more tests
SpaceCheetah Mar 30, 2024
ceb7dc0
Fix incorrect acTL frame count
SpaceCheetah Mar 30, 2024
4698e8c
re-add comment
SpaceCheetah Apr 2, 2024
b1a5593
Add test for case with frame offsets
SpaceCheetah Apr 2, 2024
8480139
re-add rest of comment
SpaceCheetah Apr 2, 2024
63d4b20
Fix args and add tests
JimBobSquarePants Apr 3, 2024
c8eab78
Rename DefaultImageAnimated to AnimateRootFrame
SpaceCheetah Apr 3, 2024
8e6532a
Merge pull request #2713 from SpaceCheetah/FixPngBackport
JimBobSquarePants Apr 3, 2024
eec9718
Merge branch 'release/3.1.x' into af/memlimit-01
JimBobSquarePants Apr 4, 2024
da49788
Revert additional changes
JimBobSquarePants Apr 5, 2024
26d44ef
Clear pixel buffers on decode.
JimBobSquarePants Apr 6, 2024
fb64b33
Clamp read palette indices.
JimBobSquarePants Apr 6, 2024
4a26acb
Merge pull request #2718 from SixLabors/js/check-palette-index
JimBobSquarePants Apr 9, 2024
d1cc651
fix BmpDecoder_ThrowsException_Issue2696
antonfirsov Apr 10, 2024
572366e
test allocation limits
antonfirsov Apr 10, 2024
b6b08ac
Merge pull request #2706 from SixLabors/af/memlimit-01
antonfirsov Apr 10, 2024
da5f09a
Merge pull request #2716 from SixLabors/js/clear-buffers
antonfirsov Apr 10, 2024
3495b01
Merge branch 'release/3.1.x' into port/v3
JimBobSquarePants Apr 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ public override void PrepareForDecoding()
this.pixelBuffer = allocator.Allocate2D<TPixel>(
pixelSize.Width,
pixelSize.Height,
this.Configuration.PreferContiguousImageBuffers);
this.Configuration.PreferContiguousImageBuffers,
AllocationOptions.Clean);
this.paddedProxyPixelRow = allocator.Allocate<TPixel>(pixelSize.Width + 3);

// Component processors from spectral to RGB
Expand Down
3 changes: 3 additions & 0 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1983,6 +1983,9 @@ 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);
Expand Down
3 changes: 2 additions & 1 deletion src/ImageSharp/Formats/Png/PngScanlineProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,12 @@ public static void ProcessInterlacedPaletteScanline<TPixel>(
ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan);
ref Color paletteBase = ref MemoryMarshal.GetReference(palette.Value.Span);
uint offset = pixelOffset + frameControl.XOffset;
int maxIndex = palette.Value.Length - 1;

for (nuint x = offset, o = 0; x < frameControl.XMax; x += increment, o++)
{
uint index = Unsafe.Add(ref scanlineSpanRef, o);
Unsafe.Add(ref rowSpanRef, x) = TPixel.FromRgba32(Unsafe.Add(ref paletteBase, index).ToPixel<Rgba32>());
Unsafe.Add(ref rowSpanRef, x) = TPixel.FromRgba32(Unsafe.Add(ref paletteBase, (int)Math.Min(index, maxIndex)).ToPixel<Rgba32>());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/ImageSharp/Formats/Tga/TgaDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
throw new UnknownImageFormatException("Width or height cannot be 0");
}

Image<TPixel> image = Image.CreateUninitialized<TPixel>(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata);
Image<TPixel> image = new(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata);
Buffer2D<TPixel> pixels = image.GetRootFramePixelBuffer();

if (this.fileHeader.ColorMapType == 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,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;
Expand Down
46 changes: 40 additions & 6 deletions src/ImageSharp/Memory/Allocators/MemoryAllocator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Six Labors Split License.

using System.Buffers;
using System.Runtime.CompilerServices;

namespace SixLabors.ImageSharp.Memory;

Expand All @@ -10,6 +11,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"/>.
Expand All @@ -20,6 +23,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>
Expand All @@ -30,16 +37,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"/>.
Expand All @@ -64,15 +79,34 @@ 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)
{
InvalidMemoryOperationException.ThrowNegativeAllocationException(totalLength);
}

ulong totalLengthInBytes = (ulong)totalLength * (ulong)Unsafe.SizeOf<T>();
if (totalLengthInBytes > (ulong)this.MemoryGroupAllocationLimitBytes)
{
InvalidMemoryOperationException.ThrowAllocationOverLimitException(totalLengthInBytes, 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);
}
21 changes: 20 additions & 1 deletion src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Six Labors.
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

namespace SixLabors.ImageSharp.Memory;
Expand All @@ -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
Expand All @@ -27,4 +28,22 @@ 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
{
get => this.allocationLimitMegabytes;
set
{
if (value.HasValue)
{
Guard.MustBeGreaterThan(value.Value, 0, nameof(this.AllocationLimitMegabytes));
}

this.allocationLimitMegabytes = value;
}
}
}
14 changes: 12 additions & 2 deletions src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)
{
InvalidMemoryOperationException.ThrowNegativeAllocationException(length);
}

ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf<T>();
if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes)
{
InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes);
}

return new BasicArrayBuffer<T>(new T[length]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,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)
{
InvalidMemoryOperationException.ThrowNegativeAllocationException(length);
}

ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf<T>();
if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes)
{
InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes);
}

if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes)
if (lengthInBytes <= (ulong)this.sharedArrayPoolThresholdInBytes)
{
var buffer = new SharedArrayPoolBuffer<T>(length);
if (options.Has(AllocationOptions.Clean))
Expand All @@ -97,7 +105,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)
Expand All @@ -111,20 +119,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));
}

Expand All @@ -134,18 +137,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();
Expand Down
13 changes: 7 additions & 6 deletions src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,16 @@ 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)
{
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)
Expand Down
15 changes: 15 additions & 0 deletions src/ImageSharp/Memory/InvalidMemoryOperationException.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

using System.Diagnostics.CodeAnalysis;

namespace SixLabors.ImageSharp.Memory;

/// <summary>
Expand All @@ -24,4 +26,17 @@ public InvalidMemoryOperationException(string message)
public InvalidMemoryOperationException()
{
}

[DoesNotReturn]
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}.");
}
12 changes: 12 additions & 0 deletions tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -558,4 +558,16 @@ public void BmpDecoder_CanDecode_Os2BitmapArray<TPixel>(TestImageProvider<TPixel
// Compare to reference output instead.
image.CompareToReferenceOutput(provider, extension: "png");
}

[Theory]
[WithFile(Issue2696, PixelTypes.Rgba32)]
public void BmpDecoder_ThrowsException_Issue2696<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
InvalidImageContentException ex = Assert.Throws<InvalidImageContentException>(() =>
{
using Image<TPixel> image = provider.GetImage(BmpDecoder.Instance);
});
Assert.IsType<InvalidMemoryOperationException>(ex.InnerException);
}
}
18 changes: 4 additions & 14 deletions tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -338,21 +338,11 @@ public void Issue2564_DecodeWorks<TPixel>(TestImageProvider<TPixel> provider)
}

[Theory]
[WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)]
public void DecodeHang<TPixel>(TestImageProvider<TPixel> provider)
[WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.Rgb24)]
public void DecodeHang_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
if (TestEnvironment.IsWindows &&
TestEnvironment.RunsOnCI)
{
// Windows CI runs consistently fail with OOM.
return;
}

using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
Assert.Equal(65503, image.Width);
Assert.Equal(65503, image.Height);
}
=> Assert.Throws<InvalidImageContentException>(
() => { using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance); });

// https://github.com/SixLabors/ImageSharp/issues/2517
[Theory]
Expand Down
8 changes: 8 additions & 0 deletions tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -693,4 +693,12 @@ public void Info_BadZTXT(string file)
string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file));
_ = Image.Identify(path);
}

[Theory]
[InlineData(TestImages.Png.Bad.Issue2714BadPalette)]
public void Decode_BadPalette(string file)
{
string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file));
using Image image = Image.Load(path);
}
}
Loading
Loading