Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit a7d1d26

Browse files
committed
Fix a few more issues in Timer tests
- Several tests were disposing the Timer before the callback with Asserts could be invoked, such that the tests were nops rather than validating what they were trying to validate. - Several tests were modifying shared state without synchronization; very unlikely to have caused a problem, but better safe than sorry. - One test had a race condition that could have resulted in a NullReferenceException, with a callback that attempted to access a captured local before it was initialized. - Several tests were susceptible to the Timer being tested getting finalized in release builds, which would cause the test to fail
1 parent 416019f commit a7d1d26

File tree

1 file changed

+20
-7
lines changed

1 file changed

+20
-7
lines changed

src/System.Threading.Timer/tests/TimerFiringTests.cs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,32 @@ public void Timer_Fires_After_DueTime_Ellapses()
2727
[Fact]
2828
public void Timer_Fires_AndPassesStateThroughCallback()
2929
{
30+
AutoResetEvent are = new AutoResetEvent(false);
31+
3032
object state = new object();
3133
using (var t = new Timer(new TimerCallback((object s) =>
3234
{
3335
Assert.Same(s, state);
34-
}), state, TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(Timeout.Infinite) /* not relevant */)) { }
36+
are.Set();
37+
}), state, TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(Timeout.Infinite) /* not relevant */))
38+
{
39+
Assert.True(are.WaitOne(TimeSpan.FromMilliseconds(MaxPositiveTimeoutInMs)));
40+
}
3541
}
3642

3743
[Fact]
3844
public void Timer_Fires_AndPassesNullStateThroughCallback()
3945
{
46+
AutoResetEvent are = new AutoResetEvent(false);
47+
4048
using (var t = new Timer(new TimerCallback((object s) =>
4149
{
4250
Assert.Null(s);
43-
}), null, TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(Timeout.Infinite) /* not relevant */)) { }
51+
are.Set();
52+
}), null, TimeSpan.FromMilliseconds(100), TimeSpan.FromMilliseconds(Timeout.Infinite) /* not relevant */))
53+
{
54+
Assert.True(are.WaitOne(TimeSpan.FromMilliseconds(MaxPositiveTimeoutInMs)));
55+
}
4456
}
4557

4658
[Fact]
@@ -51,8 +63,7 @@ public void Timer_Fires_After_DueTime_AndOn_Period()
5163

5264
using (var t = new Timer(new TimerCallback((object s) =>
5365
{
54-
count++;
55-
if (count >= 2)
66+
if (Interlocked.Increment(ref count) >= 2)
5667
{
5768
are.Set();
5869
}
@@ -70,8 +81,7 @@ public void Timer_FiresOnlyOnce_OnDueTime_With_InfinitePeriod()
7081

7182
using (var t = new Timer(new TimerCallback((object s) =>
7283
{
73-
count++;
74-
if (count >= 2)
84+
if (Interlocked.Increment(ref count) >= 2)
7585
{
7686
are.Set();
7787
}
@@ -91,8 +101,10 @@ public void Timer_CanDisposeSelfInCallback()
91101
t.Dispose();
92102
are.Set();
93103
});
94-
t = new Timer(tc, null, 1, -1);
104+
t = new Timer(tc, null, -1, -1);
105+
t.Change(1, -1);
95106
Assert.True(are.WaitOne(MaxPositiveTimeoutInMs));
107+
GC.KeepAlive(t);
96108
}
97109

98110
[Fact]
@@ -151,6 +163,7 @@ public void MultpleTimers_PeriodicTimerIsntBlockedByBlockedCallback()
151163
t.Change(1, 50);
152164

153165
Assert.True(b.SignalAndWait(MaxPositiveTimeoutInMs));
166+
GC.KeepAlive(t);
154167
}
155168

156169
[Fact]

0 commit comments

Comments
 (0)