diff --git a/src/ImageSharp/Common/Helpers/DebugGuard.cs b/src/ImageSharp/Common/Helpers/DebugGuard.cs index f56cb37a81..43622066c3 100644 --- a/src/ImageSharp/Common/Helpers/DebugGuard.cs +++ b/src/ImageSharp/Common/Helpers/DebugGuard.cs @@ -26,6 +26,20 @@ namespace SixLabors } } + /// + /// Verifies whether a specific condition is met, throwing an exception if it's false. + /// + /// Whether the object is disposed. + /// The name of the object. + [Conditional("DEBUG")] + public static void NotDisposed(bool isDisposed, string objectName) + { + if (isDisposed) + { + throw new ObjectDisposedException(objectName); + } + } + /// /// Verifies, that the target span is of same size than the 'other' span. /// diff --git a/src/ImageSharp/Memory/Allocators/Internals/IRefCounted.cs b/src/ImageSharp/Memory/Allocators/Internals/IRefCounted.cs new file mode 100644 index 0000000000..363b680483 --- /dev/null +++ b/src/ImageSharp/Memory/Allocators/Internals/IRefCounted.cs @@ -0,0 +1,21 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +namespace SixLabors.ImageSharp.Memory.Internals +{ + /// + /// Defines an common interface for ref-counted objects. + /// + internal interface IRefCounted + { + /// + /// Increments the reference counter. + /// + void AddRef(); + + /// + /// Decrements the reference counter. + /// + void ReleaseRef(); + } +} diff --git a/src/ImageSharp/Memory/Allocators/Internals/RefCountedLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/RefCountedLifetimeGuard.cs new file mode 100644 index 0000000000..61682aa567 --- /dev/null +++ b/src/ImageSharp/Memory/Allocators/Internals/RefCountedLifetimeGuard.cs @@ -0,0 +1,56 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Runtime.InteropServices; +using System.Threading; + +namespace SixLabors.ImageSharp.Memory.Internals +{ + /// + /// Implements reference counting lifetime guard mechanism similar to the one provided by , + /// but without the restriction of the guarded object being a handle. + /// + internal abstract class RefCountedLifetimeGuard : IDisposable + { + private int refCount = 1; + private int disposed; + private int released; + + ~RefCountedLifetimeGuard() + { + Interlocked.Exchange(ref this.disposed, 1); + this.ReleaseRef(); + } + + public bool IsDisposed => this.disposed == 1; + + public void AddRef() => Interlocked.Increment(ref this.refCount); + + public void ReleaseRef() + { + Interlocked.Decrement(ref this.refCount); + if (this.refCount == 0) + { + int wasReleased = Interlocked.Exchange(ref this.released, 1); + + if (wasReleased == 0) + { + this.Release(); + } + } + } + + public void Dispose() + { + int wasDisposed = Interlocked.Exchange(ref this.disposed, 1); + if (wasDisposed == 0) + { + this.ReleaseRef(); + GC.SuppressFinalize(this); + } + } + + protected abstract void Release(); + } +} diff --git a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs index 6cb2a24fb0..5027d94b44 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs @@ -3,30 +3,26 @@ using System; using System.Buffers; +using System.Diagnostics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; namespace SixLabors.ImageSharp.Memory.Internals { - internal class SharedArrayPoolBuffer : ManagedBufferBase + internal class SharedArrayPoolBuffer : ManagedBufferBase, IRefCounted where T : struct { private readonly int lengthInBytes; private byte[] array; + private LifetimeGuard lifetimeGuard; public SharedArrayPoolBuffer(int lengthInElements) { this.lengthInBytes = lengthInElements * Unsafe.SizeOf(); this.array = ArrayPool.Shared.Rent(this.lengthInBytes); + this.lifetimeGuard = new LifetimeGuard(this.array); } - // The worst thing that could happen is that a VERY poorly written user code holding a Span on the stack, - // while loosing the reference to Image (or disposing it) may write to an unrelated ArrayPool array. - // This is an unlikely scenario we mitigate by a warning in DangerousGetRowSpan(i) APIs. -#pragma warning disable CA2015 // Adding a finalizer to a type derived from MemoryManager may permit memory to be freed while it is still in use by a Span - ~SharedArrayPoolBuffer() => this.Dispose(false); -#pragma warning restore - protected override void Dispose(bool disposing) { if (this.array == null) @@ -34,12 +30,51 @@ namespace SixLabors.ImageSharp.Memory.Internals return; } - ArrayPool.Shared.Return(this.array); + this.lifetimeGuard.Dispose(); this.array = null; } - public override Span GetSpan() => MemoryMarshal.Cast(this.array.AsSpan(0, this.lengthInBytes)); + public override Span GetSpan() + { + this.CheckDisposed(); + return MemoryMarshal.Cast(this.array.AsSpan(0, this.lengthInBytes)); + } protected override object GetPinnableObject() => this.array; + + public void AddRef() + { + this.CheckDisposed(); + this.lifetimeGuard.AddRef(); + } + + public void ReleaseRef() => this.lifetimeGuard.ReleaseRef(); + + [Conditional("DEBUG")] + private void CheckDisposed() + { + if (this.array == null) + { + throw new ObjectDisposedException("SharedArrayPoolBuffer"); + } + } + + private class LifetimeGuard : RefCountedLifetimeGuard + { + private byte[] array; + + public LifetimeGuard(byte[] array) => this.array = array; + + protected override void Release() + { + // If this is called by a finalizer, we will end storing the first array of this bucket + // on the thread local storage of the finalizer thread. + // This is not ideal, but subsequent leaks will end up returning arrays to per-cpu buckets, + // meaning likely a different bucket than it was rented from, + // but this is PROBABLY better than not returning the arrays at all. + ArrayPool.Shared.Return(this.array); + this.array = null; + } + } } } diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs deleted file mode 100644 index c3d736cee4..0000000000 --- a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Buffers; -using System.Runtime.CompilerServices; - -namespace SixLabors.ImageSharp.Memory.Internals -{ - internal partial class UniformUnmanagedMemoryPool - { - public class Buffer : UnmanagedBuffer - where T : struct - { - private UniformUnmanagedMemoryPool pool; - - public Buffer(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle bufferHandle, int length) - : base(bufferHandle, length) => - this.pool = pool; - - /// - protected override void Dispose(bool disposing) - { - if (this.pool == null) - { - return; - } - - this.pool.Return(this.BufferHandle); - this.pool = null; - this.BufferHandle = null; - } - - internal void MarkDisposed() - { - this.pool = null; - this.BufferHandle = null; - } - } - - public sealed class FinalizableBuffer : Buffer - where T : struct - { - public FinalizableBuffer(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle bufferHandle, int length) - : base(pool, bufferHandle, length) - { - bufferHandle.AssignedToNewOwner(); - } - - // A VERY poorly written user code holding a Span on the stack, - // while loosing the reference to Image (or disposing it) may write to (now unrelated) pool buffer, - // or cause memory corruption if the underlying UmnanagedMemoryHandle has been released. - // This is an unlikely scenario we mitigate a warning in DangerousGetRowSpan(i) APIs. -#pragma warning disable CA2015 // Adding a finalizer to a type derived from MemoryManager may permit memory to be freed while it is still in use by a Span - ~FinalizableBuffer() => this.Dispose(false); -#pragma warning restore - - protected override void Dispose(bool disposing) - { - if (!disposing && this.BufferHandle != null) - { - // We need to prevent handle finalization here. - // See comments on UnmanagedMemoryHandle.Resurrect() - this.BufferHandle.Resurrect(); - } - - base.Dispose(disposing); - GC.SuppressFinalize(this); - } - } - } -} diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs new file mode 100644 index 0000000000..032537d381 --- /dev/null +++ b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs @@ -0,0 +1,50 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +namespace SixLabors.ImageSharp.Memory.Internals +{ + internal partial class UniformUnmanagedMemoryPool + { + public UnmanagedBuffer CreateGuardedBuffer( + UnmanagedMemoryHandle handle, + int lengthInElements, + AllocationOptions options) + where T : struct + { + var buffer = new UnmanagedBuffer(lengthInElements, new ReturnToPoolBufferLifetimeGuard(this, handle)); + if (options.Has(AllocationOptions.Clean)) + { + buffer.Clear(); + } + + return buffer; + } + + public RefCountedLifetimeGuard CreateGroupLifetimeGuard(UnmanagedMemoryHandle[] handles) => new GroupLifetimeGuard(this, handles); + + private sealed class GroupLifetimeGuard : RefCountedLifetimeGuard + { + private readonly UniformUnmanagedMemoryPool pool; + private readonly UnmanagedMemoryHandle[] handles; + + public GroupLifetimeGuard(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle[] handles) + { + this.pool = pool; + this.handles = handles; + } + + protected override void Release() => this.pool.Return(this.handles); + } + + private sealed class ReturnToPoolBufferLifetimeGuard : UnmanagedBufferLifetimeGuard + { + private readonly UniformUnmanagedMemoryPool pool; + + public ReturnToPoolBufferLifetimeGuard(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle handle) + : base(handle) => + this.pool = pool; + + protected override void Release() => this.pool.Return(this.Handle); + } + } +} diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs index 3d5c0da2c3..4cccab4afb 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs @@ -2,19 +2,24 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Threading; namespace SixLabors.ImageSharp.Memory.Internals { internal partial class UniformUnmanagedMemoryPool { + private static int minTrimPeriodMilliseconds = int.MaxValue; + private static readonly List> AllPools = new(); + private static Timer trimTimer; + private static readonly Stopwatch Stopwatch = Stopwatch.StartNew(); private readonly TrimSettings trimSettings; - private UnmanagedMemoryHandle[] buffers; + private readonly UnmanagedMemoryHandle[] buffers; private int index; - private Timer trimTimer; private long lastTrimTimestamp; public UniformUnmanagedMemoryPool(int bufferLength, int capacity) @@ -31,16 +36,7 @@ namespace SixLabors.ImageSharp.Memory.Internals if (trimSettings.Enabled) { - // Invoke the timer callback more frequently, than trimSettings.TrimPeriodMilliseconds, - // and also invoke it on Gen 2 GC. - // We are checking in the callback if enough time passed since the last trimming. If not, we do nothing. - var weakPoolRef = new WeakReference(this); - this.trimTimer = new Timer( - s => TimerCallback((WeakReference)s), - weakPoolRef, - this.trimSettings.TrimPeriodMilliseconds / 4, - this.trimSettings.TrimPeriodMilliseconds / 4); - + UpdateTimer(trimSettings, this); #if NETCORE31COMPATIBLE Gen2GcCallback.Register(s => ((UniformUnmanagedMemoryPool)s).Trim(), this); #endif @@ -52,31 +48,33 @@ namespace SixLabors.ImageSharp.Memory.Internals public int Capacity { get; } + /// + /// Rent a single buffer or return if the pool is full. + /// public UnmanagedMemoryHandle Rent() { UnmanagedMemoryHandle[] buffersLocal = this.buffers; - // Avoid taking the lock if the pool is released or is over limit: - if (buffersLocal == null || this.index == buffersLocal.Length) + // Avoid taking the lock if the pool is is over it's limit: + if (this.index == buffersLocal.Length) { - return null; + return UnmanagedMemoryHandle.NullHandle; } UnmanagedMemoryHandle buffer; - lock (buffersLocal) { // Check again after taking the lock: - if (this.buffers == null || this.index == buffersLocal.Length) + if (this.index == buffersLocal.Length) { - return null; + return UnmanagedMemoryHandle.NullHandle; } buffer = buffersLocal[this.index]; - buffersLocal[this.index++] = null; + buffersLocal[this.index++] = default; } - if (buffer == null) + if (buffer.IsInvalid) { buffer = UnmanagedMemoryHandle.Allocate(this.BufferLength); } @@ -84,12 +82,15 @@ namespace SixLabors.ImageSharp.Memory.Internals return buffer; } - public UnmanagedMemoryHandle[] Rent(int bufferCount, AllocationOptions allocationOptions = AllocationOptions.None) + /// + /// Rent buffers or return 'null' if the pool is full. + /// + public UnmanagedMemoryHandle[] Rent(int bufferCount) { UnmanagedMemoryHandle[] buffersLocal = this.buffers; - // Avoid taking the lock if the pool is released or is over limit: - if (buffersLocal == null || this.index + bufferCount >= buffersLocal.Length + 1) + // Avoid taking the lock if the pool is is over it's limit: + if (this.index + bufferCount >= buffersLocal.Length + 1) { return null; } @@ -98,7 +99,7 @@ namespace SixLabors.ImageSharp.Memory.Internals lock (buffersLocal) { // Check again after taking the lock: - if (this.buffers == null || this.index + bufferCount >= buffersLocal.Length + 1) + if (this.index + bufferCount >= buffersLocal.Length + 1) { return null; } @@ -107,128 +108,138 @@ namespace SixLabors.ImageSharp.Memory.Internals for (int i = 0; i < bufferCount; i++) { result[i] = buffersLocal[this.index]; - buffersLocal[this.index++] = null; + buffersLocal[this.index++] = UnmanagedMemoryHandle.NullHandle; } } for (int i = 0; i < result.Length; i++) { - if (result[i] == null) + if (result[i].IsInvalid) { result[i] = UnmanagedMemoryHandle.Allocate(this.BufferLength); } - - if (allocationOptions.Has(AllocationOptions.Clean)) - { - this.GetSpan(result[i]).Clear(); - } } return result; } - public void Return(UnmanagedMemoryHandle buffer) + public void Return(UnmanagedMemoryHandle bufferHandle) { - UnmanagedMemoryHandle[] buffersLocal = this.buffers; - if (buffersLocal == null) - { - buffer.Dispose(); - return; - } - - lock (buffersLocal) + Guard.IsTrue(bufferHandle.IsValid, nameof(bufferHandle), "Returning NullHandle to the pool is not allowed."); + lock (this.buffers) { // Check again after taking the lock: - if (this.buffers == null) - { - buffer.Dispose(); - return; - } - if (this.index == 0) { ThrowReturnedMoreBuffersThanRented(); // DEBUG-only exception - buffer.Dispose(); + bufferHandle.Free(); return; } - this.buffers[--this.index] = buffer; + this.buffers[--this.index] = bufferHandle; } } - public void Return(Span buffers) + public void Return(Span bufferHandles) { - UnmanagedMemoryHandle[] buffersLocal = this.buffers; - if (buffersLocal == null) - { - DisposeAll(buffers); - return; - } - - lock (buffersLocal) + lock (this.buffers) { - // Check again after taking the lock: - if (this.buffers == null) - { - DisposeAll(buffers); - return; - } - - if (this.index - buffers.Length + 1 <= 0) + if (this.index - bufferHandles.Length + 1 <= 0) { ThrowReturnedMoreBuffersThanRented(); - DisposeAll(buffers); + DisposeAll(bufferHandles); return; } - for (int i = buffers.Length - 1; i >= 0; i--) + for (int i = bufferHandles.Length - 1; i >= 0; i--) { - buffersLocal[--this.index] = buffers[i]; + ref UnmanagedMemoryHandle h = ref bufferHandles[i]; + Guard.IsTrue(h.IsValid, nameof(bufferHandles), "Returning NullHandle to the pool is not allowed."); + this.buffers[--this.index] = bufferHandles[i]; } } } public void Release() { - this.trimTimer?.Dispose(); - this.trimTimer = null; - UnmanagedMemoryHandle[] oldBuffers = Interlocked.Exchange(ref this.buffers, null); - DebugGuard.NotNull(oldBuffers, nameof(oldBuffers)); - DisposeAll(oldBuffers); + lock (this.buffers) + { + for (int i = this.index; i < this.buffers.Length; i++) + { + UnmanagedMemoryHandle buffer = this.buffers[i]; + if (buffer.IsInvalid) + { + break; + } + + buffer.Free(); + this.buffers[i] = UnmanagedMemoryHandle.NullHandle; + } + } } private static void DisposeAll(Span buffers) { foreach (UnmanagedMemoryHandle handle in buffers) { - handle?.Dispose(); + handle.Free(); } } - private unsafe Span GetSpan(UnmanagedMemoryHandle h) => - new Span((byte*)h.DangerousGetHandle(), this.BufferLength); - // This indicates a bug in the library, however Return() might be called from a finalizer, // therefore we should never throw here in production. [Conditional("DEBUG")] private static void ThrowReturnedMoreBuffersThanRented() => throw new InvalidMemoryOperationException("Returned more buffers then rented"); - private static void TimerCallback(WeakReference weakPoolRef) + private static void UpdateTimer(TrimSettings settings, UniformUnmanagedMemoryPool pool) { - if (weakPoolRef.TryGetTarget(out UniformUnmanagedMemoryPool pool)) + lock (AllPools) { - pool.Trim(); + AllPools.Add(new WeakReference(pool)); + + // Invoke the timer callback more frequently, than trimSettings.TrimPeriodMilliseconds. + // We are checking in the callback if enough time passed since the last trimming. If not, we do nothing. + int period = settings.TrimPeriodMilliseconds / 4; + if (trimTimer == null) + { + trimTimer = new Timer(_ => TimerCallback(), null, period, period); + } + else if (settings.TrimPeriodMilliseconds < minTrimPeriodMilliseconds) + { + trimTimer.Change(period, period); + } + + minTrimPeriodMilliseconds = Math.Min(minTrimPeriodMilliseconds, settings.TrimPeriodMilliseconds); } } - private bool Trim() + private static void TimerCallback() { - UnmanagedMemoryHandle[] buffersLocal = this.buffers; - if (buffersLocal == null) + lock (AllPools) { - return false; + // Remove lost references from the list: + for (int i = AllPools.Count - 1; i >= 0; i--) + { + if (!AllPools[i].TryGetTarget(out _)) + { + AllPools.RemoveAt(i); + } + } + + foreach (WeakReference weakPoolRef in AllPools) + { + if (weakPoolRef.TryGetTarget(out UniformUnmanagedMemoryPool pool)) + { + pool.Trim(); + } + } } + } + + private bool Trim() + { + UnmanagedMemoryHandle[] buffersLocal = this.buffers; bool isHighPressure = this.IsHighMemoryPressure(); @@ -250,16 +261,11 @@ namespace SixLabors.ImageSharp.Memory.Internals { lock (buffersLocal) { - if (this.buffers == null) - { - return false; - } - // Trim all: - for (int i = this.index; i < buffersLocal.Length && buffersLocal[i] != null; i++) + for (int i = this.index; i < buffersLocal.Length && buffersLocal[i].IsValid; i++) { - buffersLocal[i].Dispose(); - buffersLocal[i] = null; + buffersLocal[i].Free(); + buffersLocal[i] = UnmanagedMemoryHandle.NullHandle; } } @@ -270,14 +276,9 @@ namespace SixLabors.ImageSharp.Memory.Internals { lock (buffersLocal) { - if (this.buffers == null) - { - return false; - } - // Count the buffers in the pool: int retainedCount = 0; - for (int i = this.index; i < buffersLocal.Length && buffersLocal[i] != null; i++) + for (int i = this.index; i < buffersLocal.Length && buffersLocal[i].IsValid; i++) { retainedCount++; } @@ -288,8 +289,8 @@ namespace SixLabors.ImageSharp.Memory.Internals int trimStop = this.index + retainedCount - trimCount; for (int i = trimStart; i >= trimStop; i--) { - buffersLocal[i].Dispose(); - buffersLocal[i] = null; + buffersLocal[i].Free(); + buffersLocal[i] = UnmanagedMemoryHandle.NullHandle; } this.lastTrimTimestamp = Stopwatch.ElapsedMilliseconds; diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs new file mode 100644 index 0000000000..d59f04c930 --- /dev/null +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs @@ -0,0 +1,27 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +namespace SixLabors.ImageSharp.Memory.Internals +{ + /// + /// Defines a strategy for managing unmanaged memory ownership. + /// + internal abstract class UnmanagedBufferLifetimeGuard : RefCountedLifetimeGuard + { + private UnmanagedMemoryHandle handle; + + protected UnmanagedBufferLifetimeGuard(UnmanagedMemoryHandle handle) => this.handle = handle; + + public UnmanagedMemoryHandle Handle => this.handle; + + public sealed class FreeHandle : UnmanagedBufferLifetimeGuard + { + public FreeHandle(UnmanagedMemoryHandle handle) + : base(handle) + { + } + + protected override void Release() => this.handle.Free(); + } + } +} diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBuffer{T}.cs index c343e44a8d..5d0c6dd613 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBuffer{T}.cs @@ -5,6 +5,7 @@ using System; using System.Buffers; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Threading; namespace SixLabors.ImageSharp.Memory.Internals { @@ -13,58 +14,67 @@ namespace SixLabors.ImageSharp.Memory.Internals /// access to unmanaged buffers allocated by . /// /// The element type. - internal unsafe class UnmanagedBuffer : MemoryManager + internal sealed unsafe class UnmanagedBuffer : MemoryManager, IRefCounted where T : struct { private readonly int lengthInElements; - /// - /// Initializes a new instance of the class. - /// - /// The number of elements to allocate. - public UnmanagedBuffer(int lengthInElements) - : this(UnmanagedMemoryHandle.Allocate(lengthInElements * Unsafe.SizeOf()), lengthInElements) - { - } + private readonly UnmanagedBufferLifetimeGuard lifetimeGuard; + + private int disposed; - protected UnmanagedBuffer(UnmanagedMemoryHandle bufferHandle, int lengthInElements) + public UnmanagedBuffer(int lengthInElements, UnmanagedBufferLifetimeGuard lifetimeGuard) { + DebugGuard.NotNull(lifetimeGuard, nameof(lifetimeGuard)); + this.lengthInElements = lengthInElements; - this.BufferHandle = bufferHandle; + this.lifetimeGuard = lifetimeGuard; } - public UnmanagedMemoryHandle BufferHandle { get; protected set; } - - private void* Pointer => (void*)this.BufferHandle.DangerousGetHandle(); + private void* Pointer => this.lifetimeGuard.Handle.Pointer; - public override Span GetSpan() => new(this.Pointer, this.lengthInElements); + public override Span GetSpan() + { + DebugGuard.NotDisposed(this.disposed == 1, this.GetType().Name); + DebugGuard.NotDisposed(this.lifetimeGuard.IsDisposed, this.lifetimeGuard.GetType().Name); + return new(this.Pointer, this.lengthInElements); + } /// public override MemoryHandle Pin(int elementIndex = 0) { + DebugGuard.NotDisposed(this.disposed == 1, this.GetType().Name); + DebugGuard.NotDisposed(this.lifetimeGuard.IsDisposed, this.lifetimeGuard.GetType().Name); + // Will be released in Unpin - bool unused = false; - this.BufferHandle.DangerousAddRef(ref unused); + this.lifetimeGuard.AddRef(); void* pbData = Unsafe.Add(this.Pointer, elementIndex); return new MemoryHandle(pbData, pinnable: this); } - /// - public override void Unpin() => this.BufferHandle.DangerousRelease(); - /// protected override void Dispose(bool disposing) { - if (this.BufferHandle.IsInvalid) + DebugGuard.IsTrue(disposing, nameof(disposing), "Unmanaged buffers should not have finalizer!"); + + if (Interlocked.Exchange(ref this.disposed, 1) == 1) { + // Already disposed return; } - if (disposing) - { - this.BufferHandle.Dispose(); - } + this.lifetimeGuard.Dispose(); } + + /// + public override void Unpin() => this.lifetimeGuard.ReleaseRef(); + + public void AddRef() => this.lifetimeGuard.AddRef(); + + public void ReleaseRef() => this.lifetimeGuard.ReleaseRef(); + + public static UnmanagedBuffer Allocate(int lengthInElements) => + new(lengthInElements, new UnmanagedBufferLifetimeGuard.FreeHandle(UnmanagedMemoryHandle.Allocate(lengthInElements * Unsafe.SizeOf()))); } } diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs index 12ea933bb9..ce2fab60a2 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs @@ -2,20 +2,20 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Threading; -using Microsoft.Win32.SafeHandles; namespace SixLabors.ImageSharp.Memory.Internals { - internal sealed class UnmanagedMemoryHandle : SafeHandle + /// + /// Encapsulates the functionality around allocating and releasing unmanaged memory. NOT a . + /// + internal struct UnmanagedMemoryHandle : IEquatable { - // Number of allocation re-attempts when OutOfMemoryException is thrown. + // Number of allocation re-attempts when detecting OutOfMemoryException. private const int MaxAllocationAttempts = 1000; - private readonly int lengthInBytes; - private bool resurrected; - // Track allocations for testing purposes: private static int totalOutstandingHandles; @@ -24,10 +24,16 @@ namespace SixLabors.ImageSharp.Memory.Internals // A Monitor to wait/signal when we are low on memory. private static object lowMemoryMonitor; + public static readonly UnmanagedMemoryHandle NullHandle = default; + + private IntPtr handle; + private readonly int lengthInBytes; + private UnmanagedMemoryHandle(IntPtr handle, int lengthInBytes) - : base(handle, true) { + this.handle = handle; this.lengthInBytes = lengthInBytes; + if (lengthInBytes > 0) { GC.AddMemoryPressure(lengthInBytes); @@ -36,6 +42,14 @@ namespace SixLabors.ImageSharp.Memory.Internals Interlocked.Increment(ref totalOutstandingHandles); } + public IntPtr Handle => this.handle; + + public bool IsInvalid => this.Handle == IntPtr.Zero; + + public bool IsValid => this.Handle != IntPtr.Zero; + + public unsafe void* Pointer => (void*)this.Handle; + /// /// Gets the total outstanding handle allocations for testing purposes. /// @@ -46,36 +60,34 @@ namespace SixLabors.ImageSharp.Memory.Internals /// internal static long TotalOomRetries => totalOomRetries; - /// - public override bool IsInvalid => this.handle == IntPtr.Zero; + public static bool operator ==(UnmanagedMemoryHandle a, UnmanagedMemoryHandle b) => a.Equals(b); + + public static bool operator !=(UnmanagedMemoryHandle a, UnmanagedMemoryHandle b) => !a.Equals(b); - protected override bool ReleaseHandle() + [MethodImpl(InliningOptions.HotPath)] + public unsafe Span GetSpan() { if (this.IsInvalid) { - return false; + ThrowDisposed(); } - Marshal.FreeHGlobal(this.handle); - if (this.lengthInBytes > 0) - { - GC.RemoveMemoryPressure(this.lengthInBytes); - } + return new Span(this.Pointer, this.lengthInBytes); + } - if (lowMemoryMonitor != null) + [MethodImpl(InliningOptions.HotPath)] + public unsafe Span GetSpan(int lengthInBytes) + { + DebugGuard.MustBeLessThanOrEqualTo(lengthInBytes, this.lengthInBytes, nameof(lengthInBytes)); + if (this.IsInvalid) { - // We are low on memory. Signal all threads waiting in AllocateHandle(). - Monitor.Enter(lowMemoryMonitor); - Monitor.PulseAll(lowMemoryMonitor); - Monitor.Exit(lowMemoryMonitor); + ThrowDisposed(); } - this.handle = IntPtr.Zero; - Interlocked.Decrement(ref totalOutstandingHandles); - return true; + return new Span(this.Pointer, lengthInBytes); } - internal static UnmanagedMemoryHandle Allocate(int lengthInBytes) + public static UnmanagedMemoryHandle Allocate(int lengthInBytes) { IntPtr handle = AllocateHandle(lengthInBytes); return new UnmanagedMemoryHandle(handle, lengthInBytes); @@ -115,26 +127,38 @@ namespace SixLabors.ImageSharp.Memory.Internals return handle; } - /// - /// UnmanagedMemoryHandle's finalizer would release the underlying handle returning the memory to the OS. - /// We want to prevent this when a finalizable owner (buffer or MemoryGroup) is returning the handle to - /// in it's finalizer. - /// Since UnmanagedMemoryHandle is CriticalFinalizable, it is guaranteed that the owner's finalizer is called first. - /// - internal void Resurrect() + public void Free() { - GC.SuppressFinalize(this); - this.resurrected = true; - } + IntPtr h = Interlocked.Exchange(ref this.handle, IntPtr.Zero); - internal void AssignedToNewOwner() - { - if (this.resurrected) + if (h == IntPtr.Zero) + { + return; + } + + Marshal.FreeHGlobal(h); + Interlocked.Decrement(ref totalOutstandingHandles); + if (this.lengthInBytes > 0) + { + GC.RemoveMemoryPressure(this.lengthInBytes); + } + + if (Volatile.Read(ref lowMemoryMonitor) != null) { - // The handle has been resurrected - GC.ReRegisterForFinalize(this); - this.resurrected = false; + // We are low on memory. Signal all threads waiting in AllocateHandle(). + Monitor.Enter(lowMemoryMonitor); + Monitor.PulseAll(lowMemoryMonitor); + Monitor.Exit(lowMemoryMonitor); } } + + public bool Equals(UnmanagedMemoryHandle other) => this.handle.Equals(other.handle); + + public override bool Equals(object obj) => obj is UnmanagedMemoryHandle other && this.Equals(other); + + public override int GetHashCode() => this.handle.GetHashCode(); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowDisposed() => throw new ObjectDisposedException(nameof(UnmanagedMemoryHandle)); } } diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index d9ebb7cb95..6649468dab 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -11,26 +11,15 @@ namespace SixLabors.ImageSharp.Memory /// public abstract class MemoryAllocator { - private static MemoryAllocator defaultMemoryAllocator = Create(); - /// - /// Gets or sets the default global instance for the current process. + /// Gets the default platform-specific global instance that + /// serves as the default value for . + /// + /// This is a get-only property, + /// you should set 's + /// to change the default allocator used by and it's operations. /// - /// - /// Since is lazy-initialized, setting the value of - /// will only override 's - /// before the first read of the property. - /// After that, a manual assigment of is necessary. - /// - public static MemoryAllocator Default - { - get => defaultMemoryAllocator; - set - { - Guard.NotNull(value, nameof(Default)); - defaultMemoryAllocator = value; - } - } + public static MemoryAllocator Default { get; } = Create(); /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index ecc30c97cd..b6d1abddff 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -98,15 +98,10 @@ namespace SixLabors.ImageSharp.Memory if (lengthInBytes <= this.poolBufferSizeInBytes) { - UnmanagedMemoryHandle array = this.pool.Rent(); - if (array != null) + UnmanagedMemoryHandle mem = this.pool.Rent(); + if (mem.IsValid) { - var buffer = new UniformUnmanagedMemoryPool.FinalizableBuffer(this.pool, array, length); - if (options.Has(AllocationOptions.Clean)) - { - buffer.Clear(); - } - + UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, length, options); return buffer; } } @@ -130,15 +125,10 @@ namespace SixLabors.ImageSharp.Memory if (totalLengthInBytes <= this.poolBufferSizeInBytes) { // Optimized path renting single array from the pool - UnmanagedMemoryHandle array = this.pool.Rent(); - if (array != null) + UnmanagedMemoryHandle mem = this.pool.Rent(); + if (mem.IsValid) { - var buffer = new UniformUnmanagedMemoryPool.FinalizableBuffer(this.pool, array, (int)totalLength); - if (options.Has(AllocationOptions.Clean)) - { - buffer.Clear(); - } - + UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLength, options); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } } @@ -152,13 +142,7 @@ namespace SixLabors.ImageSharp.Memory return MemoryGroup.Allocate(this.nonPoolAllocator, totalLength, bufferAlignment, options); } - public override void ReleaseRetainedResources() - { - UniformUnmanagedMemoryPool oldPool = Interlocked.Exchange( - ref this.pool, - new UniformUnmanagedMemoryPool(this.poolBufferSizeInBytes, this.poolCapacity, this.trimSettings)); - oldPool.Release(); - } + public override void ReleaseRetainedResources() => this.pool.Release(); private static long GetDefaultMaxPoolSizeBytes() { diff --git a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs index 731c8e0149..9b0869c403 100644 --- a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs @@ -20,7 +20,7 @@ namespace SixLabors.ImageSharp.Memory public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - var buffer = new UnmanagedBuffer(length); + var buffer = UnmanagedBuffer.Allocate(length); if (options.Has(AllocationOptions.Clean)) { buffer.GetSpan().Clear(); diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index 6f92e48645..a59602efac 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -18,9 +18,7 @@ namespace SixLabors.ImageSharp.Memory public sealed class Owned : MemoryGroup, IEnumerable> { private IMemoryOwner[] memoryOwners; - private byte[][] pooledArrays; - private UniformUnmanagedMemoryPool unmanagedMemoryPool; - private UnmanagedMemoryHandle[] pooledHandles; + private RefCountedLifetimeGuard groupLifetimeGuard; public Owned(IMemoryOwner[] memoryOwners, int bufferLength, long totalLength, bool swappable) : base(bufferLength, totalLength) @@ -30,14 +28,15 @@ namespace SixLabors.ImageSharp.Memory this.View = new MemoryGroupView(this); } - public Owned(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle[] pooledArrays, int bufferLength, long totalLength, int sizeOfLastBuffer, AllocationOptions options) - : this(CreateBuffers(pool, pooledArrays, bufferLength, sizeOfLastBuffer, options), bufferLength, totalLength, true) - { - this.pooledHandles = pooledArrays; - this.unmanagedMemoryPool = pool; - } - - ~Owned() => this.Dispose(false); + public Owned( + UniformUnmanagedMemoryPool pool, + UnmanagedMemoryHandle[] pooledHandles, + int bufferLength, + long totalLength, + int sizeOfLastBuffer, + AllocationOptions options) + : this(CreateBuffers(pooledHandles, bufferLength, sizeOfLastBuffer, options), bufferLength, totalLength, true) => + this.groupLifetimeGuard = pool.CreateGroupLifetimeGuard(pooledHandles); public bool Swappable { get; } @@ -63,7 +62,6 @@ namespace SixLabors.ImageSharp.Memory } private static IMemoryOwner[] CreateBuffers( - UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle[] pooledBuffers, int bufferLength, int sizeOfLastBuffer, @@ -72,42 +70,35 @@ namespace SixLabors.ImageSharp.Memory var result = new IMemoryOwner[pooledBuffers.Length]; for (int i = 0; i < pooledBuffers.Length - 1; i++) { - pooledBuffers[i].AssignedToNewOwner(); - var currentBuffer = new UniformUnmanagedMemoryPool.Buffer(pool, pooledBuffers[i], bufferLength); - if (options.Has(AllocationOptions.Clean)) - { - currentBuffer.Clear(); - } - + var currentBuffer = ObservedBuffer.Create(pooledBuffers[i], bufferLength, options); result[i] = currentBuffer; } - var lastBuffer = new UniformUnmanagedMemoryPool.Buffer(pool, pooledBuffers[pooledBuffers.Length - 1], sizeOfLastBuffer); - if (options.Has(AllocationOptions.Clean)) - { - lastBuffer.Clear(); - } - + var lastBuffer = ObservedBuffer.Create(pooledBuffers[pooledBuffers.Length - 1], sizeOfLastBuffer, options); result[result.Length - 1] = lastBuffer; return result; } /// [MethodImpl(InliningOptions.ShortMethod)] - public override MemoryGroupEnumerator GetEnumerator() - { - return new MemoryGroupEnumerator(this); - } + public override MemoryGroupEnumerator GetEnumerator() => new(this); public override void IncreaseRefCounts() { this.EnsureNotDisposed(); - bool dummy = default; - foreach (IMemoryOwner memoryOwner in this.memoryOwners) + + if (this.groupLifetimeGuard != null) + { + this.groupLifetimeGuard.AddRef(); + } + else { - if (memoryOwner is UnmanagedBuffer unmanagedBuffer) + foreach (IMemoryOwner memoryOwner in this.memoryOwners) { - unmanagedBuffer.BufferHandle?.DangerousAddRef(ref dummy); + if (memoryOwner is IRefCounted unmanagedBuffer) + { + unmanagedBuffer.AddRef(); + } } } } @@ -115,11 +106,18 @@ namespace SixLabors.ImageSharp.Memory public override void DecreaseRefCounts() { this.EnsureNotDisposed(); - foreach (IMemoryOwner memoryOwner in this.memoryOwners) + if (this.groupLifetimeGuard != null) { - if (memoryOwner is UnmanagedBuffer unmanagedBuffer) + this.groupLifetimeGuard.ReleaseRef(); + } + else + { + foreach (IMemoryOwner memoryOwner in this.memoryOwners) { - unmanagedBuffer.BufferHandle?.DangerousRelease(); + if (memoryOwner is IRefCounted unmanagedBuffer) + { + unmanagedBuffer.ReleaseRef(); + } } } } @@ -133,34 +131,18 @@ namespace SixLabors.ImageSharp.Memory protected override void Dispose(bool disposing) { - if (this.IsDisposed) + if (this.IsDisposed || !disposing) { return; } this.View.Invalidate(); - if (this.unmanagedMemoryPool != null) + if (this.groupLifetimeGuard != null) { - this.unmanagedMemoryPool.Return(this.pooledHandles); - if (!disposing) - { - foreach (UnmanagedMemoryHandle handle in this.pooledHandles) - { - // We need to prevent handle finalization here. - // See comments on UnmanagedMemoryHandle.Resurrect() - handle.Resurrect(); - } - } - - foreach (IMemoryOwner memoryOwner in this.memoryOwners) - { - ((UniformUnmanagedMemoryPool.Buffer)memoryOwner).MarkDisposed(); - } - - GC.SuppressFinalize(this); + this.groupLifetimeGuard.Dispose(); } - else if (disposing) + else { foreach (IMemoryOwner memoryOwner in this.memoryOwners) { @@ -170,9 +152,7 @@ namespace SixLabors.ImageSharp.Memory this.memoryOwners = null; this.IsValid = false; - this.pooledArrays = null; - this.unmanagedMemoryPool = null; - this.pooledHandles = null; + this.groupLifetimeGuard = null; } [MethodImpl(InliningOptions.ShortMethod)] @@ -195,29 +175,67 @@ namespace SixLabors.ImageSharp.Memory IMemoryOwner[] tempOwners = a.memoryOwners; long tempTotalLength = a.TotalLength; int tempBufferLength = a.BufferLength; - byte[][] tempPooledArrays = a.pooledArrays; - UniformUnmanagedMemoryPool tempUnmangedPool = a.unmanagedMemoryPool; - UnmanagedMemoryHandle[] tempPooledHandles = a.pooledHandles; + RefCountedLifetimeGuard tempGroupOwner = a.groupLifetimeGuard; a.memoryOwners = b.memoryOwners; a.TotalLength = b.TotalLength; a.BufferLength = b.BufferLength; - a.pooledArrays = b.pooledArrays; - a.unmanagedMemoryPool = b.unmanagedMemoryPool; - a.pooledHandles = b.pooledHandles; + a.groupLifetimeGuard = b.groupLifetimeGuard; b.memoryOwners = tempOwners; b.TotalLength = tempTotalLength; b.BufferLength = tempBufferLength; - b.pooledArrays = tempPooledArrays; - b.unmanagedMemoryPool = tempUnmangedPool; - b.pooledHandles = tempPooledHandles; + b.groupLifetimeGuard = tempGroupOwner; a.View.Invalidate(); b.View.Invalidate(); a.View = new MemoryGroupView(a); b.View = new MemoryGroupView(b); } + + // No-ownership + private sealed class ObservedBuffer : MemoryManager + { + private readonly UnmanagedMemoryHandle handle; + private readonly int lengthInElements; + + private ObservedBuffer(UnmanagedMemoryHandle handle, int lengthInElements) + { + this.handle = handle; + this.lengthInElements = lengthInElements; + } + + public static ObservedBuffer Create( + UnmanagedMemoryHandle handle, + int lengthInElements, + AllocationOptions options) + { + var buffer = new ObservedBuffer(handle, lengthInElements); + if (options.Has(AllocationOptions.Clean)) + { + buffer.GetSpan().Clear(); + } + + return buffer; + } + + protected override void Dispose(bool disposing) + { + // No-op. + } + + public override unsafe Span GetSpan() => new(this.handle.Pointer, this.lengthInElements); + + public override unsafe MemoryHandle Pin(int elementIndex = 0) + { + void* pbData = Unsafe.Add(this.handle.Pointer, elementIndex); + return new MemoryHandle(pbData); + } + + public override void Unpin() + { + } + } } } } diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 5ff17f0c20..0fcbd6f960 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -192,7 +192,7 @@ namespace SixLabors.ImageSharp.Memory bufferCount++; } - UnmanagedMemoryHandle[] arrays = pool.Rent(bufferCount, options); + UnmanagedMemoryHandle[] arrays = pool.Rent(bufferCount); if (arrays == null) { diff --git a/tests/ImageSharp.Tests/ConfigurationTests.cs b/tests/ImageSharp.Tests/ConfigurationTests.cs index 491f717cca..f77db33f06 100644 --- a/tests/ImageSharp.Tests/ConfigurationTests.cs +++ b/tests/ImageSharp.Tests/ConfigurationTests.cs @@ -156,17 +156,14 @@ namespace SixLabors.ImageSharp.Tests static void RunTest() { - MemoryAllocator allocator = new TestMemoryAllocator(); - MemoryAllocator.Default = allocator; - var c1 = new Configuration(); var c2 = new Configuration(new MockConfigurationModule()); var c3 = Configuration.CreateDefaultInstance(); - Assert.Same(allocator, Configuration.Default.MemoryAllocator); - Assert.Same(allocator, c1.MemoryAllocator); - Assert.Same(allocator, c2.MemoryAllocator); - Assert.Same(allocator, c3.MemoryAllocator); + Assert.Same(MemoryAllocator.Default, Configuration.Default.MemoryAllocator); + Assert.Same(MemoryAllocator.Default, c1.MemoryAllocator); + Assert.Same(MemoryAllocator.Default, c2.MemoryAllocator); + Assert.Same(MemoryAllocator.Default, c3.MemoryAllocator); } } diff --git a/tests/ImageSharp.Tests/Image/ImageCloneTests.cs b/tests/ImageSharp.Tests/Image/ImageCloneTests.cs index 8cca00a732..f602643341 100644 --- a/tests/ImageSharp.Tests/Image/ImageCloneTests.cs +++ b/tests/ImageSharp.Tests/Image/ImageCloneTests.cs @@ -134,7 +134,6 @@ namespace SixLabors.ImageSharp.Tests } } }); - } } } diff --git a/tests/ImageSharp.Tests/Image/ProcessPixelRowsTestBase.cs b/tests/ImageSharp.Tests/Image/ProcessPixelRowsTestBase.cs index 255e1a9a4c..fa0752e775 100644 --- a/tests/ImageSharp.Tests/Image/ProcessPixelRowsTestBase.cs +++ b/tests/ImageSharp.Tests/Image/ProcessPixelRowsTestBase.cs @@ -181,7 +181,7 @@ namespace SixLabors.ImageSharp.Tests static void RunTest(string testTypeName, string throwExceptionStr) { bool throwExceptionInner = bool.Parse(throwExceptionStr); - var buffer = new UnmanagedBuffer(100); + var buffer = UnmanagedBuffer.Allocate(100); var allocator = new MockUnmanagedMemoryAllocator(buffer); Configuration.Default.MemoryAllocator = allocator; @@ -192,7 +192,7 @@ namespace SixLabors.ImageSharp.Tests { GetTest(testTypeName).ProcessPixelRowsImpl(image, _ => { - buffer.BufferHandle.Dispose(); + ((IDisposable)buffer).Dispose(); Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); if (throwExceptionInner) { @@ -218,8 +218,8 @@ namespace SixLabors.ImageSharp.Tests static void RunTest(string testTypeName, string throwExceptionStr) { bool throwExceptionInner = bool.Parse(throwExceptionStr); - var buffer1 = new UnmanagedBuffer(100); - var buffer2 = new UnmanagedBuffer(100); + var buffer1 = UnmanagedBuffer.Allocate(100); + var buffer2 = UnmanagedBuffer.Allocate(100); var allocator = new MockUnmanagedMemoryAllocator(buffer1, buffer2); Configuration.Default.MemoryAllocator = allocator; @@ -231,8 +231,8 @@ namespace SixLabors.ImageSharp.Tests { GetTest(testTypeName).ProcessPixelRowsImpl(image1, image2, (_, _) => { - buffer1.BufferHandle.Dispose(); - buffer2.BufferHandle.Dispose(); + ((IDisposable)buffer1).Dispose(); + ((IDisposable)buffer2).Dispose(); Assert.Equal(2, UnmanagedMemoryHandle.TotalOutstandingHandles); if (throwExceptionInner) { @@ -258,9 +258,9 @@ namespace SixLabors.ImageSharp.Tests static void RunTest(string testTypeName, string throwExceptionStr) { bool throwExceptionInner = bool.Parse(throwExceptionStr); - var buffer1 = new UnmanagedBuffer(100); - var buffer2 = new UnmanagedBuffer(100); - var buffer3 = new UnmanagedBuffer(100); + var buffer1 = UnmanagedBuffer.Allocate(100); + var buffer2 = UnmanagedBuffer.Allocate(100); + var buffer3 = UnmanagedBuffer.Allocate(100); var allocator = new MockUnmanagedMemoryAllocator(buffer1, buffer2, buffer3); Configuration.Default.MemoryAllocator = allocator; @@ -273,9 +273,9 @@ namespace SixLabors.ImageSharp.Tests { GetTest(testTypeName).ProcessPixelRowsImpl(image1, image2, image3, (_, _, _) => { - buffer1.BufferHandle.Dispose(); - buffer2.BufferHandle.Dispose(); - buffer3.BufferHandle.Dispose(); + ((IDisposable)buffer1).Dispose(); + ((IDisposable)buffer2).Dispose(); + ((IDisposable)buffer3).Dispose(); Assert.Equal(3, UnmanagedMemoryHandle.TotalOutstandingHandles); if (throwExceptionInner) { @@ -317,7 +317,7 @@ namespace SixLabors.ImageSharp.Tests protected internal override int GetBufferCapacityInBytes() => int.MaxValue; public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) => - (IMemoryOwner)this.buffers.Pop(); + this.buffers.Pop() as IMemoryOwner; } } } diff --git a/tests/ImageSharp.Tests/Memory/Allocators/RefCountingLifetimeGuardTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/RefCountingLifetimeGuardTests.cs new file mode 100644 index 0000000000..ab1aab74a6 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Allocators/RefCountingLifetimeGuardTests.cs @@ -0,0 +1,116 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Runtime.CompilerServices; +using Microsoft.DotNet.RemoteExecutor; +using SixLabors.ImageSharp.Memory.Internals; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory.Allocators +{ + public class RefCountingLifetimeGuardTests + { + [Theory] + [InlineData(1)] + [InlineData(3)] + public void Dispose_ResultsInSingleRelease(int disposeCount) + { + var guard = new MockLifetimeGuard(); + Assert.Equal(0, guard.ReleaseInvocationCount); + + for (int i = 0; i < disposeCount; i++) + { + guard.Dispose(); + } + + Assert.Equal(1, guard.ReleaseInvocationCount); + } + + [Fact] + public void Finalize_ResultsInSingleRelease() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + Assert.Equal(0, MockLifetimeGuard.GlobalReleaseInvocationCount); + LeakGuard(false); + GC.Collect(); + GC.WaitForPendingFinalizers(); + Assert.Equal(1, MockLifetimeGuard.GlobalReleaseInvocationCount); + } + } + + [Theory] + [InlineData(1)] + [InlineData(3)] + public void AddRef_PreventsReleaseOnDispose(int addRefCount) + { + var guard = new MockLifetimeGuard(); + + for (int i = 0; i < addRefCount; i++) + { + guard.AddRef(); + } + + guard.Dispose(); + + for (int i = 0; i < addRefCount; i++) + { + Assert.Equal(0, guard.ReleaseInvocationCount); + guard.ReleaseRef(); + } + + Assert.Equal(1, guard.ReleaseInvocationCount); + } + + [Fact] + public void AddRef_PreventsReleaseOnFinalize() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + LeakGuard(true); + GC.Collect(); + GC.WaitForPendingFinalizers(); + Assert.Equal(0, MockLifetimeGuard.GlobalReleaseInvocationCount); + } + } + + [Fact] + public void AddRefReleaseRefMisuse_DoesntLeadToMultipleReleases() + { + var guard = new MockLifetimeGuard(); + guard.Dispose(); + guard.AddRef(); + guard.ReleaseRef(); + + Assert.Equal(1, guard.ReleaseInvocationCount); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void LeakGuard(bool addRef) + { + var guard = new MockLifetimeGuard(); + if (addRef) + { + guard.AddRef(); + } + } + + private class MockLifetimeGuard : RefCountedLifetimeGuard + { + public int ReleaseInvocationCount { get; private set; } + + public static int GlobalReleaseInvocationCount { get; private set; } + + protected override void Release() + { + this.ReleaseInvocationCount++; + GlobalReleaseInvocationCount++; + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SharedArrayPoolBufferTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SharedArrayPoolBufferTests.cs new file mode 100644 index 0000000000..fab520e190 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Allocators/SharedArrayPoolBufferTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Buffers; +using System.Linq; +using Microsoft.DotNet.RemoteExecutor; +using SixLabors.ImageSharp.Memory.Internals; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory.Allocators +{ + public class SharedArrayPoolBufferTests + { + [Fact] + public void AllocatesArrayPoolArray() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + using (var buffer = new SharedArrayPoolBuffer(900)) + { + Assert.Equal(900, buffer.GetSpan().Length); + buffer.GetSpan().Fill(42); + } + + byte[] array = ArrayPool.Shared.Rent(900); + byte[] expected = Enumerable.Repeat((byte)42, 900).ToArray(); + + Assert.True(expected.AsSpan().SequenceEqual(array.AsSpan(0, 900))); + } + } + + [Fact] + public void OutstandingReferences_RetainArrays() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + var buffer = new SharedArrayPoolBuffer(900); + Span span = buffer.GetSpan(); + + buffer.AddRef(); + ((IDisposable)buffer).Dispose(); + span.Fill(42); + byte[] array = ArrayPool.Shared.Rent(900); + Assert.NotEqual(42, array[0]); + ArrayPool.Shared.Return(array); + + buffer.ReleaseRef(); + array = ArrayPool.Shared.Rent(900); + byte[] expected = Enumerable.Repeat((byte)42, 900).ToArray(); + Assert.True(expected.AsSpan().SequenceEqual(array.AsSpan(0, 900))); + ArrayPool.Shared.Return(array); + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.Trim.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.Trim.cs index 9726c9c585..35d2237c0f 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.Trim.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.Trim.cs @@ -3,7 +3,9 @@ using System; using System.Collections.Generic; -using System.Linq; +using System.IO; +using System.Runtime.CompilerServices; +using System.Text; using System.Threading; using Microsoft.DotNet.RemoteExecutor; using SixLabors.ImageSharp.Memory.Internals; @@ -13,14 +15,14 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { public partial class UniformUnmanagedMemoryPoolTests { - [CollectionDefinition(nameof(NonParallelTests), DisableParallelization = true)] - public class NonParallelTests - { - } - [Collection(nameof(NonParallelTests))] public class Trim { + [CollectionDefinition(nameof(NonParallelTests), DisableParallelization = true)] + public class NonParallelTests + { + } + [Fact] public void TrimPeriodElapsed_TrimsHalfOfUnusedArrays() { @@ -45,18 +47,56 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators } } + [Fact] + public void MultiplePoolInstances_TrimPeriodElapsed_AllAreTrimmed() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + var trimSettings1 = new UniformUnmanagedMemoryPool.TrimSettings { TrimPeriodMilliseconds = 6_000 }; + var pool1 = new UniformUnmanagedMemoryPool(128, 256, trimSettings1); + Thread.Sleep(8_000); // Let some callbacks fire already + var trimSettings2 = new UniformUnmanagedMemoryPool.TrimSettings { TrimPeriodMilliseconds = 3_000 }; + var pool2 = new UniformUnmanagedMemoryPool(128, 256, trimSettings2); + + pool1.Return(pool1.Rent(64)); + pool2.Return(pool2.Rent(64)); + Assert.Equal(128, UnmanagedMemoryHandle.TotalOutstandingHandles); + + // This exercises pool weak reference list trimming: + LeakPoolInstance(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + Assert.Equal(128, UnmanagedMemoryHandle.TotalOutstandingHandles); + + Thread.Sleep(15_000); + Assert.True( + UnmanagedMemoryHandle.TotalOutstandingHandles <= 64, + $"UnmanagedMemoryHandle.TotalOutstandingHandles={UnmanagedMemoryHandle.TotalOutstandingHandles} > 80"); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void LeakPoolInstance() + { + var trimSettings = new UniformUnmanagedMemoryPool.TrimSettings { TrimPeriodMilliseconds = 4_000 }; + _ = new UniformUnmanagedMemoryPool(128, 256, trimSettings); + } + } + #if NETCORE31COMPATIBLE public static readonly bool Is32BitProcess = !Environment.Is64BitProcess; - private static readonly List PressureArrays = new List(); + private static readonly List PressureArrays = new(); [ConditionalFact(nameof(Is32BitProcess))] public static void GC_Collect_OnHighLoad_TrimsEntirePool() { RemoteExecutor.Invoke(RunTest).Dispose(); + static void RunTest() { Assert.False(Environment.Is64BitProcess); - const int OneMb = 1024 * 1024; + const int OneMb = 1 << 20; var trimSettings = new UniformUnmanagedMemoryPool.TrimSettings { HighPressureThresholdRate = 0.2f }; @@ -82,6 +122,9 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators Assert.Equal(0, UnmanagedMemoryHandle.TotalOutstandingHandles); + // Prevent eager collection of the pool: + GC.KeepAlive(pool); + static void TouchPage(byte[] b) { uint size = (uint)b.Length; diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.cs index 86bbff1812..021d071fed 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedMemoryPoolTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; @@ -17,13 +18,42 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { private readonly ITestOutputHelper output; - public UniformUnmanagedMemoryPoolTests(ITestOutputHelper output) + public UniformUnmanagedMemoryPoolTests(ITestOutputHelper output) => this.output = output; + + private class CleanupUtil : IDisposable { - this.output = output; - } + private readonly UniformUnmanagedMemoryPool pool; + private readonly List handlesToDestroy = new(); + private readonly List ptrsToDestroy = new(); + + public CleanupUtil(UniformUnmanagedMemoryPool pool) + { + this.pool = pool; + } + + public void Register(UnmanagedMemoryHandle handle) => this.handlesToDestroy.Add(handle); - private static unsafe Span GetSpan(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle h) => - new Span((void*)h.DangerousGetHandle(), pool.BufferLength); + public void Register(IEnumerable handles) => this.handlesToDestroy.AddRange(handles); + + public void Register(IntPtr memoryPtr) => this.ptrsToDestroy.Add(memoryPtr); + + public void Register(IEnumerable memoryPtrs) => this.ptrsToDestroy.AddRange(memoryPtrs); + + public void Dispose() + { + foreach (UnmanagedMemoryHandle handle in this.handlesToDestroy) + { + handle.Free(); + } + + this.pool.Release(); + + foreach (IntPtr ptr in this.ptrsToDestroy) + { + Marshal.FreeHGlobal(ptr); + } + } + } [Theory] [InlineData(3, 11)] @@ -41,10 +71,13 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators public void Rent_SingleBuffer_ReturnsCorrectBuffer(int length, int capacity) { var pool = new UniformUnmanagedMemoryPool(length, capacity); + using var cleanup = new CleanupUtil(pool); + for (int i = 0; i < capacity; i++) { UnmanagedMemoryHandle h = pool.Rent(); CheckBuffer(length, pool, h); + cleanup.Register(h); } } @@ -68,9 +101,8 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators private static void CheckBuffer(int length, UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle h) { - Assert.NotNull(h); - Assert.False(h.IsClosed); - Span span = GetSpan(pool, h); + Assert.False(h.IsInvalid); + Span span = h.GetSpan(); span.Fill(123); byte[] expected = new byte[length]; @@ -86,7 +118,10 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators public void Rent_MultiBuffer_ReturnsCorrectBuffers(int length, int bufferCount) { var pool = new UniformUnmanagedMemoryPool(length, 10); + using var cleanup = new CleanupUtil(pool); UnmanagedMemoryHandle[] handles = pool.Rent(bufferCount); + cleanup.Register(handles); + Assert.NotNull(handles); Assert.Equal(bufferCount, handles.Length); @@ -100,12 +135,15 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators public void Rent_MultipleTimesWithoutReturn_ReturnsDifferentHandles() { var pool = new UniformUnmanagedMemoryPool(128, 10); + using var cleanup = new CleanupUtil(pool); UnmanagedMemoryHandle[] a = pool.Rent(2); + cleanup.Register(a); UnmanagedMemoryHandle b = pool.Rent(); + cleanup.Register(b); - Assert.NotEqual(a[0].DangerousGetHandle(), a[1].DangerousGetHandle()); - Assert.NotEqual(a[0].DangerousGetHandle(), b.DangerousGetHandle()); - Assert.NotEqual(a[1].DangerousGetHandle(), b.DangerousGetHandle()); + Assert.NotEqual(a[0].Handle, a[1].Handle); + Assert.NotEqual(a[0].Handle, b.Handle); + Assert.NotEqual(a[1].Handle, b.Handle); } [Theory] @@ -115,6 +153,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators public void RentReturnRent_SameBuffers(int totalCount, int rentUnit, int capacity) { var pool = new UniformUnmanagedMemoryPool(128, capacity); + using var cleanup = new CleanupUtil(pool); var allHandles = new HashSet(); var handleUnits = new List(); @@ -128,6 +167,9 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { allHandles.Add(array); } + + // Allocate some memory, so potential new pool allocation wouldn't allocated the same memory: + cleanup.Register(Marshal.AllocHGlobal(128)); } foreach (UnmanagedMemoryHandle[] arrayUnit in handleUnits) @@ -151,14 +193,20 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { Assert.Contains(array, allHandles); } + + cleanup.Register(allHandles); } [Fact] - public void Rent_SingleBuffer_OverCapacity_ReturnsNull() + public void Rent_SingleBuffer_OverCapacity_ReturnsInvalidBuffer() { var pool = new UniformUnmanagedMemoryPool(7, 1000); - Assert.NotNull(pool.Rent(1000)); - Assert.Null(pool.Rent()); + using var cleanup = new CleanupUtil(pool); + UnmanagedMemoryHandle[] initial = pool.Rent(1000); + Assert.NotNull(initial); + cleanup.Register(initial); + UnmanagedMemoryHandle b1 = pool.Rent(); + Assert.True(b1.IsInvalid); } [Theory] @@ -168,8 +216,12 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators public void Rent_MultiBuffer_OverCapacity_ReturnsNull(int initialRent, int attempt, int capacity) { var pool = new UniformUnmanagedMemoryPool(128, capacity); - Assert.NotNull(pool.Rent(initialRent)); - Assert.Null(pool.Rent(attempt)); + using var cleanup = new CleanupUtil(pool); + UnmanagedMemoryHandle[] initial = pool.Rent(initialRent); + Assert.NotNull(initial); + cleanup.Register(initial); + UnmanagedMemoryHandle[] b1 = pool.Rent(attempt); + Assert.Null(b1); } [Theory] @@ -180,56 +232,49 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators public void Rent_MultiBuff_BelowCapacity_Succeeds(int initialRent, int attempt, int capacity) { var pool = new UniformUnmanagedMemoryPool(128, capacity); - Assert.NotNull(pool.Rent(initialRent)); - Assert.NotNull(pool.Rent(attempt)); + using var cleanup = new CleanupUtil(pool); + UnmanagedMemoryHandle[] b0 = pool.Rent(initialRent); + Assert.NotNull(b0); + cleanup.Register(b0); + UnmanagedMemoryHandle[] b1 = pool.Rent(attempt); + Assert.NotNull(b1); + cleanup.Register(b1); } [Theory] [InlineData(false)] [InlineData(true)] - public void Release_SubsequentRentReturnsNull(bool multiple) + public void RentReturnRelease_SubsequentRentReturnsDifferentHandles(bool multiple) { var pool = new UniformUnmanagedMemoryPool(16, 16); - pool.Rent(); // Dummy rent + using var cleanup = new CleanupUtil(pool); + UnmanagedMemoryHandle b0 = pool.Rent(); + IntPtr h0 = b0.Handle; + UnmanagedMemoryHandle b1 = pool.Rent(); + IntPtr h1 = b1.Handle; + pool.Return(b0); + pool.Return(b1); pool.Release(); + + // Do some unmanaged allocations to make sure new pool buffers are different: + IntPtr[] dummy = Enumerable.Range(0, 100).Select(_ => Marshal.AllocHGlobal(16)).ToArray(); + cleanup.Register(dummy); + if (multiple) { UnmanagedMemoryHandle b = pool.Rent(); - Assert.Null(b); + cleanup.Register(b); + Assert.NotEqual(h0, b.Handle); + Assert.NotEqual(h1, b.Handle); } else { UnmanagedMemoryHandle[] b = pool.Rent(2); - Assert.Null(b); - } - } - - [Theory] - [InlineData(false)] - [InlineData(true)] - public void Release_SubsequentReturnClosesHandle(bool multiple) - { - var pool = new UniformUnmanagedMemoryPool(16, 16); - if (multiple) - { - UnmanagedMemoryHandle[] b = pool.Rent(2); - pool.Release(); - - Assert.False(b[0].IsClosed); - Assert.False(b[1].IsClosed); - - pool.Return(b); - - Assert.True(b[0].IsClosed); - Assert.True(b[1].IsClosed); - } - else - { - UnmanagedMemoryHandle b = pool.Rent(); - pool.Release(); - Assert.False(b.IsClosed); - pool.Return(b); - Assert.True(b.IsClosed); + cleanup.Register(b); + Assert.NotEqual(h0, b[0].Handle); + Assert.NotEqual(h1, b[0].Handle); + Assert.NotEqual(h0, b[1].Handle); + Assert.NotEqual(h1, b[1].Handle); } } @@ -257,6 +302,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { int count = Environment.ProcessorCount * 200; var pool = new UniformUnmanagedMemoryPool(8, count); + using var cleanup = new CleanupUtil(pool); var rnd = new Random(0); Parallel.For(0, Environment.ProcessorCount, (int i) => @@ -267,8 +313,8 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { UnmanagedMemoryHandle[] data = pool.Rent(2); - GetSpan(pool, data[0]).Fill((byte)i); - GetSpan(pool, data[1]).Fill((byte)i); + data[0].GetSpan().Fill((byte)i); + data[1].GetSpan().Fill((byte)i); allArrays.Add(data[0]); allArrays.Add(data[1]); @@ -283,7 +329,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators foreach (UnmanagedMemoryHandle array in allArrays) { - Assert.True(expected.SequenceEqual(GetSpan(pool, array))); + Assert.True(expected.SequenceEqual(array.GetSpan())); pool.Return(new[] { array }); } }); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index e67831e197..12acbb914d 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -247,6 +247,8 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators Assert.Equal(5, UnmanagedMemoryHandle.TotalOutstandingHandles); b.Dispose(); g.Dispose(); + Assert.Equal(5, UnmanagedMemoryHandle.TotalOutstandingHandles); + allocator.ReleaseRetainedResources(); Assert.Equal(0, UnmanagedMemoryHandle.TotalOutstandingHandles); } } @@ -307,7 +309,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators [ConditionalTheory(nameof(IsWindows))] [InlineData(300)] [InlineData(600)] - [InlineData(1200)] public void MemoryOwnerFinalizer_ReturnsToPool(int length) { // RunTest(length.ToString()); @@ -332,9 +333,11 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators using IMemoryOwner g = allocator.Allocate(lengthInner); Assert.Equal(42, g.GetSpan()[0]); + GC.KeepAlive(allocator); } } + [MethodImpl(MethodImplOptions.NoInlining)] private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllocator allocator, int length, bool check = false) { IMemoryOwner g = allocator.Allocate(length); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedBufferTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedBufferTests.cs new file mode 100644 index 0000000000..68251be861 --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedBufferTests.cs @@ -0,0 +1,93 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Buffers; +using System.Collections.Generic; +using Microsoft.DotNet.RemoteExecutor; +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.Memory.Internals; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory.Allocators +{ + public class UnmanagedBufferTests + { + public class AllocatorBufferTests : BufferTestSuite + { + public AllocatorBufferTests() + : base(new UnmanagedMemoryAllocator(1024 * 64)) + { + } + } + + [Fact] + public void Allocate_CreatesValidBuffer() + { + using var buffer = UnmanagedBuffer.Allocate(10); + Span span = buffer.GetSpan(); + Assert.Equal(10, span.Length); + span[9] = 123; + Assert.Equal(123, span[9]); + } + + [Fact] + public unsafe void Dispose_DoesNotReleaseOutstandingReferences() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + var buffer = UnmanagedBuffer.Allocate(10); + Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); + Span span = buffer.GetSpan(); + + // Pin should AddRef + using (MemoryHandle h = buffer.Pin()) + { + int* ptr = (int*)h.Pointer; + ((IDisposable)buffer).Dispose(); + Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); + ptr[3] = 13; + Assert.Equal(13, span[3]); + } // Unpin should ReleaseRef + + Assert.Equal(0, UnmanagedMemoryHandle.TotalOutstandingHandles); + } + } + + [Theory] + [InlineData(2)] + [InlineData(12)] + public void BufferFinalization_TracksAllocations(int count) + { + RemoteExecutor.Invoke(RunTest, count.ToString()).Dispose(); + + static void RunTest(string countStr) + { + int countInner = int.Parse(countStr); + List> l = FillList(countInner); + + l.RemoveRange(0, l.Count / 2); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.Equal(countInner / 2, l.Count); // This is here to prevent eager finalization of the list's elements + Assert.Equal(countInner / 2, UnmanagedMemoryHandle.TotalOutstandingHandles); + } + + static List> FillList(int countInner) + { + var l = new List>(); + for (int i = 0; i < countInner; i++) + { + var h = UnmanagedBuffer.Allocate(42); + l.Add(h); + } + + return l; + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedMemoryHandleTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedMemoryHandleTests.cs index ecc2188eb1..a3f827355f 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedMemoryHandleTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UnmanagedMemoryHandleTests.cs @@ -14,10 +14,10 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators [Fact] public unsafe void Allocate_AllocatesReadWriteMemory() { - using var h = UnmanagedMemoryHandle.Allocate(128); - Assert.False(h.IsClosed); + var h = UnmanagedMemoryHandle.Allocate(128); Assert.False(h.IsInvalid); - byte* ptr = (byte*)h.DangerousGetHandle(); + Assert.True(h.IsValid); + byte* ptr = (byte*)h.Handle; for (int i = 0; i < 128; i++) { ptr[i] = (byte)i; @@ -27,21 +27,23 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators { Assert.Equal((byte)i, ptr[i]); } + + h.Free(); } [Fact] - public void Dispose_ClosesHandle() + public void Free_ClosesHandle() { var h = UnmanagedMemoryHandle.Allocate(128); - h.Dispose(); - Assert.True(h.IsClosed); + h.Free(); Assert.True(h.IsInvalid); + Assert.Equal(IntPtr.Zero, h.Handle); } [Theory] [InlineData(1)] [InlineData(13)] - public void CreateDispose_TracksAllocations(int count) + public void Create_Free_AllocationsAreTracked(int count) { RemoteExecutor.Invoke(RunTest, count.ToString()).Dispose(); @@ -60,125 +62,39 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators for (int i = 0; i < countInner; i++) { Assert.Equal(countInner - i, UnmanagedMemoryHandle.TotalOutstandingHandles); - l[i].Dispose(); + l[i].Free(); Assert.Equal(countInner - i - 1, UnmanagedMemoryHandle.TotalOutstandingHandles); } } } - [Theory] - [InlineData(2)] - [InlineData(12)] - public void CreateFinalize_TracksAllocations(int count) - { - RemoteExecutor.Invoke(RunTest, count.ToString()).Dispose(); - - static void RunTest(string countStr) - { - int countInner = int.Parse(countStr); - List l = FillList(countInner); - - l.RemoveRange(0, l.Count / 2); - - GC.Collect(); - GC.WaitForPendingFinalizers(); - - Assert.Equal(countInner / 2, l.Count); // This is here to prevent eager finalization of the list's elements - Assert.Equal(countInner / 2, UnmanagedMemoryHandle.TotalOutstandingHandles); - } - - static List FillList(int countInner) - { - var l = new List(); - for (int i = 0; i < countInner; i++) - { - var h = UnmanagedMemoryHandle.Allocate(42); - l.Add(h); - } - - return l; - } - } - [Fact] - public void Resurrect_PreventsFinalization() + public void Equality_WhenTrue() { - RemoteExecutor.Invoke(RunTest).Dispose(); - - static void RunTest() - { - AllocateResurrect(); - Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); - GC.Collect(); - GC.WaitForPendingFinalizers(); - Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); - GC.Collect(); - GC.WaitForPendingFinalizers(); - Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); - } - - static void AllocateResurrect() - { - var h = UnmanagedMemoryHandle.Allocate(42); - h.Resurrect(); - } - } - - private static UnmanagedMemoryHandle resurrectedHandle; - - private class HandleOwner - { - private UnmanagedMemoryHandle handle; - - public HandleOwner(UnmanagedMemoryHandle handle) => this.handle = handle; - - ~HandleOwner() - { - resurrectedHandle = this.handle; - this.handle.Resurrect(); - } + var h1 = UnmanagedMemoryHandle.Allocate(10); + UnmanagedMemoryHandle h2 = h1; + + Assert.True(h1.Equals(h2)); + Assert.True(h2.Equals(h1)); + Assert.True(h1 == h2); + Assert.False(h1 != h2); + Assert.True(h1.GetHashCode() == h2.GetHashCode()); + h1.Free(); } [Fact] - public void AssignedToNewOwner_ReRegistersForFinalization() + public void Equality_WhenFalse() { - RemoteExecutor.Invoke(RunTest).Dispose(); - - static void RunTest() - { - AllocateAndForget(); - Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); - GC.Collect(); - GC.WaitForPendingFinalizers(); - VerifyResurrectedHandle(true); - GC.Collect(); - GC.WaitForPendingFinalizers(); - VerifyResurrectedHandle(false); - GC.Collect(); - GC.WaitForPendingFinalizers(); - - Assert.Equal(0, UnmanagedMemoryHandle.TotalOutstandingHandles); - } - - static void AllocateAndForget() - { - _ = new HandleOwner(UnmanagedMemoryHandle.Allocate(42)); - } + var h1 = UnmanagedMemoryHandle.Allocate(10); + var h2 = UnmanagedMemoryHandle.Allocate(10); - static void VerifyResurrectedHandle(bool reAssign) - { - Assert.NotNull(resurrectedHandle); - Assert.Equal(1, UnmanagedMemoryHandle.TotalOutstandingHandles); - Assert.False(resurrectedHandle.IsClosed); - Assert.False(resurrectedHandle.IsInvalid); - resurrectedHandle.AssignedToNewOwner(); - if (reAssign) - { - _ = new HandleOwner(resurrectedHandle); - } + Assert.False(h1.Equals(h2)); + Assert.False(h2.Equals(h1)); + Assert.False(h1 == h2); + Assert.True(h1 != h2); - resurrectedHandle = null; - } + h1.Free(); + h2.Free(); } } } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs index e6d07a191c..257874f1fb 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs @@ -19,8 +19,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers public class Allocate : MemoryGroupTestsBase { #pragma warning disable SA1509 - public static TheoryData AllocateData = - new TheoryData() + public static TheoryData AllocateData = new() { { default(S5), 22, 4, 4, 1, 4, 4 }, { default(S5), 22, 4, 7, 2, 4, 3 }, @@ -100,9 +99,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers g.Dispose(); } - private static unsafe Span GetSpan(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle h) => - new Span((void*)h.DangerousGetHandle(), pool.BufferLength); - [Theory] [InlineData(AllocationOptions.None)] [InlineData(AllocationOptions.Clean)] @@ -112,7 +108,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers UnmanagedMemoryHandle[] buffers = pool.Rent(5); foreach (UnmanagedMemoryHandle b in buffers) { - GetSpan(pool, b).Fill(42); + b.GetSpan().Fill(42); } pool.Return(buffers); diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs index 3b8d0073ed..2d1c6e2241 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/BasicTestPatternProvider.cs @@ -61,8 +61,6 @@ namespace SixLabors.ImageSharp.Tests } }); - - return result; }