finalize and document the API

src/ImageSharp/Diagnostics/MemoryDiagnostics.cs

@@ -6,18 +6,58 @@ using System.Threading;
 
 namespace SixLabors.ImageSharp.Diagnostics
 {
+    /// <summary>
+    /// Represents the method to handle <see cref="MemoryDiagnostics.UndisposedAllocation"/>.
+    /// </summary>
+    public delegate void UndisposedMemoryResourceDelegate(string allocationStackTrace);
+
+    /// <summary>
+    /// Utilities to track memory usage and detect memory leaks from not disposing ImageSharp objects.
+    /// </summary>
     public static class MemoryDiagnostics
     {
         private static int totalUndisposedAllocationCount;
 
-        public static MemoryInfo GetMemoryInfo() => new MemoryInfo(totalUndisposedAllocationCount);
+        private static UndisposedMemoryResourceDelegate undisposedMemoryResource;
+        private static int undisposedMemoryResourceSubscriptionCounter;
+
+        /// <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 static event UndisposedMemoryResourceDelegate UndisposedAllocation
+        {
+            add
+            {
+                Interlocked.Increment(ref undisposedMemoryResourceSubscriptionCounter);
+                undisposedMemoryResource += value;
+            }
+
+            remove
+            {
+                undisposedMemoryResource -= value;
+                Interlocked.Decrement(ref undisposedMemoryResourceSubscriptionCounter);
+            }
+        }
 
-        public static bool EnableStrictDisposeWatcher { get; set; }
+        /// <summary>
+        /// Gets a value indicating the total number of memory resource objects leaked to the finalizer.
+        /// </summary>
+        public static int TotalUndisposedAllocationCount => totalUndisposedAllocationCount;
+
+        internal static bool MemoryResourceLeakedSubscribed => undisposedMemoryResourceSubscriptionCounter > 0;
 
         internal static void IncrementTotalUndisposedAllocationCount() =>
             Interlocked.Increment(ref totalUndisposedAllocationCount);
 
         internal static void DecrementTotalUndisposedAllocationCount() =>
             Interlocked.Decrement(ref totalUndisposedAllocationCount);
+
+        internal static void RaiseUndisposedMemoryResource(string allocationStackTrace)
+        {
+            // Schedule on the ThreadPool, to avoid user callback messing up the finalizer thread.
+            ThreadPool.QueueUserWorkItem(_ => undisposedMemoryResource?.Invoke(allocationStackTrace));
+        }
     }
 }

src/ImageSharp/Diagnostics/MemoryInfo.cs

@@ -1,13 +0,0 @@
-// Copyright (c) Six Labors.
-// Licensed under the Apache License, Version 2.0.
-
-namespace SixLabors.ImageSharp.Diagnostics
-{
-    public readonly struct MemoryInfo
-    {
-        internal MemoryInfo(long totalUndisposedAllocationCount)
-            => this.TotalUndisposedAllocationCount = totalUndisposedAllocationCount;
-
-        public long TotalUndisposedAllocationCount { get; }
-    }
-}

src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs

