diff --git a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs index 419f7531f0..6bd81d7e5f 100644 --- a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs +++ b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs @@ -6,18 +6,58 @@ using System.Threading; namespace SixLabors.ImageSharp.Diagnostics { + /// + /// Represents the method to handle . + /// + public delegate void UndisposedMemoryResourceDelegate(string allocationStackTrace); + + /// + /// Utilities to track memory usage and detect memory leaks from not disposing ImageSharp objects. + /// public static class MemoryDiagnostics { private static int totalUndisposedAllocationCount; - public static MemoryInfo GetMemoryInfo() => new MemoryInfo(totalUndisposedAllocationCount); + private static UndisposedMemoryResourceDelegate undisposedMemoryResource; + private static int undisposedMemoryResourceSubscriptionCounter; + + /// + /// 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 . + /// + 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; } + /// + /// Gets a value indicating the total number of memory resource objects leaked to the finalizer. + /// + 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)); + } } } diff --git a/src/ImageSharp/Diagnostics/MemoryInfo.cs b/src/ImageSharp/Diagnostics/MemoryInfo.cs deleted file mode 100644 index 54ae83bd0a..0000000000 --- a/src/ImageSharp/Diagnostics/MemoryInfo.cs +++ /dev/null @@ -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; } - } -} diff --git a/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs index 534cec0f8f..333ef85c25 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs @@ -10,15 +10,24 @@ namespace SixLabors.ImageSharp.Memory.Internals { /// /// Implements reference counting lifetime guard mechanism for memory resources - /// also maintaining the current value of . + /// and maintains the value of . /// 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(); } diff --git a/tests/ImageSharp.Tests/Memory/Allocators/MemoryDiagnosticsTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/MemoryDiagnosticsTests.cs index 55f14fdfa7..5fab655cb3 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/MemoryDiagnosticsTests.cs +++ b/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 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(length, 1024) : Allocator.Allocate(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(true)] - public void BufferAndGroupFinalizer_DoesNotReduce_TotalUndisposedLogicalAllocationCount(bool isGroupOuter) + [InlineData(false, false)] + [InlineData(false, true)] + [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)]