Allow PeriodicTimer to have an infinite timeout (#82986)

* Allow PeriodicTimer to have an infinite timeout

When PeriodicTimer was introduced, we prohibited infinite timeouts as they made no sense: you couldn't change the period after construction, so creating a PeriodicTimer with an infinite timeout meant creating an instance that would never tick.  But now that we've added a Period property with a setter that allows changing the period, infinite timeouts do make sense: as with Timer, they can be used as a mechanism to stop and start the timer, e.g. creating the PeriodicTimer with infinite and only in response to some later action updating its Period to start it.

* Update PeriodicTimer.cs

diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PeriodicTimer.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PeriodicTimer.cs
index 6df8877..27bba5e 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Threading/PeriodicTimer.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PeriodicTimer.cs
@@ -12,7 +12,8 @@ namespace System.Threading
     /// <remarks>
     /// This timer is intended to be used only by a single consumer at a time: only one call to <see cref="WaitForNextTickAsync" />
     /// may be in flight at any given moment.  <see cref="Dispose"/> may be used concurrently with an active <see cref="WaitForNextTickAsync"/>
-    /// to interrupt it and cause it to return false.
+    /// to interrupt it and cause it to return false. Similarly, <see cref="Period"/> may be used concurrently with a consumer accessing
+    /// <see cref="WaitForNextTickAsync"/> in order to change the timer's period.
     /// </remarks>
     public sealed class PeriodicTimer : IDisposable
     {
@@ -23,7 +24,7 @@ namespace System.Threading
 
         /// <summary>Initializes the timer.</summary>
         /// <param name="period">The period between ticks</param>
-        /// <exception cref="ArgumentOutOfRangeException"><paramref name="period"/> must represent a number of milliseconds equal to or larger than 1, and smaller than <see cref="uint.MaxValue"/>.</exception>
+        /// <exception cref="ArgumentOutOfRangeException"><paramref name="period"/> must be <see cref="Timeout.InfiniteTimeSpan"/> or represent a number of milliseconds equal to or larger than 1 and smaller than <see cref="uint.MaxValue"/>.</exception>
         public PeriodicTimer(TimeSpan period)
         {
             if (!TryGetMilliseconds(period, out uint ms))
@@ -37,7 +38,7 @@ namespace System.Threading
         }
 
         /// <summary>Gets or sets the period between ticks.</summary>
-        /// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> must represent a number of milliseconds equal to or larger than 1, and smaller than <see cref="uint.MaxValue"/>.</exception>
+        /// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> must be <see cref="Timeout.InfiniteTimeSpan"/> or represent a number of milliseconds equal to or larger than 1 and smaller than <see cref="uint.MaxValue"/>.</exception>
         /// <remarks>
         /// All prior ticks of the timer, including any that may be waiting to be consumed by <see cref="WaitForNextTickAsync"/>,
         /// are unaffected by changes to <see cref="Period"/>. Setting <see cref="Period"/> affects only and all subsequent times
@@ -45,7 +46,7 @@ namespace System.Threading
         /// </remarks>
         public TimeSpan Period
         {
-            get => TimeSpan.FromMilliseconds(_timer._period);
+            get => _timer._period == Timeout.UnsignedInfinite ? Timeout.InfiniteTimeSpan : TimeSpan.FromMilliseconds(_timer._period);
             set
             {
                 if (!TryGetMilliseconds(value, out uint ms))
@@ -65,7 +66,7 @@ namespace System.Threading
         private static bool TryGetMilliseconds(TimeSpan value, out uint milliseconds)
         {
             long ms = (long)value.TotalMilliseconds;
-            if (ms >= 1 && ms <= Timer.MaxSupportedTimeout)
+            if ((ms >= 1 && ms <= Timer.MaxSupportedTimeout) || value == Timeout.InfiniteTimeSpan)
             {
                 milliseconds = (uint)ms;
                 return true;
@@ -121,6 +122,11 @@ namespace System.Threading
             /// PeriodicTimer to be finalized and unroot the TimerQueueTimer. Thus, we keep this field set during<see cref="WaitForNextTickAsync"/>
             /// so that the timer roots any async continuation chain awaiting it, and then keep it unset otherwise so that everything
             /// can be GC'd appropriately.
+            ///
+            /// Note that if the period is set to infinite, even when there's an active waiter the PeriodicTimer won't
+            /// be rooted because TimerQueueTimer won't be rooted via the static linked list.  That's fine, as the timer
+            /// will never tick in such a case, and for the timer's period to be changed, the user's code would need
+            /// some other reference to PeriodicTimer keeping it alive, anyway.
             /// </remarks>
             private PeriodicTimer? _owner;
             /// <summary>Core of the <see cref="IValueTaskSource{TResult}"/> implementation.</summary>

diff --git a/src/libraries/System.Runtime/tests/System/Threading/PeriodicTimerTests.cs b/src/libraries/System.Runtime/tests/System/Threading/PeriodicTimerTests.cs
index 82d6b0a..231a192 100644 (file)
--- a/src/libraries/System.Runtime/tests/System/Threading/PeriodicTimerTests.cs
+++ b/src/libraries/System.Runtime/tests/System/Threading/PeriodicTimerTests.cs
@@ -12,15 +12,15 @@ namespace System.Threading.Tests
         [Fact]
         public void Ctor_InvalidArguments_Throws()
         {
-            AssertExtensions.Throws<ArgumentOutOfRangeException>("period", () => new PeriodicTimer(TimeSpan.FromMilliseconds(-1)));
             AssertExtensions.Throws<ArgumentOutOfRangeException>("period", () => new PeriodicTimer(TimeSpan.Zero));
             AssertExtensions.Throws<ArgumentOutOfRangeException>("period", () => new PeriodicTimer(TimeSpan.FromMilliseconds(uint.MaxValue)));
         }
 
         [Theory]
+        [InlineData(-1)]
         [InlineData(1)]
         [InlineData(uint.MaxValue - 1)]
-        public void Ctor_ValidArguments_Succeeds(uint milliseconds)
+        public void Ctor_ValidArguments_Succeeds(double milliseconds)
         {
             using var timer = new PeriodicTimer(TimeSpan.FromMilliseconds(milliseconds));
         }
@@ -29,7 +29,6 @@ namespace System.Threading.Tests
         public void Period_InvalidArguments_Throws()
         {
             PeriodicTimer timer = new PeriodicTimer(TimeSpan.FromMilliseconds(1));
-            AssertExtensions.Throws<ArgumentOutOfRangeException>("value", () => timer.Period = TimeSpan.FromMilliseconds(-1));
             AssertExtensions.Throws<ArgumentOutOfRangeException>("value", () => timer.Period = TimeSpan.Zero);
             AssertExtensions.Throws<ArgumentOutOfRangeException>("value", () => timer.Period = TimeSpan.FromMilliseconds(uint.MaxValue));
 
@@ -43,6 +42,9 @@ namespace System.Threading.Tests
             using PeriodicTimer timer = new PeriodicTimer(TimeSpan.FromMilliseconds(1));
             Assert.Equal(TimeSpan.FromMilliseconds(1), timer.Period);
 
+            timer.Period = Timeout.InfiniteTimeSpan;
+            Assert.Equal(Timeout.InfiniteTimeSpan, timer.Period);
+
             timer.Period = TimeSpan.FromDays(1);
             Assert.Equal(TimeSpan.FromDays(1), timer.Period);
 
@@ -53,7 +55,7 @@ namespace System.Threading.Tests
         [Fact]
         public async Task Period_AffectsPendingWaits()
         {
-            using PeriodicTimer timer = new PeriodicTimer(TimeSpan.FromDays(40));
+            using PeriodicTimer timer = new PeriodicTimer(Timeout.InfiniteTimeSpan);
 
             ValueTask<bool> task = timer.WaitForNextTickAsync();
             Assert.False(task.IsCompleted);