Skip to content

Make UnmanagedMemoryHandle members readonly and improve pool finalization tests #3040

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.

Sign up for GitHub

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

Merged
JimBobSquarePants merged 2 commits into from
Jan 12, 2026
+88 −85
Merged

Make UnmanagedMemoryHandle members readonly and improve pool finalization tests #3040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bca6197
Make UnmanagedMemoryHandle members readonly and improve pool finaliza…
JimBobSquarePants Jan 12, 2026
55818d3
Minor cleanup
JimBobSquarePants Jan 12, 2026
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
14 changes: 7 additions & 7 deletions src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ private UnmanagedMemoryHandle(IntPtr handle, int lengthInBytes)
Interlocked.Increment(ref totalOutstandingHandles);
}

public IntPtr Handle => this.handle;
public readonly IntPtr Handle => this.handle;

public bool IsInvalid => this.Handle == IntPtr.Zero;
public readonly bool IsInvalid => this.Handle == IntPtr.Zero;

public bool IsValid => this.Handle != IntPtr.Zero;
public readonly bool IsValid => this.Handle != IntPtr.Zero;

public unsafe void* Pointer => (void*)this.Handle;
public readonly unsafe void* Pointer => (void*)this.Handle;

/// <summary>
/// Gets the total outstanding handle allocations for testing purposes.
Expand Down Expand Up @@ -121,9 +121,9 @@ public void Free()
this.lengthInBytes = 0;
}

public bool Equals(UnmanagedMemoryHandle other) => this.handle.Equals(other.handle);
public readonly bool Equals(UnmanagedMemoryHandle other) => this.handle.Equals(other.handle);

public override bool Equals(object? obj) => obj is UnmanagedMemoryHandle other && this.Equals(other);
public override readonly bool Equals(object? obj) => obj is UnmanagedMemoryHandle other && this.Equals(other);

public override int GetHashCode() => this.handle.GetHashCode();
public override readonly int GetHashCode() => this.handle.GetHashCode();
}
159 changes: 81 additions & 78 deletions tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs
View file
Open in desktop
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.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Microsoft.DotNet.RemoteExecutor;
Expand Down Expand Up @@ -273,137 +274,139 @@ static void RunTest()
[InlineData(1200)] // Group of two UniformUnmanagedMemoryPool buffers
public void AllocateMemoryGroup_Finalization_ReturnsToPool(int length)
{
if (TestEnvironment.IsMacOS)
{
// Skip on macOS: https://github.com/SixLabors/ImageSharp/issues/1887
return;
}

if (TestEnvironment.OSArchitecture == Architecture.Arm64)
{
// Skip on ARM64: https://github.com/SixLabors/ImageSharp/issues/2342
return;
}

if (!TestEnvironment.RunsOnCI)
{
// This may fail in local runs resulting in high memory load.
// Remove the condition for local debugging!
return;
}

// RunTest(length.ToString());
RemoteExecutor.Invoke(RunTest, length.ToString()).Dispose();
RemoteExecutor.Invoke(RunTest, length.ToString(CultureInfo.InvariantCulture)).Dispose();

static void RunTest(string lengthStr)
{
UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(512, 1024, 16 * 1024, 1024);
int lengthInner = int.Parse(lengthStr);

int lengthInner = int.Parse(lengthStr, CultureInfo.InvariantCulture);

// We want to verify that a leaked (not disposed) `MemoryGroup<byte>` still returns its
// unmanaged handles into the pool when it is finalized.
//
// We intentionally do NOT validate this by checking the contents of the re-rented memory
// (contents are not guaranteed to be preserved) nor by comparing pointer values
// (the pool may return a different handle while still correctly pooling).
//
// Instead, we validate that after a forced GC+finalization cycle, a subsequent allocation
// of the same size does not cause the number of outstanding unmanaged handles to *increase*
// compared to a known baseline.

// Establish a baseline: create one allocation and dispose it so the pool is initialized.
// (This ensures subsequent observations are not biased by first-time pool growth.)
allocator.AllocateGroup<byte>(lengthInner, 100).Dispose();
int baselineHandles = UnmanagedMemoryHandle.TotalOutstandingHandles;

// Leak one allocation and force finalization.
AllocateGroupAndForget(allocator, lengthInner);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();

AllocateGroupAndForget(allocator, lengthInner, true);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();

using MemoryGroup<byte> g = allocator.AllocateGroup<byte>(lengthInner, 100);
Assert.Equal(42, g.First().Span[0]);
// Allocate again. If the leaked group was finalized correctly and returned to the pool,
// this should not require additional unmanaged allocations (ie, the handle count must not grow).
allocator.AllocateGroup<byte>(lengthInner, 100).Dispose();

// Note: we use "<=" instead of "==" here.
//
// After we record the baseline, the pool is allowed to legitimately *decrease*
// `UnmanagedMemoryHandle.TotalOutstandingHandles` by trimming retained buffers
// (eg. via the pool's trim timer/GC callbacks/high-pressure logic).
//
// What must not happen is the opposite: the leaked (non-disposed) group should be finalized
// and its handles returned to the pool such that allocating again does NOT require creating
// additional unmanaged handles. Therefore the only invariant we can reliably assert here is
// "no growth" relative to the baseline.
Assert.True(UnmanagedMemoryHandle.TotalOutstandingHandles <= baselineHandles);
}
}

