diff --git a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs index 057242bae..237f85b0f 100644 --- a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs +++ b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.Threading; namespace SixLabors.ImageSharp.Diagnostics @@ -15,10 +16,8 @@ namespace SixLabors.ImageSharp.Diagnostics /// public static class MemoryDiagnostics { - internal static readonly InteralMemoryDiagnostics Default = new(); - private static AsyncLocal localInstance = null; + private static int totalUndisposedAllocationCount; - // the async local end up out of scope during finalizers so putting into thte internal class is useless private static UndisposedAllocationDelegate undisposedAllocation; private static int undisposedAllocationSubscriptionCounter; private static readonly object SyncRoot = new(); @@ -49,42 +48,34 @@ namespace SixLabors.ImageSharp.Diagnostics } } - internal static InteralMemoryDiagnostics Current - { - get - { - if (localInstance != null && localInstance.Value != null) - { - return localInstance.Value; - } - - return Default; - } - - set - { - if (localInstance == null) - { - lock (SyncRoot) - { - localInstance ??= new AsyncLocal(); - } - } + /// + /// Fires when ImageSharp allocates memory from a MemoryAllocator + /// + internal static event Action MemoryAllocated; - localInstance.Value = value; - } - } + /// + /// Fires when ImageSharp releases allocated from a MemoryAllocator + /// + internal static event Action MemoryReleased; /// /// Gets a value indicating the total number of memory resource objects leaked to the finalizer. /// - public static int TotalUndisposedAllocationCount => Current.TotalUndisposedAllocationCount; + public static int TotalUndisposedAllocationCount => totalUndisposedAllocationCount; internal static bool UndisposedAllocationSubscribed => Volatile.Read(ref undisposedAllocationSubscriptionCounter) > 0; - internal static void IncrementTotalUndisposedAllocationCount() => Current.IncrementTotalUndisposedAllocationCount(); + internal static void IncrementTotalUndisposedAllocationCount() + { + Interlocked.Increment(ref totalUndisposedAllocationCount); + MemoryAllocated?.Invoke(); + } - internal static void DecrementTotalUndisposedAllocationCount() => Current.DecrementTotalUndisposedAllocationCount(); + internal static void DecrementTotalUndisposedAllocationCount() + { + Interlocked.Decrement(ref totalUndisposedAllocationCount); + MemoryReleased?.Invoke(); + } internal static void RaiseUndisposedMemoryResource(string allocationStackTrace) { @@ -105,21 +96,5 @@ namespace SixLabors.ImageSharp.Diagnostics allocationStackTrace); #endif } - - internal class InteralMemoryDiagnostics - { - private int totalUndisposedAllocationCount; - - /// - /// Gets a value indicating the total number of memory resource objects leaked to the finalizer. - /// - public int TotalUndisposedAllocationCount => this.totalUndisposedAllocationCount; - - internal void IncrementTotalUndisposedAllocationCount() => - Interlocked.Increment(ref this.totalUndisposedAllocationCount); - - internal void DecrementTotalUndisposedAllocationCount() => - Interlocked.Decrement(ref this.totalUndisposedAllocationCount); - } } } diff --git a/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs b/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs index 2c3d98eb1..13664ee9b 100644 --- a/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs +++ b/tests/ImageSharp.Tests/MemoryAllocatorValidator.cs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Diagnostics; +using System.Threading; using SixLabors.ImageSharp.Diagnostics; using Xunit; @@ -10,34 +10,68 @@ namespace SixLabors.ImageSharp.Tests { public static class MemoryAllocatorValidator { - public static IDisposable MonitorAllocations(int max = 0) + private static readonly AsyncLocal LocalInstance = new(); + + public static bool MonitoringAllocations => LocalInstance.Value != null; + + static MemoryAllocatorValidator() { - MemoryDiagnostics.Current = new(); - return new TestMemoryAllocatorDisposable(max); + MemoryDiagnostics.MemoryAllocated += MemoryDiagnostics_MemoryAllocated; + MemoryDiagnostics.MemoryReleased += MemoryDiagnostics_MemoryReleased; } - public static void ValidateAllocation(int max = 0) + private static void MemoryDiagnostics_MemoryReleased() { - var count = MemoryDiagnostics.TotalUndisposedAllocationCount; - var pass = count <= max; - Assert.True(pass, $"Expected a max of {max} undisposed buffers but found {count}"); + TestMemoryDiagnostics backing = LocalInstance.Value; + if (backing != null) + { + backing.TotalRemainingAllocated--; + } + } - if (count > 0) + private static void MemoryDiagnostics_MemoryAllocated() + { + TestMemoryDiagnostics backing = LocalInstance.Value; + if (backing != null) { - Debug.WriteLine("We should have Zero undisposed memory allocations."); + backing.TotalAllocated++; + backing.TotalRemainingAllocated++; } + } - MemoryDiagnostics.Current = null; + public static TestMemoryDiagnostics MonitorAllocations() + { + var diag = new TestMemoryDiagnostics(); + LocalInstance.Value = diag; + return diag; } - public struct TestMemoryAllocatorDisposable : IDisposable + public static void StopMonitoringAllocations() => LocalInstance.Value = null; + + public static void ValidateAllocations(int expectedAllocationCount = 0) + => LocalInstance.Value?.Validate(expectedAllocationCount); + + public class TestMemoryDiagnostics : IDisposable { - private readonly int max; + public int TotalAllocated { get; set; } - public TestMemoryAllocatorDisposable(int max) => this.max = max; + public int TotalRemainingAllocated { get; set; } + + public void Validate(int expectedAllocationCount) + { + var count = this.TotalRemainingAllocated; + var pass = expectedAllocationCount == count; + Assert.True(pass, $"Expected a {expectedAllocationCount} undisposed buffers but found {count}"); + } public void Dispose() - => ValidateAllocation(this.max); + { + this.Validate(0); + if (LocalInstance.Value == this) + { + StopMonitoringAllocations(); + } + } } } } diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs index 3675356c3..63c5ce31a 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs @@ -159,8 +159,8 @@ namespace SixLabors.ImageSharp.Tests return this.LoadImage(decoder); } - // do cache so we can track allocation correctly when validating memory - if (MemoryDiagnostics.Current != MemoryDiagnostics.Default) + // do not cache so we can track allocation correctly when validating memory + if (MemoryAllocatorValidator.MonitoringAllocations) { return this.LoadImage(decoder); } diff --git a/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs b/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs index 95d13b595..65ed990dd 100644 --- a/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs +++ b/tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs @@ -11,26 +11,23 @@ namespace SixLabors.ImageSharp.Tests [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] public class ValidateDisposedMemoryAllocationsAttribute : BeforeAfterTestAttribute { - private readonly int max = 0; + private readonly int expected = 0; public ValidateDisposedMemoryAllocationsAttribute() : this(0) { } - public ValidateDisposedMemoryAllocationsAttribute(int max) - { - this.max = max; - if (max > 0) - { - Debug.WriteLine("Needs fixing, we shoudl have Zero undisposed memory allocations."); - } - } + public ValidateDisposedMemoryAllocationsAttribute(int expected) + => this.expected = expected; public override void Before(MethodInfo methodUnderTest) - => MemoryAllocatorValidator.MonitorAllocations(this.max); // the disposable isn't important cause the validate below does the same thing + => MemoryAllocatorValidator.MonitorAllocations(); public override void After(MethodInfo methodUnderTest) - => MemoryAllocatorValidator.ValidateAllocation(this.max); + { + MemoryAllocatorValidator.ValidateAllocations(this.expected); + MemoryAllocatorValidator.StopMonitoringAllocations(); + } } }