@@ -10,15 +10,24 @@ namespace SixLabors.ImageSharp.Memory.Internals
 {
     /// <summary>
     /// Implements reference counting lifetime guard mechanism for memory resources
-    /// also maintaining the current value of <see cref="MemoryInfo.TotalUndisposedAllocationCount"/>.
+    /// and maintains the value of <see cref="MemoryDiagnostics.TotalUndisposedAllocationCount"/>.
     /// </summary>
     internal abstract class RefCountedMemoryLifetimeGuard : IDisposable
     {
         private int refCount = 1;
         private int disposed;
         private int released;
+        private string allocationStackTrace;
 
-        public RefCountedMemoryLifetimeGuard() => MemoryDiagnostics.IncrementTotalUndisposedAllocationCount();
+        protected RefCountedMemoryLifetimeGuard()
+        {
+            if (MemoryDiagnostics.MemoryResourceLeakedSubscribed)
+            {
+                this.allocationStackTrace = Environment.StackTrace;
+            }
+
+            MemoryDiagnostics.IncrementTotalUndisposedAllocationCount();
+        }
 
         ~RefCountedMemoryLifetimeGuard()
         {
@@ -57,6 +66,10 @@ namespace SixLabors.ImageSharp.Memory.Internals
                     {
                         MemoryDiagnostics.DecrementTotalUndisposedAllocationCount();
                     }
+                    else if (this.allocationStackTrace != null)
+                    {
+                        MemoryDiagnostics.RaiseUndisposedMemoryResource(this.allocationStackTrace);
+                    }
 
                     this.Release();
                 }

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

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
+using System.Threading;
 using Microsoft.DotNet.RemoteExecutor;
 using SixLabors.ImageSharp.Diagnostics;
 using SixLabors.ImageSharp.Memory;
@@ -20,60 +21,88 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators
         [Theory]
         [InlineData(false)]
         [InlineData(true)]
-        public void AllocateDispose_Maintains_TotalUndisposedLogicalAllocationCount(bool isGroupOuter)
+        public void PerfectCleanup_NoLeaksReported(bool isGroupOuter)
         {
             RemoteExecutor.Invoke(RunTest, isGroupOuter.ToString()).Dispose();
 
             static void RunTest(string isGroupInner)
             {
                 bool isGroup = bool.Parse(isGroupInner);
+                int leakCounter = 0;
+                MemoryDiagnostics.UndisposedAllocation += _ => Interlocked.Increment(ref leakCounter);
+
                 List<IDisposable> buffers = new();
 
-                Assert.Equal(0, MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount);
+                Assert.Equal(0, MemoryDiagnostics.TotalUndisposedAllocationCount);
                 for (int length = 1024; length <= 64 * OneMb; length *= 2)
                 {
-                    long cntBefore = MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount;
+                    long cntBefore = MemoryDiagnostics.TotalUndisposedAllocationCount;
                     IDisposable buffer = isGroup ?
                         Allocator.AllocateGroup<byte>(length, 1024) :
                         Allocator.Allocate<byte>(length);
                     buffers.Add(buffer);
-                    long cntAfter = MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount;
+                    long cntAfter = MemoryDiagnostics.TotalUndisposedAllocationCount;
                     Assert.True(cntAfter > cntBefore);
                 }
 
                 foreach (IDisposable buffer in buffers)
                 {
-                    long cntBefore = MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount;
+                    long cntBefore = MemoryDiagnostics.TotalUndisposedAllocationCount;
                     buffer.Dispose();
-                    long cntAfter = MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount;
+                    long cntAfter = MemoryDiagnostics.TotalUndisposedAllocationCount;
                     Assert.True(cntAfter < cntBefore);
                 }
 
-                Assert.Equal(0, MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount);
+                Assert.Equal(0, MemoryDiagnostics.TotalUndisposedAllocationCount);
+                Assert.Equal(0, leakCounter);
             }
         }
 
         [Theory]
-        [InlineData(false)]
+        [InlineData(false, false)]
-        [InlineData(true)]
+        [InlineData(false, true)]
-        public void BufferAndGroupFinalizer_DoesNotReduce_TotalUndisposedLogicalAllocationCount(bool isGroupOuter)
+        [InlineData(true, false)]
+        [InlineData(true, true)]
+        public void MissingCleanup_LeaksAreReported(bool isGroupOuter, bool subscribeLeakHandleOuter)
         {
-            RemoteExecutor.Invoke(RunTest, isGroupOuter.ToString()).Dispose();
+            RemoteExecutor.Invoke(RunTest, isGroupOuter.ToString(), subscribeLeakHandleOuter.ToString()).Dispose();
 
-            static void RunTest(string isGroupInner)
+            static void RunTest(string isGroupInner, string subscribeLeakHandleInner)
             {
                 bool isGroup = bool.Parse(isGroupInner);
-                Assert.Equal(0, MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount);
+                bool subscribeLeakHandle = bool.Parse(subscribeLeakHandleInner);
+                int leakCounter = 0;
+                bool stackTraceOk = true;
+                if (subscribeLeakHandle)
+                {
+                    MemoryDiagnostics.UndisposedAllocation += stackTrace =>
+                    {
+                        Interlocked.Increment(ref leakCounter);
+                        stackTraceOk &= stackTrace.Contains(nameof(RunTest)) && stackTrace.Contains(nameof(AllocateAndForget));
+                        Assert.Contains(nameof(AllocateAndForget), stackTrace);
+                    };
+                }
+
+                Assert.Equal(0, MemoryDiagnostics.TotalUndisposedAllocationCount);
                 for (int length = 1024; length <= 64 * OneMb; length *= 2)
                 {
-                    long cntBefore = MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount;
+                    long cntBefore = MemoryDiagnostics.TotalUndisposedAllocationCount;
                     AllocateAndForget(length, isGroup);
                     GC.Collect();
                     GC.WaitForPendingFinalizers();
                     GC.Collect();
-                    long cntAfter = MemoryDiagnostics.GetMemoryInfo().TotalUndisposedAllocationCount;
+                    long cntAfter = MemoryDiagnostics.TotalUndisposedAllocationCount;
                     Assert.True(cntAfter > cntBefore);
                 }
+
+                if (subscribeLeakHandle)
+                {
+                    // Make sure at least some of the leak callbacks have time to complete on the ThreadPool
+                    Thread.Sleep(200);
+                    Assert.True(leakCounter > 3, $"leakCounter did not count enough leaks ({leakCounter} only)");
+                }
+
+                Assert.True(stackTraceOk);
             }
 
             [MethodImpl(MethodImplOptions.NoInlining)]