Fix potential infinite loop in FakeTimeProvider when a timer callback changes the current time #4277
Conversation
… changes the current time Fixes #4275
| if (oldTicks != newTicks) | ||
| { | ||
| // time changed while in the callback, readjust the wake time accordingly | ||
| candidate.WakeupTime = newTicks + candidate.Period; | ||
| } | ||
| else | ||
| { | ||
| // move on to the next period | ||
| candidate.WakeupTime += candidate.Period; | ||
| } |
There was a problem hiding this comment.
Good solution. This is safe because the current thread that is invoking the waiters/callback is the only one that can change _ now, since it is protected by the lock, and it is only that thread that can reenter the lock. So a change to _now could only come from the same thread.
Is my understanding correct?
| @@ -18,7 +18,7 @@ public class FakeTimeProvider : TimeProvider | |||
| internal readonly HashSet<Waiter> Waiters = new(); | |||
| private DateTimeOffset _now = new(2000, 1, 1, 0, 0, 0, 0, TimeSpan.Zero); | |||
| private TimeZoneInfo _localTimeZone = TimeZoneInfo.Utc; | |||
| private int _wakeWaitersGate; | |||
| private volatile int _wakeWaitersGate; | |||
There was a problem hiding this comment.
Didn't think volatile was needed when you read the value using Interlocked.CompareExchange?
Is it because it is being set by direct assignment later?
There was a problem hiding this comment.
Didn't think volatile was needed when you read the value using Interlocked.CompareExchange?
Is it because it is being set by direct assignment later?
Yes, just as an added precaution against funny business in the compiler.
ghost
locked as resolved and limited conversation to collaborators
Fixes #4275
Microsoft Reviewers: Open in CodeFlow