diff --git a/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs new file mode 100644 index 000000000..d83e737f1 --- /dev/null +++ b/src/ImageSharp/Diagnostics/MemoryDiagnostics.cs @@ -0,0 +1,83 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System.Threading; + +namespace SixLabors.ImageSharp.Diagnostics +{ + /// + /// Represents the method to handle . + /// + public delegate void UndisposedAllocationDelegate(string allocationStackTrace); + + /// + /// Utilities to track memory usage and detect memory leaks from not disposing ImageSharp objects. + /// + public static class MemoryDiagnostics + { + private static int totalUndisposedAllocationCount; + + private static UndisposedAllocationDelegate undisposedAllocation; + private static int undisposedAllocationSubscriptionCounter; + private static readonly object SyncRoot = new(); + + /// + /// 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 UndisposedAllocationDelegate UndisposedAllocation + { + add + { + lock (SyncRoot) + { + undisposedAllocationSubscriptionCounter++; + undisposedAllocation += value; + } + } + + remove + { + lock (SyncRoot) + { + undisposedAllocation -= value; + undisposedAllocationSubscriptionCounter--; + } + } + } + + /// + /// Gets a value indicating the total number of memory resource objects leaked to the finalizer. + /// + public static int TotalUndisposedAllocationCount => totalUndisposedAllocationCount; + + internal static bool UndisposedAllocationSubscribed => Volatile.Read(ref undisposedAllocationSubscriptionCounter) > 0; + + internal static void IncrementTotalUndisposedAllocationCount() => + Interlocked.Increment(ref totalUndisposedAllocationCount); + + internal static void DecrementTotalUndisposedAllocationCount() => + Interlocked.Decrement(ref totalUndisposedAllocationCount); + + internal static void RaiseUndisposedMemoryResource(string allocationStackTrace) + { + if (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); +#else + ThreadPool.QueueUserWorkItem( + stackTrace => undisposedAllocation?.Invoke((string)stackTrace), + allocationStackTrace); +#endif + } + } +} diff --git a/src/ImageSharp/Memory/Allocators/Internals/RefCountedLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/RefCountedLifetimeGuard.cs deleted file mode 100644 index 61682aa56..000000000 --- a/src/ImageSharp/Memory/Allocators/Internals/RefCountedLifetimeGuard.cs +++ /dev/null @@ -1,56 +0,0 @@ -// 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/RefCountedMemoryLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs new file mode 100644 index 000000000..1c7d6cdfa --- /dev/null +++ b/src/ImageSharp/Memory/Allocators/Internals/RefCountedMemoryLifetimeGuard.cs @@ -0,0 +1,79 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Runtime.InteropServices; +using System.Threading; +using SixLabors.ImageSharp.Diagnostics; + +namespace SixLabors.ImageSharp.Memory.Internals +{ + /// + /// Implements reference counting lifetime guard mechanism for memory resources + /// and maintains the value of . + /// + internal abstract class RefCountedMemoryLifetimeGuard : IDisposable + { + private int refCount = 1; + private int disposed; + private int released; + private string allocationStackTrace; + + protected RefCountedMemoryLifetimeGuard() + { + if (MemoryDiagnostics.UndisposedAllocationSubscribed) + { + this.allocationStackTrace = Environment.StackTrace; + } + + MemoryDiagnostics.IncrementTotalUndisposedAllocationCount(); + } + + ~RefCountedMemoryLifetimeGuard() + { + Interlocked.Exchange(ref this.disposed, 1); + this.ReleaseRef(true); + } + + public bool IsDisposed => this.disposed == 1; + + public void AddRef() => Interlocked.Increment(ref this.refCount); + + public void ReleaseRef() => this.ReleaseRef(false); + + public void Dispose() + { + int wasDisposed = Interlocked.Exchange(ref this.disposed, 1); + if (wasDisposed == 0) + { + this.ReleaseRef(); + GC.SuppressFinalize(this); + } + } + + protected abstract void Release(); + + private void ReleaseRef(bool finalizing) + { + Interlocked.Decrement(ref this.refCount); + if (this.refCount == 0) + { + int wasReleased = Interlocked.Exchange(ref this.released, 1); + + if (wasReleased == 0) + { + if (!finalizing) + { + MemoryDiagnostics.DecrementTotalUndisposedAllocationCount(); + } + else if (this.allocationStackTrace != null) + { + MemoryDiagnostics.RaiseUndisposedMemoryResource(this.allocationStackTrace); + } + + this.Release(); + } + } + } + } +} diff --git a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs index 9302c67e7..2ea76da95 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/SharedArrayPoolBuffer{T}.cs @@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Memory.Internals } } - private sealed class LifetimeGuard : RefCountedLifetimeGuard + private sealed class LifetimeGuard : RefCountedMemoryLifetimeGuard { private byte[] array; diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs index 666b24855..151fef69c 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.LifetimeGuards.cs @@ -20,9 +20,9 @@ namespace SixLabors.ImageSharp.Memory.Internals return buffer; } - public RefCountedLifetimeGuard CreateGroupLifetimeGuard(UnmanagedMemoryHandle[] handles) => new GroupLifetimeGuard(this, handles); + public RefCountedMemoryLifetimeGuard CreateGroupLifetimeGuard(UnmanagedMemoryHandle[] handles) => new GroupLifetimeGuard(this, handles); - private sealed class GroupLifetimeGuard : RefCountedLifetimeGuard + private sealed class GroupLifetimeGuard : RefCountedMemoryLifetimeGuard { private readonly UniformUnmanagedMemoryPool pool; private readonly UnmanagedMemoryHandle[] handles; diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs index 5f0759f20..822112024 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedBufferLifetimeGuard.cs @@ -6,7 +6,7 @@ namespace SixLabors.ImageSharp.Memory.Internals /// /// Defines a strategy for managing unmanaged memory ownership. /// - internal abstract class UnmanagedBufferLifetimeGuard : RefCountedLifetimeGuard + internal abstract class UnmanagedBufferLifetimeGuard : RefCountedMemoryLifetimeGuard { private UnmanagedMemoryHandle handle; diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index 21faf8e56..01aac3148 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp.Memory public sealed class Owned : MemoryGroup, IEnumerable> { private IMemoryOwner[] memoryOwners; - private RefCountedLifetimeGuard groupLifetimeGuard; + private RefCountedMemoryLifetimeGuard groupLifetimeGuard; public Owned(IMemoryOwner[] memoryOwners, int bufferLength, long totalLength, bool swappable) : base(bufferLength, totalLength) diff --git a/tests/ImageSharp.Tests/Memory/Allocators/MemoryDiagnosticsTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/MemoryDiagnosticsTests.cs new file mode 100644 index 000000000..5fab655cb --- /dev/null +++ b/tests/ImageSharp.Tests/Memory/Allocators/MemoryDiagnosticsTests.cs @@ -0,0 +1,122 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +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; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Memory.Allocators +{ + public class MemoryDiagnosticsTests + { + private const int OneMb = 1 << 20; + + private static MemoryAllocator Allocator => Configuration.Default.MemoryAllocator; + + [Theory] + [InlineData(false)] + [InlineData(true)] + 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.TotalUndisposedAllocationCount); + for (int length = 1024; length <= 64 * OneMb; length *= 2) + { + long cntBefore = MemoryDiagnostics.TotalUndisposedAllocationCount; + IDisposable buffer = isGroup ? + Allocator.AllocateGroup(length, 1024) : + Allocator.Allocate(length); + buffers.Add(buffer); + long cntAfter = MemoryDiagnostics.TotalUndisposedAllocationCount; + Assert.True(cntAfter > cntBefore); + } + + foreach (IDisposable buffer in buffers) + { + long cntBefore = MemoryDiagnostics.TotalUndisposedAllocationCount; + buffer.Dispose(); + long cntAfter = MemoryDiagnostics.TotalUndisposedAllocationCount; + Assert.True(cntAfter < cntBefore); + } + + Assert.Equal(0, MemoryDiagnostics.TotalUndisposedAllocationCount); + Assert.Equal(0, leakCounter); + } + } + + [Theory] + [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(), subscribeLeakHandleOuter.ToString()).Dispose(); + + static void RunTest(string isGroupInner, string subscribeLeakHandleInner) + { + bool isGroup = bool.Parse(isGroupInner); + 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.TotalUndisposedAllocationCount; + AllocateAndForget(length, isGroup); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + 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)] + static void AllocateAndForget(int length, bool isGroup) + { + if (isGroup) + { + _ = Allocator.AllocateGroup(length, 1024); + } + else + { + _ = Allocator.Allocate(length); + } + } + } + } +} diff --git a/tests/ImageSharp.Tests/Memory/Allocators/RefCountedLifetimeGuardTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/RefCountedLifetimeGuardTests.cs index 7fb3b7b7b..4b808e863 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/RefCountedLifetimeGuardTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/RefCountedLifetimeGuardTests.cs @@ -110,7 +110,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators } } - private class MockLifetimeGuard : RefCountedLifetimeGuard + private class MockLifetimeGuard : RefCountedMemoryLifetimeGuard { public int ReleaseInvocationCount { get; private set; }