diff --git a/src/System.Private.CoreLib/src/System/Threading/Timer.Unix.cs b/src/System.Private.CoreLib/src/System/Threading/Timer.Unix.cs index 9a51dfbd057..f5ec9e54ee0 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Timer.Unix.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Timer.Unix.cs @@ -19,12 +19,12 @@ internal partial class TimerQueue /// This event is used by the timer thread to wait for timer expiration. It is also /// used to notify the timer thread that a new timer has been set. /// - private static AutoResetEvent s_timerEvent; + private AutoResetEvent _timerEvent; /// /// This field stores the value of next timer that the timer thread should install. /// - private static volatile int s_nextTimerDuration; + private volatile int _nextTimerDuration; private TimerQueue(int id) { @@ -35,21 +35,21 @@ private bool SetTimer(uint actualDuration) // Note: AutoResetEvent.WaitOne takes an Int32 value as a timeout. // The TimerQueue code ensures that timer duration is not greater than max Int32 value Debug.Assert(actualDuration <= (uint)int.MaxValue); - s_nextTimerDuration = (int)actualDuration; + _nextTimerDuration = (int)actualDuration; // If this is the first time the timer is set then we need to create a thread that // will manage and respond to timer requests. Otherwise, simply signal the timer thread // to notify it that the timer duration has changed. - if (s_timerEvent == null) + if (_timerEvent == null) { - s_timerEvent = new AutoResetEvent(false); + _timerEvent = new AutoResetEvent(false); RuntimeThread thread = RuntimeThread.Create(TimerThread, 0); thread.IsBackground = true; // Keep this thread from blocking process shutdown thread.Start(); } else { - s_timerEvent.Set(); + _timerEvent.Set(); } return true; @@ -65,7 +65,7 @@ private void TimerThread() int currentTimerInterval; // Get wait time for the next timer - currentTimerInterval = Interlocked.Exchange(ref s_nextTimerDuration, Timeout.Infinite); + currentTimerInterval = Interlocked.Exchange(ref _nextTimerDuration, Timeout.Infinite); for (;;) { @@ -73,7 +73,7 @@ private void TimerThread() // We will be woken up because either 1) the wait times out, which will indicate that // the current timer has expired and/or 2) the TimerQueue installs a new (earlier) timer. int startWait = TickCount; - bool timerHasExpired = !s_timerEvent.WaitOne(currentTimerInterval); + bool timerHasExpired = !_timerEvent.WaitOne(currentTimerInterval); uint elapsedTime = (uint)(TickCount - startWait); // The timer event can be set after this thread reads the new timer interval but before it enters @@ -99,11 +99,11 @@ private void TimerThread() // When FireNextTimers() installs a new timer, it also sets the timer event. // Reset the event so the timer thread is not woken up right away unnecessary. - s_timerEvent.Reset(); + _timerEvent.Reset(); currentTimerInterval = Timeout.Infinite; } - int nextTimerInterval = Interlocked.Exchange(ref s_nextTimerDuration, Timeout.Infinite); + int nextTimerInterval = Interlocked.Exchange(ref _nextTimerDuration, Timeout.Infinite); if (nextTimerInterval != Timeout.Infinite) { currentTimerInterval = nextTimerInterval; diff --git a/tests/src/Simple/Threading/Threading.cs b/tests/src/Simple/Threading/Threading.cs index 67fb50d6fa1..93879a47289 100644 --- a/tests/src/Simple/Threading/Threading.cs +++ b/tests/src/Simple/Threading/Threading.cs @@ -38,6 +38,9 @@ public static int Main() //Console.WriteLine(" WaitSubsystemTests.MutexMaximumReacquireCountTest"); //WaitSubsystemTests.MutexMaximumReacquireCountTest(); + Console.WriteLine(" TimersCreatedConcurrentlyOnDifferentThreadsAllFire"); + TimerTests.TimersCreatedConcurrentlyOnDifferentThreadsAllFire(); + Console.WriteLine(" ThreadPoolTests.RunProcessorCountItemsInParallel"); ThreadPoolTests.RunProcessorCountItemsInParallel(); @@ -837,6 +840,62 @@ public static void MutexMaximumReacquireCountTest() } } +internal static class TimerTests +{ + [Fact] + public static void TimersCreatedConcurrentlyOnDifferentThreadsAllFire() + { + int processorCount = Environment.ProcessorCount; + + int timerTickCount = 0; + TimerCallback timerCallback = data => Interlocked.Increment(ref timerTickCount); + + var threadStarted = new AutoResetEvent(false); + var createTimers = new ManualResetEvent(false); + var timers = new Timer[processorCount]; + Action createTimerThreadStart = data => + { + int i = (int)data; + var sw = new Stopwatch(); + threadStarted.Set(); + createTimers.WaitOne(); + + // Use the CPU a bit around creating the timer to try to have some of these threads run concurrently + sw.Restart(); + do + { + Thread.SpinWait(1000); + } while (sw.ElapsedMilliseconds < 10); + + timers[i] = new Timer(timerCallback, null, 1, Timeout.Infinite); + + // Use the CPU a bit around creating the timer to try to have some of these threads run concurrently + sw.Restart(); + do + { + Thread.SpinWait(1000); + } while (sw.ElapsedMilliseconds < 10); + }; + + var waitsForThread = new Action[timers.Length]; + for (int i = 0; i < timers.Length; ++i) + { + var t = ThreadTestHelpers.CreateGuardedThread(out waitsForThread[i], createTimerThreadStart); + t.IsBackground = true; + t.Start(i); + threadStarted.CheckedWait(); + } + + createTimers.Set(); + ThreadTestHelpers.WaitForCondition(() => timerTickCount == timers.Length); + + foreach (var waitForThread in waitsForThread) + { + waitForThread(); + } + } +} + internal static class ThreadPoolTests { [Fact] @@ -1353,6 +1412,70 @@ internal static class ThreadTestHelpers public const int ExpectedMeasurableTimeoutMilliseconds = 500; public const int UnexpectedTimeoutMilliseconds = 1000 * 30; + // Wait longer for a thread to time out, so that an unexpected timeout in the thread is more likely to expire first and + // provide a better stack trace for the failure + public const int UnexpectedThreadTimeoutMilliseconds = UnexpectedTimeoutMilliseconds * 2; + + public static Thread CreateGuardedThread(out Action waitForThread, Action start) + { + Action checkForThreadErrors; + return CreateGuardedThread(out checkForThreadErrors, out waitForThread, start); + } + + public static Thread CreateGuardedThread(out Action checkForThreadErrors, out Action waitForThread, Action start) + { + Exception backgroundEx = null; + var t = + new Thread(parameter => + { + try + { + start(parameter); + } + catch (Exception ex) + { + backgroundEx = ex; + Interlocked.MemoryBarrier(); + } + }); + Action localCheckForThreadErrors = checkForThreadErrors = // cannot use ref or out parameters in lambda + () => + { + Interlocked.MemoryBarrier(); + if (backgroundEx != null) + { + throw new AggregateException(backgroundEx); + } + }; + waitForThread = + () => + { + Assert.True(t.Join(UnexpectedThreadTimeoutMilliseconds)); + localCheckForThreadErrors(); + }; + return t; + } + + public static void WaitForCondition(Func condition) + { + WaitForConditionWithCustomDelay(condition, () => Thread.Sleep(1)); + } + + public static void WaitForConditionWithCustomDelay(Func condition, Action delay) + { + if (condition()) + { + return; + } + + var startTime = Environment.TickCount; + do + { + delay(); + Assert.True(Environment.TickCount - startTime < UnexpectedTimeoutMilliseconds); + } while (!condition()); + } + public static void CheckedWait(this WaitHandle wh) { Assert.True(wh.WaitOne(UnexpectedTimeoutMilliseconds)); @@ -1365,12 +1488,23 @@ internal sealed class InvalidWaitHandle : WaitHandle internal sealed class Stopwatch { - private DateTime _startTimeTicks; - private DateTime _endTimeTicks; + private int _startTimeMs; + private int _endTimeMs; + private bool _isStopped; + + public void Restart() + { + _isStopped = false; + _startTimeMs = Environment.TickCount; + } + + public void Stop() + { + _endTimeMs = Environment.TickCount; + _isStopped = true; + } - public void Restart() => _startTimeTicks = DateTime.UtcNow; - public void Stop() => _endTimeTicks = DateTime.UtcNow; - public long ElapsedMilliseconds => (long)(_endTimeTicks - _startTimeTicks).TotalMilliseconds; + public long ElapsedMilliseconds => (_isStopped ? _endTimeMs : Environment.TickCount) - _startTimeMs; } internal static class Assert