Skip to content

Commit 99d1266

Browse files
committed
Async local doesn't actually work for the finalizers, revert that piece.
1 parent d9af239 commit 99d1266

File tree

1 file changed

+40
-56
lines changed

1 file changed

+40
-56
lines changed

src/ImageSharp/Diagnostics/MemoryDiagnostics.cs

Lines changed: 40 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public static class MemoryDiagnostics
1717
{
1818
internal static readonly InteralMemoryDiagnostics Default = new();
1919
private static AsyncLocal<InteralMemoryDiagnostics> localInstance = null;
20+
21+
// the async local end up out of scope during finalizers so putting into thte internal class is useless
22+
private static UndisposedAllocationDelegate undisposedAllocation;
23+
private static int undisposedAllocationSubscriptionCounter;
2024
private static readonly object SyncRoot = new();
2125

2226
/// <summary>
@@ -26,8 +30,23 @@ public static class MemoryDiagnostics
2630
/// </summary>
2731
public static event UndisposedAllocationDelegate UndisposedAllocation
2832
{
29-
add => Current.UndisposedAllocation += value;
30-
remove => Current.UndisposedAllocation -= value;
33+
add
34+
{
35+
lock (SyncRoot)
36+
{
37+
undisposedAllocationSubscriptionCounter++;
38+
undisposedAllocation += value;
39+
}
40+
}
41+
42+
remove
43+
{
44+
lock (SyncRoot)
45+
{
46+
undisposedAllocation -= value;
47+
undisposedAllocationSubscriptionCounter--;
48+
}
49+
}
3150
}
3251

3352
internal static InteralMemoryDiagnostics Current
@@ -61,81 +80,46 @@ internal static InteralMemoryDiagnostics Current
6180
/// </summary>
6281
public static int TotalUndisposedAllocationCount => Current.TotalUndisposedAllocationCount;
6382

64-
internal static bool UndisposedAllocationSubscribed => Current.UndisposedAllocationSubscribed;
83+
internal static bool UndisposedAllocationSubscribed => Volatile.Read(ref undisposedAllocationSubscriptionCounter) > 0;
6584

6685
internal static void IncrementTotalUndisposedAllocationCount() => Current.IncrementTotalUndisposedAllocationCount();
6786

6887
internal static void DecrementTotalUndisposedAllocationCount() => Current.DecrementTotalUndisposedAllocationCount();
6988

7089
internal static void RaiseUndisposedMemoryResource(string allocationStackTrace)
71-
=> Current.RaiseUndisposedMemoryResource(allocationStackTrace);
90+
{
91+
if (undisposedAllocation is null)
92+
{
93+
return;
94+
}
95+
96+
// Schedule on the ThreadPool, to avoid user callback messing up the finalizer thread.
97+
#if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER
98+
ThreadPool.QueueUserWorkItem(
99+
stackTrace => undisposedAllocation?.Invoke(stackTrace),
100+
allocationStackTrace,
101+
preferLocal: false);
102+
#else
103+
ThreadPool.QueueUserWorkItem(
104+
stackTrace => undisposedAllocation?.Invoke((string)stackTrace),
105+
allocationStackTrace);
106+
#endif
107+
}
72108

73109
internal class InteralMemoryDiagnostics
74110
{
75111
private int totalUndisposedAllocationCount;
76112

77-
private UndisposedAllocationDelegate undisposedAllocation;
78-
private int undisposedAllocationSubscriptionCounter;
79-
private readonly object syncRoot = new();
80-
81-
/// <summary>
82-
/// Fires when an ImageSharp object's undisposed memory resource leaks to the finalizer.
83-
/// The event brings significant overhead, and is intended to be used for troubleshooting only.
84-
/// For production diagnostics, use <see cref="TotalUndisposedAllocationCount"/>.
85-
/// </summary>
86-
public event UndisposedAllocationDelegate UndisposedAllocation
87-
{
88-
add
89-
{
90-
lock (this.syncRoot)
91-
{
92-
this.undisposedAllocationSubscriptionCounter++;
93-
this.undisposedAllocation += value;
94-
}
95-
}
96-
97-
remove
98-
{
99-
lock (this.syncRoot)
100-
{
101-
this.undisposedAllocation -= value;
102-
this.undisposedAllocationSubscriptionCounter--;
103-
}
104-
}
105-
}
106-
107113
/// <summary>
108114
/// Gets a value indicating the total number of memory resource objects leaked to the finalizer.
109115
/// </summary>
110116
public int TotalUndisposedAllocationCount => this.totalUndisposedAllocationCount;
111117

112-
internal bool UndisposedAllocationSubscribed => Volatile.Read(ref this.undisposedAllocationSubscriptionCounter) > 0;
113-
114118
internal void IncrementTotalUndisposedAllocationCount() =>
115119
Interlocked.Increment(ref this.totalUndisposedAllocationCount);
116120

117121
internal void DecrementTotalUndisposedAllocationCount() =>
118122
Interlocked.Decrement(ref this.totalUndisposedAllocationCount);
119-
120-
internal void RaiseUndisposedMemoryResource(string allocationStackTrace)
121-
{
122-
if (this.undisposedAllocation is null)
123-
{
124-
return;
125-
}
126-
127-
// Schedule on the ThreadPool, to avoid user callback messing up the finalizer thread.
128-
#if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER
129-
ThreadPool.QueueUserWorkItem(
130-
stackTrace => this.undisposedAllocation?.Invoke(stackTrace),
131-
allocationStackTrace,
132-
preferLocal: false);
133-
#else
134-
ThreadPool.QueueUserWorkItem(
135-
stackTrace => this.undisposedAllocation?.Invoke((string)stackTrace),
136-
allocationStackTrace);
137-
#endif
138-
}
139123
}
140124
}
141125
}

0 commit comments

Comments
 (0)