Skip to content

Commit 23b312c

Browse files
Fix multi-buffer group tracking and enforce limits
1 parent afc97df commit 23b312c

5 files changed

Lines changed: 70 additions & 6 deletions

File tree

src/ImageSharp/Memory/Allocators/MemoryAllocator.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ public abstract class MemoryAllocator
3434
internal long MemoryGroupAllocationLimitBytes { get; private protected set; } = Environment.Is64BitProcess ? 4L * OneGigabyte : OneGigabyte;
3535

3636
/// <summary>
37-
/// Gets the maximum cumulative size, in bytes, of all active allocations made through this allocator instance.
37+
/// Gets the maximum accumulative size, in bytes, of all active allocations made through this allocator instance.
3838
/// </summary>
3939
/// <remarks>
40-
/// Defaults to <see cref="long.MaxValue"/>, effectively imposing no limit on the cumulative total.
40+
/// Defaults to <see cref="long.MaxValue"/>, effectively imposing no limit on the accumulative total.
4141
/// When set, this provides a safeguard against excessive memory consumption by capping the combined size of
4242
/// outstanding allocations issued by this instance.<br/>
43-
/// When the cumulative size of active allocations exceeds this limit, an <see cref="InvalidMemoryOperationException"/> will be thrown to
43+
/// When the accumulative size of active allocations exceeds this limit, an <see cref="InvalidMemoryOperationException"/> will be thrown to
4444
/// prevent further allocations and signal that the limit has been breached.
4545
/// </remarks>
4646
internal long AccumulativeAllocationLimitBytes { get; private protected set; } = long.MaxValue;

src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,23 @@ public int? AllocationLimitMegabytes
4242
if (value.HasValue)
4343
{
4444
Guard.MustBeGreaterThan(value.Value, 0, nameof(this.AllocationLimitMegabytes));
45+
if (this.AccumulativeAllocationLimitMegabytes.HasValue)
46+
{
47+
Guard.MustBeLessThanOrEqualTo(
48+
value.Value,
49+
this.AccumulativeAllocationLimitMegabytes.Value,
50+
nameof(this.AllocationLimitMegabytes));
51+
}
4552
}
4653

4754
this.allocationLimitMegabytes = value;
4855
}
4956
}
5057

5158
/// <summary>
52-
/// Gets or sets a value defining the maximum cumulative size, in Megabytes, of all active allocations made
59+
/// Gets or sets a value defining the maximum accumulative size, in Megabytes, of all active allocations made
5360
/// through the created <see cref="MemoryAllocator"/> instance.
54-
/// <see langword="null"/> (the default) imposes no limit on the cumulative total.
61+
/// <see langword="null"/> (the default) imposes no limit on the accumulative total.
5562
/// </summary>
5663
public int? AccumulativeAllocationLimitMegabytes
5764
{

src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,37 @@ internal override void AttachAllocationTracking(MemoryAllocator allocator, long
7878
return;
7979
}
8080

81+
if (memoryOwners?.Length > 1)
82+
{
83+
foreach (IMemoryOwner<T> memoryOwner in memoryOwners)
84+
{
85+
if (memoryOwner is not AllocationTrackedMemoryManager<T>)
86+
{
87+
// Splitting is only valid when every segment can own its reservation. A single
88+
// untracked segment makes the whole group ineligible, and this preflight has
89+
// not attached anything yet, so the entire group can fall back immediately.
90+
base.AttachAllocationTracking(allocator, lengthInBytes);
91+
return;
92+
}
93+
}
94+
95+
// Non-pool multi-buffer groups have no group-level finalizer, so each segment carries
96+
// its own share of the reservation through the segment owner or its lifetime guard.
97+
long remainingLengthInBytes = lengthInBytes;
98+
int lastOwnerIndex = memoryOwners.Length - 1;
99+
for (int i = 0; i < lastOwnerIndex; i++)
100+
{
101+
trackedOwner = (AllocationTrackedMemoryManager<T>)memoryOwners[i];
102+
long ownerLengthInBytes = (long)trackedOwner.Memory.Length * Unsafe.SizeOf<T>();
103+
trackedOwner.AttachAllocationTracking(allocator, ownerLengthInBytes);
104+
remainingLengthInBytes -= ownerLengthInBytes;
105+
}
106+
107+
trackedOwner = (AllocationTrackedMemoryManager<T>)memoryOwners[lastOwnerIndex];
108+
trackedOwner.AttachAllocationTracking(allocator, remainingLengthInBytes);
109+
return;
110+
}
111+
81112
base.AttachAllocationTracking(allocator, lengthInBytes);
82113
}
83114

src/ImageSharp/Memory/InvalidMemoryOperationException.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,5 @@ internal static void ThrowAllocationOverLimitException(ulong length, long limit)
4343
[DoesNotReturn]
4444
internal static void ThrowAccumulativeAllocationOverLimitException(long requestedLength, long totalLength, long limit) =>
4545
throw new InvalidMemoryOperationException(
46-
$"Attempted to allocate a buffer of length={requestedLength} that would increase the cumulative allocation size to {totalLength}, exceeding the limit {limit}.");
46+
$"Attempted to allocate a buffer of length={requestedLength} that would increase the accumulative allocation size to {totalLength}, exceeding the limit {limit}.");
4747
}

tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,32 @@ public void AllocateGroup_AccumulativeLimit_NonPoolFallback_TracksOncePerGroup()
558558
allocator.AllocateGroup<byte>(768 * 1024, 1024).Dispose();
559559
}
560560

561+
[Fact]
562+
public void AllocateGroup_AccumulativeLimit_NonPoolFallback_Finalization_ReleasesGroupReservation()
563+
{
564+
RemoteExecutor.Invoke(RunTest).Dispose();
565+
566+
static void RunTest()
567+
{
568+
UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(
569+
sharedArrayPoolThresholdInBytes: 64 * 1024,
570+
poolBufferSizeInBytes: 128 * 1024,
571+
maxPoolSizeInBytes: 0,
572+
unmanagedBufferSizeInBytes: 256 * 1024,
573+
new MemoryAllocatorOptions { AccumulativeAllocationLimitMegabytes = 1 });
574+
575+
// This exercises the non-pool multi-segment fallback, where reservation ownership has
576+
// to follow the finalizable segment guards because the MemoryGroup itself has no finalizer.
577+
AllocateGroupAndForget(allocator, 768 * 1024);
578+
GC.Collect();
579+
GC.WaitForPendingFinalizers();
580+
GC.Collect();
581+
GC.WaitForPendingFinalizers();
582+
583+
allocator.AllocateGroup<byte>(768 * 1024, 1024).Dispose();
584+
}
585+
}
586+
561587
[ConditionalFact(typeof(Environment), nameof(Environment.Is64BitProcess))]
562588
public void MemoryAllocator_Create_SetHighLimit()
563589
{

0 commit comments

Comments
 (0)