private static void AllocateGroupAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length, bool check = false)
[MethodImpl(MethodImplOptions.NoInlining)]
private static void AllocateGroupAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length)
{
// Allocate a group and drop the reference without disposing.
// The test relies on the group's finalizer to return the rented memory to the pool.
MemoryGroup<byte> g = allocator.AllocateGroup<byte>(length, 100);
if (check)
{
Assert.Equal(42, g.First().Span[0]);
}

g.First().Span[0] = 42;
// Touch the memory to ensure the buffer is actually materialized/usable.
g[0].Span[0] = 42;

if (length < 512)
{
// For ArrayPool.Shared, first array will be returned to the TLS storage of the finalizer thread,
// repeat rental to make sure per-core buckets are also utilized.
// For ArrayPool.Shared, the first rented array may be stored in TLS on the finalizer thread.
// Repeat rental to increase the chance that per-core buckets are involved when length
// is small and allocations go through ArrayPool.
MemoryGroup<byte> g1 = allocator.AllocateGroup<byte>(length, 100);
g1.First().Span[0] = 42;
g1[0].Span[0] = 42;
g1 = null;
}

g = null;
}

[Theory]
[InlineData(300)] // Group of single SharedArrayPoolBuffer<T>
[InlineData(600)] // Group of single UniformUnmanagedMemoryPool buffer
public void AllocateSingleMemoryOwner_Finalization_ReturnsToPool(int length)
{
if (TestEnvironment.IsMacOS)
{
// Skip on macOS: https://github.com/SixLabors/ImageSharp/issues/1887
return;
}

if (TestEnvironment.OSArchitecture == Architecture.Arm64)
{
// Skip on ARM64: https://github.com/SixLabors/ImageSharp/issues/2342
return;
}

if (!TestEnvironment.RunsOnCI)
{
// This may fail in local runs resulting in high memory load.
// Remove the condition for local debugging!
return;
}

// RunTest(length.ToString());
RemoteExecutor.Invoke(RunTest, length.ToString()).Dispose();
RemoteExecutor.Invoke(RunTest, length.ToString(CultureInfo.InvariantCulture)).Dispose();

static void RunTest(string lengthStr)
{
UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(512, 1024, 16 * 1024, 1024);
int lengthInner = int.Parse(lengthStr);

int lengthInner = int.Parse(lengthStr, CultureInfo.InvariantCulture);

// This test verifies pooling behavior when an `IMemoryOwner<byte>` is leaked (not disposed)
// and must be returned to the pool by finalization.
//
// We do NOT use a sentinel byte value to prove reuse because the contents of pooled buffers
// are not required to be preserved across rentals.
//
// Instead, we assert that after forcing GC+finalization, renting the same size again does not
// increase `UnmanagedMemoryHandle.TotalOutstandingHandles` above a baseline.

// Establish a baseline: allocate+dispose once so the pool has a chance to materialize/retain buffers.
allocator.Allocate<byte>(lengthInner).Dispose();
int baselineHandles = UnmanagedMemoryHandle.TotalOutstandingHandles;

// Leak one allocation and force finalization.
AllocateSingleAndForget(allocator, lengthInner);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();

AllocateSingleAndForget(allocator, lengthInner, true);
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
// Allocate again. If the leaked owner was finalized correctly and returned to the pool,
// this should not require additional unmanaged allocations (ie, the handle count must not grow).
allocator.Allocate<byte>(lengthInner).Dispose();

using IMemoryOwner<byte> g = allocator.Allocate<byte>(lengthInner);
Assert.Equal(42, g.GetSpan()[0]);
GC.KeepAlive(allocator);
// Note: we use "<=" rather than "==". The pool may legitimately trim and free retained buffers,
// reducing the handle count between baseline and check. The invariant is "no growth".
Assert.True(UnmanagedMemoryHandle.TotalOutstandingHandles <= baselineHandles);
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length, bool check = false)
private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length)
{
// Allocate and intentionally do not dispose.
IMemoryOwner<byte> g = allocator.Allocate<byte>(length);
if (check)
{
Assert.Equal(42, g.GetSpan()[0]);
}

// Touch the memory to ensure the buffer is actually materialized/usable.
g.GetSpan()[0] = 42;

if (length < 512)
{
// For ArrayPool.Shared, first array will be returned to the TLS storage of the finalizer thread,
// repeat rental to make sure per-core buckets are also utilized.
// For ArrayPool.Shared, the first rented array may be stored in TLS on the finalizer thread.
// Repeat rental to increase the chance that per-core buckets are involved when length
// is small and allocations go through ArrayPool.
IMemoryOwner<byte> g1 = allocator.Allocate<byte>(length);
g1.GetSpan()[0] = 42;
g1 = null;
}

g = null;
}

[Fact]
Expand Down
Loading