From d20a959c449515482a8e98690130f2be26e81699 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Fri, 7 Apr 2023 15:46:45 -0700 Subject: [PATCH 1/4] Apply the TimeProvider design feedback --- .../Common/src/System/TimeProvider.cs | 287 ++++++++---------- .../Common/tests/System/TimeProviderTests.cs | 61 ++-- .../ref/Microsoft.Bcl.TimeProvider.cs | 15 +- .../System.Runtime/ref/System.Runtime.cs | 15 +- 4 files changed, 180 insertions(+), 198 deletions(-) diff --git a/src/libraries/Common/src/System/TimeProvider.cs b/src/libraries/Common/src/System/TimeProvider.cs index b8d7fb9fa6aa2..dea542a66f4f2 100644 --- a/src/libraries/Common/src/System/TimeProvider.cs +++ b/src/libraries/Common/src/System/TimeProvider.cs @@ -10,36 +10,21 @@ namespace System /// Provides an abstraction for time. public abstract class TimeProvider { - private readonly double _timeToTicksRatio; - /// /// Gets a that provides a clock based on , /// a time zone based on , a high-performance time stamp based on , /// and a timer based on . /// /// - /// If the changes after the object is returned, the change will be reflected in any subsequent operations that retrieve . + /// If the changes after the object is returned, the change will be reflected in any subsequent operations that retrieve . /// - public static TimeProvider System { get; } = new SystemTimeProvider(null); + public static TimeProvider System { get; } = new SystemTimeProvider(); /// - /// Initializes the instance with the timestamp frequency. + /// Initializes new instance of . /// - /// The value of is negative or zero. - /// Frequency of the values returned from method. - protected TimeProvider(long timestampFrequency) + protected TimeProvider() { -#if SYSTEM_PRIVATE_CORELIB - ArgumentOutOfRangeException.ThrowIfNegativeOrZero(timestampFrequency); -#else - if (timestampFrequency <= 0) - { - throw new ArgumentOutOfRangeException(nameof(timestampFrequency), timestampFrequency, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeNonNegativeNonZero, nameof(timestampFrequency))); - } -#endif // SYSTEM_PRIVATE_CORELIB - - TimestampFrequency = timestampFrequency; - _timeToTicksRatio = (double)TimeSpan.TicksPerSecond / TimestampFrequency; } /// @@ -47,69 +32,56 @@ protected TimeProvider(long timestampFrequency) /// Coordinated Universal Time (UTC) date and time and whose offset is Zero, /// all according to this 's notion of time. /// - public abstract DateTimeOffset UtcNow { get; } + /// + /// The default implementation returns . + /// + public virtual DateTimeOffset GetUtcNow() => DateTimeOffset.UtcNow; private static readonly long s_minDateTicks = DateTime.MinValue.Ticks; private static readonly long s_maxDateTicks = DateTime.MaxValue.Ticks; /// /// Gets a value that is set to the current date and time according to this 's - /// notion of time based on , with the offset set to the 's offset from Coordinated Universal Time (UTC). + /// notion of time based on , with the offset set to the 's offset from Coordinated Universal Time (UTC). /// - public DateTimeOffset LocalNow + public DateTimeOffset GetLocalNow() { - get - { - DateTime utcDateTime = UtcNow.UtcDateTime; - TimeSpan offset = LocalTimeZone.GetUtcOffset(utcDateTime); - - long localTicks = utcDateTime.Ticks + offset.Ticks; - if ((ulong)localTicks > (ulong)s_maxDateTicks) - { - localTicks = localTicks < s_minDateTicks ? s_minDateTicks : s_maxDateTicks; - } + DateTimeOffset utcDateTime = GetUtcNow(); + TimeSpan offset = (LocalTimeZone ?? TimeZoneInfo.Local).GetUtcOffset(utcDateTime); - return new DateTimeOffset(localTicks, offset); + long localTicks = utcDateTime.Ticks + offset.Ticks; + if ((ulong)localTicks > (ulong)s_maxDateTicks) + { + localTicks = localTicks < s_minDateTicks ? s_minDateTicks : s_maxDateTicks; } + + return new DateTimeOffset(localTicks, offset); } /// /// Gets a object that represents the local time zone according to this 's notion of time. /// - public abstract TimeZoneInfo LocalTimeZone { get; } + /// + /// The default implementation returns . + /// + public virtual TimeZoneInfo LocalTimeZone { get => TimeZoneInfo.Local; } /// /// Gets the frequency of of high-frequency value per second. /// - public long TimestampFrequency { get; } - - /// - /// Creates a that provides a clock based on , - /// a time zone based on , a high-performance time stamp based on , - /// and a timer based on . - /// - /// The time zone to use in getting the local time using . - /// A new instance of . - /// is null. - public static TimeProvider FromLocalTimeZone(TimeZoneInfo timeZone) - { -#if SYSTEM_PRIVATE_CORELIB - ArgumentNullException.ThrowIfNull(timeZone); -#else - if (timeZone is null) - { - throw new ArgumentNullException(nameof(timeZone)); - } -#endif // SYSTEM_PRIVATE_CORELIB - - return new SystemTimeProvider(timeZone); - } + /// + /// The default implementation returns . + /// + public virtual long TimestampFrequency { get => Stopwatch.Frequency; } /// /// Gets the current high-frequency value designed to measure small time intervals with high accuracy in the timer mechanism. /// /// A long integer representing the high-frequency counter value of the underlying timer mechanism. - public abstract long GetTimestamp(); + /// + /// The default implementation returns . + /// + public virtual long GetTimestamp() => Stopwatch.GetTimestamp(); /// /// Gets the elapsed time between two timestamps retrieved using . @@ -117,8 +89,21 @@ public static TimeProvider FromLocalTimeZone(TimeZoneInfo timeZone) /// The timestamp marking the beginning of the time period. /// The timestamp marking the end of the time period. /// A for the elapsed time between the starting and ending timestamps. - public TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) => - new TimeSpan((long)((endingTimestamp - startingTimestamp) * _timeToTicksRatio)); + public TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) + { + long timestampFrequency = TimestampFrequency; + if (timestampFrequency <= 0) + { +#if SYSTEM_PRIVATE_CORELIB + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(TimestampFrequency); +#else + string propertyName = nameof(TimestampFrequency); + throw new ArgumentOutOfRangeException(propertyName, timestampFrequency, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeNonNegativeNonZero, nameof(TimestampFrequency))); +#endif // SYSTEM_PRIVATE_CORELIB + } + + return new TimeSpan((long)((endingTimestamp - startingTimestamp) * ((double)TimeSpan.TicksPerSecond / timestampFrequency))); + } /// Creates a new instance, using values to measure time intervals. /// @@ -153,136 +138,122 @@ public TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) => /// each time it's called. That capture can be suppressed with . /// /// - public abstract ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period); - - /// - /// Provides a default implementation of based on , - /// , , and . - /// - private sealed class SystemTimeProvider : TimeProvider + public virtual ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) { - /// The time zone to treat as local. If null, is used. - private readonly TimeZoneInfo? _localTimeZone; - - /// Initializes the instance. - /// The time zone to treat as local. If null, is used. - internal SystemTimeProvider(TimeZoneInfo? localTimeZone) : base(Stopwatch.Frequency) => _localTimeZone = localTimeZone; - - /// - public override TimeZoneInfo LocalTimeZone => _localTimeZone ?? TimeZoneInfo.Local; - - /// - public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) - { #if SYSTEM_PRIVATE_CORELIB - ArgumentNullException.ThrowIfNull(callback); + ArgumentNullException.ThrowIfNull(callback); #else - if (callback is null) - { - throw new ArgumentNullException(nameof(callback)); - } -#endif // SYSTEM_PRIVATE_CORELIB - - return new SystemTimeProviderTimer(dueTime, period, callback, state); + if (callback is null) + { + throw new ArgumentNullException(nameof(callback)); } +#endif // SYSTEM_PRIVATE_CORELIB - /// - public override long GetTimestamp() => Stopwatch.GetTimestamp(); - - /// - public override DateTimeOffset UtcNow => DateTimeOffset.UtcNow; + return new SystemTimeProviderTimer(dueTime, period, callback, state); + } - /// Thin wrapper for a . - /// - /// We don't return a TimerQueueTimer directly as it implements IThreadPoolWorkItem and we don't - /// want it exposed in a way that user code could directly queue the timer to the thread pool. - /// We also use this instead of Timer because CreateTimer needs to return a timer that's implicitly - /// rooted while scheduled. - /// - private sealed class SystemTimeProviderTimer : ITimer - { + /// Thin wrapper for a . + /// + /// We don't return a TimerQueueTimer directly as it implements IThreadPoolWorkItem and we don't + /// want it exposed in a way that user code could directly queue the timer to the thread pool. + /// We also use this instead of Timer because CreateTimer needs to return a timer that's implicitly + /// rooted while scheduled. + /// + private sealed class SystemTimeProviderTimer : ITimer + { #if SYSTEM_PRIVATE_CORELIB - private readonly TimerQueueTimer _timer; + private readonly TimerQueueTimer _timer; #else - private readonly Timer _timer; + private readonly Timer _timer; #endif // SYSTEM_PRIVATE_CORELIB - public SystemTimeProviderTimer(TimeSpan dueTime, TimeSpan period, TimerCallback callback, object? state) - { - (uint duration, uint periodTime) = CheckAndGetValues(dueTime, period); + public SystemTimeProviderTimer(TimeSpan dueTime, TimeSpan period, TimerCallback callback, object? state) + { + (uint duration, uint periodTime) = CheckAndGetValues(dueTime, period); #if SYSTEM_PRIVATE_CORELIB - _timer = new TimerQueueTimer(callback, state, duration, periodTime, flowExecutionContext: true); + _timer = new TimerQueueTimer(callback, state, duration, periodTime, flowExecutionContext: true); #else - // We want to ensure the timer we create will be tracked as long as it is scheduled. - // To do that, we call the constructor which track only the callback which will make the time to be tracked by the scheduler - // then we call Change on the timer to set the desired duration and period. - _timer = new Timer(_ => callback(state)); - _timer.Change(duration, periodTime); + // We want to ensure the timer we create will be tracked as long as it is scheduled. + // To do that, we call the constructor which track only the callback which will make the time to be tracked by the scheduler + // then we call Change on the timer to set the desired duration and period. + _timer = new Timer(_ => callback(state)); + _timer.Change(duration, periodTime); #endif // SYSTEM_PRIVATE_CORELIB - } + } - public bool Change(TimeSpan dueTime, TimeSpan period) + public bool Change(TimeSpan dueTime, TimeSpan period) + { + (uint duration, uint periodTime) = CheckAndGetValues(dueTime, period); + try { - (uint duration, uint periodTime) = CheckAndGetValues(dueTime, period); - try - { - return _timer.Change(duration, periodTime); - } - catch (ObjectDisposedException) - { - return false; - } + return _timer.Change(duration, periodTime); } + catch (ObjectDisposedException) + { + return false; + } + } - public void Dispose() => _timer.Dispose(); + public void Dispose() => _timer.Dispose(); #if SYSTEM_PRIVATE_CORELIB - public ValueTask DisposeAsync() => _timer.DisposeAsync(); + public ValueTask DisposeAsync() => _timer.DisposeAsync(); #else - public ValueTask DisposeAsync() - { - _timer.Dispose(); - return default; - } + public ValueTask DisposeAsync() + { + _timer.Dispose(); + return default; + } #endif // SYSTEM_PRIVATE_CORELIB - private static (uint duration, uint periodTime) CheckAndGetValues(TimeSpan dueTime, TimeSpan periodTime) - { - long dueTm = (long)dueTime.TotalMilliseconds; - long periodTm = (long)periodTime.TotalMilliseconds; + private static (uint duration, uint periodTime) CheckAndGetValues(TimeSpan dueTime, TimeSpan periodTime) + { + long dueTm = (long)dueTime.TotalMilliseconds; + long periodTm = (long)periodTime.TotalMilliseconds; #if SYSTEM_PRIVATE_CORELIB - ArgumentOutOfRangeException.ThrowIfLessThan(dueTm, -1, nameof(dueTime)); - ArgumentOutOfRangeException.ThrowIfGreaterThan(dueTm, Timer.MaxSupportedTimeout, nameof(dueTime)); + ArgumentOutOfRangeException.ThrowIfLessThan(dueTm, -1, nameof(dueTime)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(dueTm, Timer.MaxSupportedTimeout, nameof(dueTime)); - ArgumentOutOfRangeException.ThrowIfLessThan(periodTm, -1, nameof(periodTime)); - ArgumentOutOfRangeException.ThrowIfGreaterThan(periodTm, Timer.MaxSupportedTimeout, nameof(periodTime)); + ArgumentOutOfRangeException.ThrowIfLessThan(periodTm, -1, nameof(periodTime)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(periodTm, Timer.MaxSupportedTimeout, nameof(periodTime)); #else - const uint MaxSupportedTimeout = 0xfffffffe; + const uint MaxSupportedTimeout = 0xfffffffe; - if (dueTm < -1) - { - throw new ArgumentOutOfRangeException(nameof(dueTime), dueTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreaterOrEqual, nameof(dueTime), -1)); - } + if (dueTm < -1) + { + throw new ArgumentOutOfRangeException(nameof(dueTime), dueTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreaterOrEqual, nameof(dueTime), -1)); + } - if (dueTm > MaxSupportedTimeout) - { - throw new ArgumentOutOfRangeException(nameof(dueTime), dueTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeLessOrEqual, nameof(dueTime), MaxSupportedTimeout)); - } + if (dueTm > MaxSupportedTimeout) + { + throw new ArgumentOutOfRangeException(nameof(dueTime), dueTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeLessOrEqual, nameof(dueTime), MaxSupportedTimeout)); + } - if (periodTm < -1) - { - throw new ArgumentOutOfRangeException(nameof(periodTm), periodTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreaterOrEqual, nameof(periodTm), -1)); - } + if (periodTm < -1) + { + throw new ArgumentOutOfRangeException(nameof(periodTm), periodTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreaterOrEqual, nameof(periodTm), -1)); + } - if (periodTm > MaxSupportedTimeout) - { - throw new ArgumentOutOfRangeException(nameof(periodTm), periodTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeLessOrEqual, nameof(periodTm), MaxSupportedTimeout)); - } + if (periodTm > MaxSupportedTimeout) + { + throw new ArgumentOutOfRangeException(nameof(periodTm), periodTm, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeLessOrEqual, nameof(periodTm), MaxSupportedTimeout)); + } #endif // SYSTEM_PRIVATE_CORELIB - return ((uint)dueTm, (uint)periodTm); - } + return ((uint)dueTm, (uint)periodTm); + } + } + + /// + /// Used to create a instance returned from and uses the default implementation + /// provided by which uses , , , and . + /// + private sealed class SystemTimeProvider : TimeProvider + { + /// Initializes the instance. + internal SystemTimeProvider() : base() + { } } } diff --git a/src/libraries/Common/tests/System/TimeProviderTests.cs b/src/libraries/Common/tests/System/TimeProviderTests.cs index 2f169512adabd..5c34bea8defd6 100644 --- a/src/libraries/Common/tests/System/TimeProviderTests.cs +++ b/src/libraries/Common/tests/System/TimeProviderTests.cs @@ -17,7 +17,7 @@ public class TimeProviderTests public void TestUtcSystemTime() { DateTimeOffset dto1 = DateTimeOffset.UtcNow; - DateTimeOffset providerDto = TimeProvider.System.UtcNow; + DateTimeOffset providerDto = TimeProvider.System.GetUtcNow(); DateTimeOffset dto2 = DateTimeOffset.UtcNow; Assert.InRange(providerDto.Ticks, dto1.Ticks, dto2.Ticks); @@ -28,7 +28,7 @@ public void TestUtcSystemTime() public void TestLocalSystemTime() { DateTimeOffset dto1 = DateTimeOffset.Now; - DateTimeOffset providerDto = TimeProvider.System.LocalNow; + DateTimeOffset providerDto = TimeProvider.System.GetLocalNow(); DateTimeOffset dto2 = DateTimeOffset.Now; // Ensure there was no daylight saving shift during the test execution. @@ -39,6 +39,17 @@ public void TestLocalSystemTime() } } + private class ZonedTimeProvider : TimeProvider + { + private TimeZoneInfo _zoneInfo; + public ZonedTimeProvider(TimeZoneInfo zoneInfo) : base() + { + _zoneInfo = zoneInfo ?? TimeZoneInfo.Local; + } + public override TimeZoneInfo LocalTimeZone { get => _zoneInfo; } + public static TimeProvider FromLocalTimeZone(TimeZoneInfo zoneInfo) => new ZonedTimeProvider(zoneInfo); + } + [Fact] public void TestSystemProviderWithTimeZone() { @@ -50,11 +61,11 @@ public void TestSystemProviderWithTimeZone() TimeZoneInfo tzi = TimeZoneInfo.FindSystemTimeZoneById(OperatingSystem.IsWindows() ? "Pacific Standard Time" : "America/Los_Angeles"); #endif // NETFRAMEWORK - TimeProvider tp = TimeProvider.FromLocalTimeZone(tzi); + TimeProvider tp = ZonedTimeProvider.FromLocalTimeZone(tzi); Assert.Equal(tzi.Id, tp.LocalTimeZone.Id); DateTimeOffset utcDto1 = DateTimeOffset.UtcNow; - DateTimeOffset localDto = tp.LocalNow; + DateTimeOffset localDto = tp.GetLocalNow(); DateTimeOffset utcDto2 = DateTimeOffset.UtcNow; DateTimeOffset utcConvertedDto = TimeZoneInfo.ConvertTime(localDto, TimeZoneInfo.Utc); @@ -139,12 +150,12 @@ public void FastClockTest() for (int i = 0; i < 20; i++) { - DateTimeOffset fastNow = fastClock.UtcNow; + DateTimeOffset fastNow = fastClock.GetUtcNow(); DateTimeOffset now = DateTimeOffset.UtcNow; Assert.True(fastNow > now, $"Expected {fastNow} > {now}"); - fastNow = fastClock.LocalNow; + fastNow = fastClock.GetLocalNow(); now = DateTimeOffset.Now; Assert.True(fastNow > now, $"Expected {fastNow} > {now}"); @@ -375,9 +386,10 @@ public static async void PeriodicTimerTests(TimeProvider provider) [Fact] public static void NegativeTests() { - Assert.Throws(() => new FastClock(-1)); // negative frequency - Assert.Throws(() => new FastClock(0)); // zero frequency - Assert.Throws(() => TimeProvider.FromLocalTimeZone(null)); + FastClock clock = new FastClock(-1); // negative frequency + Assert.Throws(() => clock.GetElapsedTime(1, 2)); + clock = new FastClock(0); // zero frequency + Assert.Throws(() => clock.GetElapsedTime(1, 2)); Assert.Throws(() => TimeProvider.System.CreateTimer(null, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan)); Assert.Throws(() => TimeProvider.System.CreateTimer(obj => { }, null, TimeSpan.FromMilliseconds(-2), Timeout.InfiniteTimeSpan)); @@ -414,34 +426,35 @@ class FastClock : TimeProvider { private long _minutesToAdd; private TimeZoneInfo _zone; + private long _timestampFrequency; - public FastClock(long timestampFrequency = TimeSpan.TicksPerSecond, TimeZoneInfo? zone = null) : base(timestampFrequency) + public FastClock(long timestampFrequency = TimeSpan.TicksPerSecond, TimeZoneInfo? zone = null) : base() { + _timestampFrequency = timestampFrequency; _zone = zone ?? TimeZoneInfo.Local; } - public override DateTimeOffset UtcNow + public override DateTimeOffset GetUtcNow() { - get - { - DateTimeOffset now = DateTimeOffset.UtcNow; - - _minutesToAdd++; - long remainingTicks = (DateTimeOffset.MaxValue.Ticks - now.Ticks); + DateTimeOffset now = DateTimeOffset.UtcNow; - if (_minutesToAdd * TimeSpan.TicksPerMinute > remainingTicks) - { - _minutesToAdd = 0; - return now; - } + _minutesToAdd++; + long remainingTicks = (DateTimeOffset.MaxValue.Ticks - now.Ticks); - return now.AddMinutes(_minutesToAdd); + if (_minutesToAdd * TimeSpan.TicksPerMinute > remainingTicks) + { + _minutesToAdd = 0; + return now; } + + return now.AddMinutes(_minutesToAdd); } + public override long TimestampFrequency { get => _timestampFrequency; } + public override TimeZoneInfo LocalTimeZone => _zone; - public override long GetTimestamp() => UtcNow.Ticks; + public override long GetTimestamp() => GetUtcNow().Ticks; public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) => new FastTimer(callback, state, dueTime, period); diff --git a/src/libraries/Microsoft.Bcl.TimeProvider/ref/Microsoft.Bcl.TimeProvider.cs b/src/libraries/Microsoft.Bcl.TimeProvider/ref/Microsoft.Bcl.TimeProvider.cs index 3d11ef6fa5418..fd3b8e759ce7d 100644 --- a/src/libraries/Microsoft.Bcl.TimeProvider/ref/Microsoft.Bcl.TimeProvider.cs +++ b/src/libraries/Microsoft.Bcl.TimeProvider/ref/Microsoft.Bcl.TimeProvider.cs @@ -8,15 +8,14 @@ namespace System public abstract class TimeProvider { public static TimeProvider System { get; } - protected TimeProvider(long timestampFrequency) { throw null; } - public abstract System.DateTimeOffset UtcNow { get; } - public System.DateTimeOffset LocalNow { get; } - public abstract System.TimeZoneInfo LocalTimeZone { get; } - public long TimestampFrequency { get; } - public static TimeProvider FromLocalTimeZone(System.TimeZoneInfo timeZone) { throw null; } - public abstract long GetTimestamp(); + protected TimeProvider() { throw null; } + public virtual System.DateTimeOffset GetUtcNow() { throw null; } + public System.DateTimeOffset GetLocalNow() { throw null; } + public virtual System.TimeZoneInfo LocalTimeZone { get; } + public virtual long TimestampFrequency { get; } + public virtual long GetTimestamp() { throw null; } public TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) { throw null; } - public abstract System.Threading.ITimer CreateTimer(System.Threading.TimerCallback callback, object? state, System.TimeSpan dueTime, System.TimeSpan period); + public virtual System.Threading.ITimer CreateTimer(System.Threading.TimerCallback callback, object? state, System.TimeSpan dueTime, System.TimeSpan period) { throw null; } } } namespace System.Threading diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 86305a208d9be..55b5380ed5e5a 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -1854,15 +1854,14 @@ public enum DayOfWeek public abstract class TimeProvider { public static TimeProvider System { get; } - protected TimeProvider(long timestampFrequency) { throw null; } - public abstract System.DateTimeOffset UtcNow { get; } - public System.DateTimeOffset LocalNow { get; } - public abstract System.TimeZoneInfo LocalTimeZone { get; } - public long TimestampFrequency { get; } - public static TimeProvider FromLocalTimeZone(System.TimeZoneInfo timeZone) { throw null; } - public abstract long GetTimestamp(); + protected TimeProvider() { throw null; } + public virtual System.DateTimeOffset GetUtcNow() { throw null; } + public System.DateTimeOffset GetLocalNow() { throw null; } + public virtual System.TimeZoneInfo LocalTimeZone { get; } + public virtual long TimestampFrequency { get; } + public virtual long GetTimestamp() { throw null; } public TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) { throw null; } - public abstract System.Threading.ITimer CreateTimer(System.Threading.TimerCallback callback, object? state, System.TimeSpan dueTime, System.TimeSpan period); + public virtual System.Threading.ITimer CreateTimer(System.Threading.TimerCallback callback, object? state, System.TimeSpan dueTime, System.TimeSpan period) { throw null; } } public sealed partial class DBNull : System.IConvertible, System.Runtime.Serialization.ISerializable { From 8cf4c07c06e9648514928d1dad6492aa1611a3fe Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Sat, 8 Apr 2023 13:18:02 -0700 Subject: [PATCH 2/4] Address the feedback --- src/libraries/Common/src/System/TimeProvider.cs | 15 ++++++++++++--- .../src/Resources/Strings.resx | 3 +++ .../src/Resources/Strings.resx | 3 +++ .../src/System/ThrowHelper.cs | 3 +++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/src/System/TimeProvider.cs b/src/libraries/Common/src/System/TimeProvider.cs index dea542a66f4f2..1d3848900ca40 100644 --- a/src/libraries/Common/src/System/TimeProvider.cs +++ b/src/libraries/Common/src/System/TimeProvider.cs @@ -21,7 +21,7 @@ public abstract class TimeProvider public static TimeProvider System { get; } = new SystemTimeProvider(); /// - /// Initializes new instance of . + /// Initializes the . /// protected TimeProvider() { @@ -47,7 +47,16 @@ protected TimeProvider() public DateTimeOffset GetLocalNow() { DateTimeOffset utcDateTime = GetUtcNow(); - TimeSpan offset = (LocalTimeZone ?? TimeZoneInfo.Local).GetUtcOffset(utcDateTime); + TimeZoneInfo zoneInfo = LocalTimeZone; + if (zoneInfo is null) + { +#if SYSTEM_PRIVATE_CORELIB + ThrowHelper.ThrowInvalidOperationException(ExceptionResource.InvalidOperation_TimeProviderNullLocalTimeZone); +#else + throw new InvalidOperationException(SR.InvalidOperation_TimeProviderNullLocalTimeZone); +#endif // SYSTEM_PRIVATE_CORELIB + } + TimeSpan offset = zoneInfo.GetUtcOffset(utcDateTime); long localTicks = utcDateTime.Ticks + offset.Ticks; if ((ulong)localTicks > (ulong)s_maxDateTicks) @@ -70,7 +79,7 @@ public DateTimeOffset GetLocalNow() /// Gets the frequency of of high-frequency value per second. /// /// - /// The default implementation returns . + /// The default implementation returns . For a given TimeProvider instance, the value must be idempotent and remain unchanged. /// public virtual long TimestampFrequency { get => Stopwatch.Frequency; } diff --git a/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx b/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx index 48056b82ad12a..f161cea2908bf 100644 --- a/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx @@ -66,4 +66,7 @@ '{0}' must be less than or equal to '{1}'. + + The operation cannot performed when having TimeProvider.LocalTimeZone is returning null. + diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 16ad6d3e156b1..15f9097aa6358 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2984,6 +2984,9 @@ This operation is invalid on overlapping buffers. + + The operation cannot performed when having TimeProvider.LocalTimeZone is returning null. + Invoking default method with named arguments is not supported. diff --git a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs index f47ccf71aee49..3d77248f90836 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs @@ -1094,6 +1094,8 @@ private static string GetResourceString(ExceptionResource resource) return SR.ArgumentOutOfRange_NotGreaterThanBufferLength; case ExceptionResource.InvalidOperation_SpanOverlappedOperation: return SR.InvalidOperation_SpanOverlappedOperation; + case ExceptionResource.InvalidOperation_TimeProviderNullLocalTimeZone: + return SR.InvalidOperation_TimeProviderNullLocalTimeZone; default: Debug.Fail("The enum value is not defined, please check the ExceptionResource Enum."); return ""; @@ -1285,5 +1287,6 @@ internal enum ExceptionResource CancellationTokenSource_Disposed, Argument_AlignmentMustBePow2, InvalidOperation_SpanOverlappedOperation, + InvalidOperation_TimeProviderNullLocalTimeZone, } } From e9b6e699ce87da02f19fd0682423b76198a3e8f7 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Sat, 8 Apr 2023 13:38:59 -0700 Subject: [PATCH 3/4] Fix the exception thrown because of the timestamp frequency --- src/libraries/Common/src/System/TimeProvider.cs | 5 ++--- src/libraries/Common/tests/System/TimeProviderTests.cs | 4 ++-- .../Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx | 3 +++ .../System.Private.CoreLib/src/Resources/Strings.resx | 3 +++ .../System.Private.CoreLib/src/System/ThrowHelper.cs | 3 +++ 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/src/System/TimeProvider.cs b/src/libraries/Common/src/System/TimeProvider.cs index 1d3848900ca40..426a2f31451fe 100644 --- a/src/libraries/Common/src/System/TimeProvider.cs +++ b/src/libraries/Common/src/System/TimeProvider.cs @@ -104,10 +104,9 @@ public TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) if (timestampFrequency <= 0) { #if SYSTEM_PRIVATE_CORELIB - ArgumentOutOfRangeException.ThrowIfNegativeOrZero(TimestampFrequency); + ThrowHelper.ThrowInvalidOperationException(ExceptionResource.InvalidOperation_TimeProviderInvalidTimestampFrequency); #else - string propertyName = nameof(TimestampFrequency); - throw new ArgumentOutOfRangeException(propertyName, timestampFrequency, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeNonNegativeNonZero, nameof(TimestampFrequency))); + throw new InvalidOperationException(SR.InvalidOperation_TimeProviderInvalidTimestampFrequency); #endif // SYSTEM_PRIVATE_CORELIB } diff --git a/src/libraries/Common/tests/System/TimeProviderTests.cs b/src/libraries/Common/tests/System/TimeProviderTests.cs index 5c34bea8defd6..ded26cb024cb4 100644 --- a/src/libraries/Common/tests/System/TimeProviderTests.cs +++ b/src/libraries/Common/tests/System/TimeProviderTests.cs @@ -387,9 +387,9 @@ public static async void PeriodicTimerTests(TimeProvider provider) public static void NegativeTests() { FastClock clock = new FastClock(-1); // negative frequency - Assert.Throws(() => clock.GetElapsedTime(1, 2)); + Assert.Throws(() => clock.GetElapsedTime(1, 2)); clock = new FastClock(0); // zero frequency - Assert.Throws(() => clock.GetElapsedTime(1, 2)); + Assert.Throws(() => clock.GetElapsedTime(1, 2)); Assert.Throws(() => TimeProvider.System.CreateTimer(null, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan)); Assert.Throws(() => TimeProvider.System.CreateTimer(obj => { }, null, TimeSpan.FromMilliseconds(-2), Timeout.InfiniteTimeSpan)); diff --git a/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx b/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx index f161cea2908bf..a4b27feeda80e 100644 --- a/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx @@ -69,4 +69,7 @@ The operation cannot performed when having TimeProvider.LocalTimeZone is returning null. + + The operation cannot performed when having TimeProvider.TimestampFrequency has a zero or negative value. + diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 15f9097aa6358..6ad3365365441 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2987,6 +2987,9 @@ The operation cannot performed when having TimeProvider.LocalTimeZone is returning null. + + The operation cannot performed when having TimeProvider.TimestampFrequency has a zero or negative value. + Invoking default method with named arguments is not supported. diff --git a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs index 3d77248f90836..e8b9232e48901 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs @@ -1096,6 +1096,8 @@ private static string GetResourceString(ExceptionResource resource) return SR.InvalidOperation_SpanOverlappedOperation; case ExceptionResource.InvalidOperation_TimeProviderNullLocalTimeZone: return SR.InvalidOperation_TimeProviderNullLocalTimeZone; + case ExceptionResource.InvalidOperation_TimeProviderInvalidTimestampFrequency: + return SR.InvalidOperation_TimeProviderInvalidTimestampFrequency; default: Debug.Fail("The enum value is not defined, please check the ExceptionResource Enum."); return ""; @@ -1288,5 +1290,6 @@ internal enum ExceptionResource Argument_AlignmentMustBePow2, InvalidOperation_SpanOverlappedOperation, InvalidOperation_TimeProviderNullLocalTimeZone, + InvalidOperation_TimeProviderInvalidTimestampFrequency, } } From df6e180497a39a66adf7026d7398e7497524cfbc Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Sun, 9 Apr 2023 12:45:09 -0700 Subject: [PATCH 4/4] Fix exception message. --- .../Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx | 4 ++-- .../System.Private.CoreLib/src/Resources/Strings.resx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx b/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx index a4b27feeda80e..576c030f69228 100644 --- a/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Bcl.TimeProvider/src/Resources/Strings.resx @@ -67,9 +67,9 @@ '{0}' must be less than or equal to '{1}'. - The operation cannot performed when having TimeProvider.LocalTimeZone is returning null. + The operation cannot be performed when TimeProvider.LocalTimeZone is null. - The operation cannot performed when having TimeProvider.TimestampFrequency has a zero or negative value. + The operation cannot be performed when TimeProvider.TimestampFrequency is zero or negative. diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 6ad3365365441..7d1d0bf567f89 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2985,10 +2985,10 @@ This operation is invalid on overlapping buffers. - The operation cannot performed when having TimeProvider.LocalTimeZone is returning null. + The operation cannot be performed when TimeProvider.LocalTimeZone is null. - The operation cannot performed when having TimeProvider.TimestampFrequency has a zero or negative value. + The operation cannot be performed when TimeProvider.TimestampFrequency is zero or negative. Invoking default method with named arguments is not supported.