Skip to content

Commit 812dfb9

Browse files
authored
Fix TimerAwaitable rooting (#20937)
* Fix TimerAwaitable rooting - This fixes an issue where components that use timer awaitable currently need to be explicitly stopped and disposed for it to be unrooted. This makes it possible to write a finalizer.
1 parent 54408ae commit 812dfb9

File tree

2 files changed

+84
-2
lines changed

2 files changed

+84
-2
lines changed
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Runtime.CompilerServices;
6+
using System.Threading.Tasks;
7+
using Microsoft.AspNetCore.Internal;
8+
using Xunit;
9+
10+
namespace Microsoft.AspNetCore.SignalR.Client.Tests
11+
{
12+
public class TimerAwaitableTests
13+
{
14+
[Fact]
15+
public void FinalizerRunsIfTimerAwaitableReferencesObject()
16+
{
17+
var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
18+
UseTimerAwaitableAndUnref(tcs);
19+
20+
// Make sure it *really* cleans up
21+
for (int i = 0; i < 5 && !tcs.Task.IsCompleted; i++)
22+
{
23+
GC.Collect();
24+
GC.WaitForPendingFinalizers();
25+
}
26+
27+
// Make sure the finalizer runs
28+
Assert.True(tcs.Task.IsCompleted);
29+
}
30+
31+
[MethodImpl(MethodImplOptions.NoInlining)]
32+
private void UseTimerAwaitableAndUnref(TaskCompletionSource<object> tcs)
33+
{
34+
_ = new ObjectWithTimerAwaitable(tcs).Start();
35+
}
36+
}
37+
38+
// This object holds onto a TimerAwaitable referencing the callback (the async continuation is the callback)
39+
// it also has a finalizer that triggers a tcs so callers can be notified when this object is being cleaned up.
40+
public class ObjectWithTimerAwaitable
41+
{
42+
private readonly TimerAwaitable _timer;
43+
private readonly TaskCompletionSource<object> _tcs;
44+
private int _count;
45+
46+
public ObjectWithTimerAwaitable(TaskCompletionSource<object> tcs)
47+
{
48+
_tcs = tcs;
49+
_timer = new TimerAwaitable(TimeSpan.Zero, TimeSpan.FromSeconds(1));
50+
_timer.Start();
51+
}
52+
53+
public async Task Start()
54+
{
55+
using (_timer)
56+
{
57+
while (await _timer)
58+
{
59+
_count++;
60+
}
61+
}
62+
}
63+
64+
~ObjectWithTimerAwaitable()
65+
{
66+
_tcs.TrySetResult(null);
67+
}
68+
}
69+
}

src/SignalR/common/Shared/TimerAwaitable.cs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,20 @@ public void Start()
5050
restoreFlow = true;
5151
}
5252

53-
_timer = new Timer(state => ((TimerAwaitable)state).Tick(), this, _dueTime, _period);
53+
// This fixes the cycle by using a WeakReference to the state object. The object graph now looks like this:
54+
// Timer -> TimerHolder -> TimerQueueTimer -> WeakReference<TimerAwaitable> -> Timer -> ...
55+
// If TimerAwaitable falls out of scope, the timer should be released.
56+
_timer = new Timer(state =>
57+
{
58+
var weakRef = (WeakReference<TimerAwaitable>)state;
59+
if (weakRef.TryGetTarget(out var thisRef))
60+
{
61+
thisRef.Tick();
62+
}
63+
},
64+
new WeakReference<TimerAwaitable>(this),
65+
_dueTime,
66+
_period);
5467
}
5568
finally
5669
{
@@ -125,4 +138,4 @@ void IDisposable.Dispose()
125138
}
126139
}
127140
}
128-
}
141+
}

0 commit comments

Comments
 (0)