Skip to content

Commit 312ae0f

Browse files
Switch Debounce DispatcherQueueTimer extension to use ConditionalWeakTable
This prevents capture holding onto the timer for garbage collection, validated with a unit test which fails with the ConcurrentDictionary, but succeeds with the ConditionalWeakTable Because of the new Trailing/Leading behavior, we need the table in order to know if something was scheduled, otherwise we'd need reflection if we only stored the event handler, which wouldn't be AOT compatible.
1 parent f92be97 commit 312ae0f

File tree

2 files changed

+58
-7
lines changed

2 files changed

+58
-7
lines changed

components/Extensions/src/Dispatcher/DispatcherQueueTimerExtensions.cs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Collections.Concurrent;
6+
using System.Runtime.CompilerServices;
7+
68

79
#if WINAPPSDK
810
using DispatcherQueueTimer = Microsoft.UI.Dispatching.DispatcherQueueTimer;
@@ -17,8 +19,8 @@ namespace CommunityToolkit.WinUI;
1719
/// </summary>
1820
public static class DispatcherQueueTimerExtensions
1921
{
20-
// TODO: We should use a WeakReference here...
21-
private static ConcurrentDictionary<DispatcherQueueTimer, Action> _debounceInstances = new ConcurrentDictionary<DispatcherQueueTimer, Action>();
22+
//// https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2
23+
private static ConditionalWeakTable<DispatcherQueueTimer, Action> _debounceInstances = new();
2224

2325
/// <summary>
2426
/// <para>Used to debounce (rate-limit) an event. The action will be postponed and executed after the interval has elapsed. At the end of the interval, the function will be called with the arguments that were passed most recently to the debounced function. Useful for smoothing keyboard input, for instance.</para>
@@ -60,11 +62,11 @@ public static void Debounce(this DispatcherQueueTimer timer, Action action, Time
6062
{
6163
// If we have a _debounceInstance queued, then we were running in trailing mode,
6264
// so if we now have the immediate flag, we should ignore this timer, and run immediately.
63-
if (_debounceInstances.ContainsKey(timer))
65+
if (_debounceInstances.TryGetValue(timer, out var _))
6466
{
6567
timeout = false;
6668

67-
_debounceInstances.Remove(timer, out var _);
69+
_debounceInstances.Remove(timer);
6870
}
6971

7072
// If we're in immediate mode then we only execute if the timer wasn't running beforehand
@@ -79,7 +81,7 @@ public static void Debounce(this DispatcherQueueTimer timer, Action action, Time
7981
timer.Tick += Timer_Tick;
8082

8183
// Store/Update function
82-
_debounceInstances.AddOrUpdate(timer, action, (k, v) => action);
84+
_debounceInstances.AddOrUpdate(timer, action);
8385
}
8486

8587
// Start the timer to keep track of the last call here.
@@ -94,8 +96,9 @@ private static void Timer_Tick(object sender, object e)
9496
timer.Tick -= Timer_Tick;
9597
timer.Stop();
9698

97-
if (_debounceInstances.TryRemove(timer, out Action? action))
99+
if (_debounceInstances.TryGetValue(timer, out Action? action))
98100
{
101+
_debounceInstances.Remove(timer);
99102
action?.Invoke();
100103
}
101104
}

components/Extensions/tests/DispatcherQueueTimerExtensionTests.cs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
using CommunityToolkit.Tests;
66
using CommunityToolkit.Tooling.TestGen;
7-
using CommunityToolkit.WinUI;
87

98
#if WINAPPSDK
109
using DispatcherQueue = Microsoft.UI.Dispatching.DispatcherQueue;
@@ -327,4 +326,53 @@ public async Task DispatcherQueueTimer_Debounce_Leading_Switch_Trailing_Interrup
327326
Assert.AreEqual(value3, triggeredValue, "Expected value to now be the last value provided.");
328327
Assert.AreEqual(2, triggeredCount, "Expected to interrupt execution of 2nd request.");
329328
}
329+
330+
[TestCategory("DispatcherQueueTimerExtensions")]
331+
[UIThreadTestMethod]
332+
public async Task DispatcherQueueTimer_Debounce_Trailing_Stop_Lifetime()
333+
{
334+
// Our test indicator
335+
WeakReference? reference = null;
336+
337+
// Still need to capture this on our UI thread
338+
DispatcherQueue _queue = DispatcherQueue.GetForCurrentThread();
339+
340+
await Task.Run(() =>
341+
{
342+
// This test checks the lifetime of the timer and if we hold a reference to it.
343+
var debounceTimer = _queue.CreateTimer();
344+
345+
// Track the DispatcherQueueTimer object
346+
reference = new WeakReference(debounceTimer, true);
347+
348+
var triggeredCount = 0;
349+
string? triggeredValue = null;
350+
351+
var value = "He";
352+
debounceTimer.Debounce(
353+
() =>
354+
{
355+
triggeredCount++;
356+
triggeredValue = value;
357+
},
358+
TimeSpan.FromMilliseconds(60));
359+
360+
// Stop the timer before it would fire, with our proper method to clean-up
361+
debounceTimer.Stop();
362+
363+
Assert.AreEqual(false, debounceTimer.IsRunning, "Expected to stop the timer.");
364+
365+
debounceTimer = null;
366+
});
367+
368+
// Now out of scope and see if GC cleans up
369+
GC.Collect();
370+
GC.WaitForPendingFinalizers();
371+
372+
// Clean-up any UI thread work
373+
await CompositionTargetHelper.ExecuteAfterCompositionRenderingAsync(() => { });
374+
375+
Assert.IsNotNull(reference, "Didn't capture weak reference.");
376+
Assert.IsNull(reference.Target, "Strong reference to DispatcherQueueTimer still exists.");
377+
}
330378
}

0 commit comments

Comments
 (0)