xunit helper to track undisposed memory

Author: tocsoft

src/ImageSharp/Diagnostics/MemoryDiagnostics.cs

@@ -15,10 +15,8 @@ namespace SixLabors.ImageSharp.Diagnostics
     /// </summary>
     public static class MemoryDiagnostics
     {
-        private static int totalUndisposedAllocationCount;
-
-        private static UndisposedAllocationDelegate undisposedAllocation;
-        private static int undisposedAllocationSubscriptionCounter;
+        internal static readonly InteralMemoryDiagnostics Default = new();
+        private static AsyncLocal<InteralMemoryDiagnostics> localInstance = null;
         private static readonly object SyncRoot = new();
 
         /// <summary>
@@ -28,56 +26,116 @@ public static class MemoryDiagnostics
         /// </summary>
         public static event UndisposedAllocationDelegate UndisposedAllocation
         {
-            add
+            add => Current.UndisposedAllocation += value;
+            remove => Current.UndisposedAllocation -= value;
+        }
+
+        internal static InteralMemoryDiagnostics Current
+        {
+            get
             {
-                lock (SyncRoot)
+                if (localInstance != null && localInstance.Value != null)
                 {
-                    undisposedAllocationSubscriptionCounter++;
-                    undisposedAllocation += value;
+                    return localInstance.Value;
                 }
+
+                return Default;
             }
 
-            remove
+            set
             {
-                lock (SyncRoot)
+                if (localInstance == null)
                 {
-                    undisposedAllocation -= value;
-                    undisposedAllocationSubscriptionCounter--;
+                    lock (SyncRoot)
+                    {
+                        localInstance ??= new AsyncLocal<InteralMemoryDiagnostics>();
+                    }
                 }
+
+                localInstance.Value = value;
             }
         }
 
         /// <summary>
         /// Gets a value indicating the total number of memory resource objects leaked to the finalizer.
         /// </summary>
-        public static int TotalUndisposedAllocationCount => totalUndisposedAllocationCount;
+        public static int TotalUndisposedAllocationCount => Current.TotalUndisposedAllocationCount;
 
-        internal static bool UndisposedAllocationSubscribed => Volatile.Read(ref undisposedAllocationSubscriptionCounter) > 0;
+        internal static bool UndisposedAllocationSubscribed => Current.UndisposedAllocationSubscribed;
 
-        internal static void IncrementTotalUndisposedAllocationCount() =>
-            Interlocked.Increment(ref totalUndisposedAllocationCount);
+        internal static void IncrementTotalUndisposedAllocationCount() => Current.IncrementTotalUndisposedAllocationCount();
 
-        internal static void DecrementTotalUndisposedAllocationCount() =>
-            Interlocked.Decrement(ref totalUndisposedAllocationCount);
+        internal static void DecrementTotalUndisposedAllocationCount() => Current.DecrementTotalUndisposedAllocationCount();
 
         internal static void RaiseUndisposedMemoryResource(string allocationStackTrace)
+            => Current.RaiseUndisposedMemoryResource(allocationStackTrace);
+
+        internal class InteralMemoryDiagnostics
         {
-            if (undisposedAllocation is null)
+            private int totalUndisposedAllocationCount;
+
+            private UndisposedAllocationDelegate undisposedAllocation;
+            private int undisposedAllocationSubscriptionCounter;
+            private readonly object syncRoot = new();
+
+            /// <summary>
+            /// Fires when an ImageSharp object's undisposed memory resource leaks to the finalizer.
+            /// The event brings significant overhead, and is intended to be used for troubleshooting only.
+            /// For production diagnostics, use <see cref="TotalUndisposedAllocationCount"/>.
+            /// </summary>
+            public event UndisposedAllocationDelegate UndisposedAllocation
             {
-                return;
+                add
+                {
+                    lock (this.syncRoot)
+                    {
+                        this.undisposedAllocationSubscriptionCounter++;
+                        this.undisposedAllocation += value;
+                    }
+                }
+
+                remove
+                {
+                    lock (this.syncRoot)
+                    {
+                        this.undisposedAllocation -= value;
+                        this.undisposedAllocationSubscriptionCounter--;
+                    }
+                }
             }
 
-            // Schedule on the ThreadPool, to avoid user callback messing up the finalizer thread.
+            /// <summary>
+            /// Gets a value indicating the total number of memory resource objects leaked to the finalizer.
+            /// </summary>
+            public int TotalUndisposedAllocationCount => this.totalUndisposedAllocationCount;
+
+            internal bool UndisposedAllocationSubscribed => Volatile.Read(ref this.undisposedAllocationSubscriptionCounter) > 0;
+
+            internal void IncrementTotalUndisposedAllocationCount() =>
+                Interlocked.Increment(ref this.totalUndisposedAllocationCount);
+
+            internal void DecrementTotalUndisposedAllocationCount() =>
+                Interlocked.Decrement(ref this.totalUndisposedAllocationCount);
+
+            internal void RaiseUndisposedMemoryResource(string allocationStackTrace)
+            {
+                if (this.undisposedAllocation is null)
+                {
+                    return;
+                }
+
+                // Schedule on the ThreadPool, to avoid user callback messing up the finalizer thread.
 #if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER
-            ThreadPool.QueueUserWorkItem(
-                stackTrace => undisposedAllocation?.Invoke(stackTrace),
-                allocationStackTrace,
-                preferLocal: false);
+                ThreadPool.QueueUserWorkItem(
+                    stackTrace => this.undisposedAllocation?.Invoke(stackTrace),
+                    allocationStackTrace,
+                    preferLocal: false);
 #else
             ThreadPool.QueueUserWorkItem(
-                stackTrace => undisposedAllocation?.Invoke((string)stackTrace),
+                stackTrace => this.undisposedAllocation?.Invoke((string)stackTrace),
                 allocationStackTrace);
 #endif
+            }
         }
     }
 }

tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs

@@ -103,10 +103,14 @@ public async Task DecodeAsync_NonGeneric_CreatesCorrectImageType(string path, Ty
 
         [Theory]
         [WithFileCollection(nameof(CommonTestImages), PixelTypes.Rgba32)]
+        [ValidateDisposedMemoryAllocations]
         public void Decode<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel>
         {
             using Image<TPixel> image = provider.GetImage(PngDecoder);
+            //var testFile = TestFile.Create(provider.SourceFileOrDescription);
+            //using Image<TPixel> image = Image.Load<TPixel>(provider.Configuration, testFile.Bytes, PngDecoder);
+
             image.DebugSave(provider);
             image.CompareToOriginal(provider, ImageComparer.Exact);
         }

tests/ImageSharp.Tests/Image/ImageTests.LoadPixelData.cs

@@ -14,6 +14,7 @@ public class LoadPixelData
             [Theory]
             [InlineData(false)]
             [InlineData(true)]
+            [ValidateDisposedMemoryAllocations]
             public void FromPixels(bool useSpan)
             {
                 Rgba32[] data = { Color.Black, Color.White, Color.White, Color.Black, };

tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs

@@ -55,6 +55,8 @@ public void PreferContiguousImageBuffers_LoadImage_BufferIsContiguous(string for
 
             static void RunTest(string formatInner)
             {
+                using IDisposable mem = MemoryAllocatorValidator.MonitorAllocations();
+
                 Configuration configuration = Configuration.Default.Clone();
                 configuration.PreferContiguousImageBuffers = true;
                 IImageEncoder encoder = configuration.ImageFormatsManager.FindEncoder(

tests/ImageSharp.Tests/MemoryAllocatorValidator.cs

@@ -0,0 +1,43 @@
+// Copyright (c) Six Labors.
+// Licensed under the Apache License, Version 2.0.
+
+using System;
+using System.Diagnostics;
+using SixLabors.ImageSharp.Diagnostics;
+using Xunit;
+
+namespace SixLabors.ImageSharp.Tests
+{
+    public static class MemoryAllocatorValidator
+    {
+        public static IDisposable MonitorAllocations(int max = 0)
+        {
+            MemoryDiagnostics.Current = new();
+            return new TestMemoryAllocatorDisposable(max);
+        }
+
+        public static void ValidateAllocation(int max = 0)
+        {
+            var count = MemoryDiagnostics.TotalUndisposedAllocationCount;
+            var pass = count <= max;
+            Assert.True(pass, $"Expected a max of {max} undisposed buffers but found {count}");
+
+            if (count > 0)
+            {
+                Debug.WriteLine("We should have Zero undisposed memory allocations.");
+            }
+
+            MemoryDiagnostics.Current = null;
+        }
+
+        public struct TestMemoryAllocatorDisposable : IDisposable
+        {
+            private readonly int max;
+
+            public TestMemoryAllocatorDisposable(int max) => this.max = max;
+
+            public void Dispose()
+                => ValidateAllocation(this.max);
+        }
+    }
+}

tests/ImageSharp.Tests/TestUtilities/ImageProviders/FileProvider.cs

@@ -7,6 +7,7 @@
 using System.IO;
 using System.Reflection;
 using System.Threading.Tasks;
+using SixLabors.ImageSharp.Diagnostics;
 using SixLabors.ImageSharp.Formats;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;
@@ -158,8 +159,13 @@ public override Image<TPixel> GetImage(IImageDecoder decoder)
                     return this.LoadImage(decoder);
                 }
 
-                var key = new Key(this.PixelType, this.FilePath, decoder);
+                // do cache so we can track allocation correctly when validating memory
+                if (MemoryDiagnostics.Current != MemoryDiagnostics.Default)
+                {
+                    return this.LoadImage(decoder);
+                }
 
+                var key = new Key(this.PixelType, this.FilePath, decoder);
                 Image<TPixel> cachedImage = Cache.GetOrAdd(key, _ => this.LoadImage(decoder));
 
                 return cachedImage.Clone(this.Configuration);

tests/ImageSharp.Tests/ValidateDisposedMemoryAllocationsAttribute.cs

@@ -0,0 +1,36 @@
+// Copyright (c) Six Labors.
+// Licensed under the Apache License, Version 2.0.
+
+using System;
+using System.Diagnostics;
+using System.Reflection;
+using Xunit.Sdk;
+
+namespace SixLabors.ImageSharp.Tests
+{
+    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
+    public class ValidateDisposedMemoryAllocationsAttribute : BeforeAfterTestAttribute
+    {
+        private readonly int max = 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 override void Before(MethodInfo methodUnderTest)
+            => MemoryAllocatorValidator.MonitorAllocations(this.max); // the disposable isn't important cause the validate below does the same thing
+
+        public override void After(MethodInfo methodUnderTest)
+            => MemoryAllocatorValidator.ValidateAllocation(this.max);
+    }
+}