Extend allowed SemaphoreSlim wait TimeSpan values to match Timer and other APIs (#117398)

Author: eduardo-vp

src/libraries/System.Private.CoreLib/src/Resources/Strings.resx

@@ -3413,6 +3413,9 @@
   <data name="SemaphoreSlim_Wait_TimeoutWrong" xml:space="preserve">
     <value>The timeout must represent a value between -1 and Int32.MaxValue, inclusive.</value>
   </data>
+  <data name="SemaphoreSlim_Wait_TimeSpanTimeoutWrong" xml:space="preserve">
+    <value>The value needs to translate in milliseconds to -1 (signifying an infinite timeout), or be non-negative.</value>
+  </data>
   <data name="Serialization_BadParameterInfo" xml:space="preserve">
     <value>Non existent ParameterInfo. Position bigger than member's parameters length.</value>
   </data>

src/libraries/System.Private.CoreLib/src/System/Threading/ManualResetEventSlim.cs

@@ -508,7 +508,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
 
 
                 // We spin briefly before falling back to allocating and/or waiting on a true event.
-                uint startTime = 0;
+                long startTime = 0;
                 bool bNeedTimeoutAdjustment = false;
                 int realMillisecondsTimeout = millisecondsTimeout; // this will be adjusted if necessary.
 
@@ -520,7 +520,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
                     // period of time.  The timeout adjustments only take effect when and if we actually
                     // decide to block in the kernel below.
 
-                    startTime = TimeoutHelper.GetTime();
+                    startTime = Environment.TickCount64;
                     bNeedTimeoutAdjustment = true;
                 }
 
@@ -558,7 +558,8 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
                             // update timeout (delays in wait commencement are due to spinning and/or spurious wakeups from other waits being canceled)
                             if (bNeedTimeoutAdjustment)
                             {
-                                realMillisecondsTimeout = TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
+                                // TimeoutHelper.UpdateTimeOut returns a long but the value is capped as millisecondsTimeout is an int.
+                                realMillisecondsTimeout = (int)TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
                                 if (realMillisecondsTimeout <= 0)
                                     return false;
                             }

src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs

@@ -178,7 +178,7 @@ public void Wait()
             if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
 #endif
             // Call wait with infinite timeout
-            Wait(Timeout.Infinite, CancellationToken.None);
+            WaitCore(Timeout.Infinite, CancellationToken.None);
         }
 
         /// <summary>
@@ -198,7 +198,7 @@ public void Wait(CancellationToken cancellationToken)
             if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
 #endif
             // Call wait with infinite timeout
-            Wait(Timeout.Infinite, cancellationToken);
+            WaitCore(Timeout.Infinite, cancellationToken);
         }
 
         /// <summary>
@@ -211,8 +211,7 @@ public void Wait(CancellationToken cancellationToken)
         /// <returns>true if the current thread successfully entered the <see cref="SemaphoreSlim"/>;
         /// otherwise, false.</returns>
         /// <exception cref="ArgumentOutOfRangeException"><paramref name="timeout"/> is a negative
-        /// number other than -1 milliseconds, which represents an infinite time-out -or- timeout is greater
-        /// than <see cref="int.MaxValue"/>.</exception>
+        /// number other than -1 milliseconds, which represents an infinite time-out.</exception>
         [UnsupportedOSPlatform("browser")]
         public bool Wait(TimeSpan timeout)
         {
@@ -221,14 +220,14 @@ public bool Wait(TimeSpan timeout)
 #endif
             // Validate the timeout
             long totalMilliseconds = (long)timeout.TotalMilliseconds;
-            if (totalMilliseconds < -1 || totalMilliseconds > int.MaxValue)
+            if (totalMilliseconds < -1)
             {
                 throw new ArgumentOutOfRangeException(
-                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
+                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
             }
 
             // Call wait with the timeout milliseconds
-            return Wait((int)timeout.TotalMilliseconds, CancellationToken.None);
+            return WaitCore(totalMilliseconds, CancellationToken.None);
         }
 
         /// <summary>
@@ -244,8 +243,7 @@ public bool Wait(TimeSpan timeout)
         /// <returns>true if the current thread successfully entered the <see cref="SemaphoreSlim"/>;
         /// otherwise, false.</returns>
         /// <exception cref="ArgumentOutOfRangeException"><paramref name="timeout"/> is a negative
-        /// number other than -1 milliseconds, which represents an infinite time-out -or- timeout is greater
-        /// than <see cref="int.MaxValue"/>.</exception>
+        /// number other than -1 milliseconds, which represents an infinite time-out.</exception>
         /// <exception cref="OperationCanceledException"><paramref name="cancellationToken"/> was canceled.</exception>
         [UnsupportedOSPlatform("browser")]
         public bool Wait(TimeSpan timeout, CancellationToken cancellationToken)
@@ -255,14 +253,14 @@ public bool Wait(TimeSpan timeout, CancellationToken cancellationToken)
 #endif
             // Validate the timeout
             long totalMilliseconds = (long)timeout.TotalMilliseconds;
-            if (totalMilliseconds < -1 || totalMilliseconds > int.MaxValue)
+            if (totalMilliseconds < -1)
             {
                 throw new ArgumentOutOfRangeException(
-                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
+                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
             }
 
             // Call wait with the timeout milliseconds
-            return Wait((int)timeout.TotalMilliseconds, cancellationToken);
+            return WaitCore(totalMilliseconds, cancellationToken);
         }
 
         /// <summary>
@@ -281,7 +279,7 @@ public bool Wait(int millisecondsTimeout)
 #if TARGET_WASI
             if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
 #endif
-            return Wait(millisecondsTimeout, CancellationToken.None);
+            return WaitCore(millisecondsTimeout, CancellationToken.None);
         }
 
         /// <summary>
@@ -302,17 +300,33 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
 #if TARGET_WASI
             if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
 #endif
-            CheckDispose();
-#if FEATURE_WASM_MANAGED_THREADS
-            Thread.AssureBlockingPossible();
-#endif
 
             if (millisecondsTimeout < -1)
             {
                 throw new ArgumentOutOfRangeException(
                     nameof(millisecondsTimeout), millisecondsTimeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
             }
 
+            return WaitCore(millisecondsTimeout, cancellationToken);
+        }
+
+        /// <summary>
+        /// Blocks the current thread until it can enter the <see cref="SemaphoreSlim"/>,
+        /// using a 32-bit unsigned integer to measure the time interval,
+        /// while observing a <see cref="CancellationToken"/>.
+        /// </summary>
+        /// <param name="millisecondsTimeout">The number of milliseconds to wait, or <see cref="Timeout.UnsignedInfinite"/> to
+        /// wait indefinitely.</param>
+        /// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe.</param>
+        /// <returns>true if the current thread successfully entered the <see cref="SemaphoreSlim"/>; otherwise, false.</returns>
+        /// <exception cref="OperationCanceledException"><paramref name="cancellationToken"/> was canceled.</exception>
+        [UnsupportedOSPlatform("browser")]
+        private bool WaitCore(long millisecondsTimeout, CancellationToken cancellationToken)
+        {
+            CheckDispose();
+#if FEATURE_WASM_MANAGED_THREADS
+            Thread.AssureBlockingPossible();
+#endif
             cancellationToken.ThrowIfCancellationRequested();
 
             // Perf: Check the stack timeout parameter before checking the volatile count
@@ -322,10 +336,10 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
                 return false;
             }
 
-            uint startTime = 0;
+            long startTime = 0;
             if (millisecondsTimeout != Timeout.Infinite && millisecondsTimeout > 0)
             {
-                startTime = TimeoutHelper.GetTime();
+                startTime = Environment.TickCount64;
             }
 
             bool waitSuccessful = false;
@@ -368,7 +382,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
                 if (m_asyncHead is not null)
                 {
                     Debug.Assert(m_asyncTail is not null, "tail should not be null if head isn't");
-                    asyncWaitTask = WaitAsync(millisecondsTimeout, cancellationToken);
+                    asyncWaitTask = WaitAsyncCore(millisecondsTimeout, cancellationToken);
                 }
                 // There are no async waiters, so we can proceed with normal synchronous waiting.
                 else
@@ -449,30 +463,45 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
         /// <param name="cancellationToken">The CancellationToken to observe.</param>
         /// <returns>true if the monitor received a signal, false if the timeout expired</returns>
         [UnsupportedOSPlatform("browser")]
-        private bool WaitUntilCountOrTimeout(int millisecondsTimeout, uint startTime, CancellationToken cancellationToken)
+        private bool WaitUntilCountOrTimeout(long millisecondsTimeout, long startTime, CancellationToken cancellationToken)
         {
 #if TARGET_WASI
             if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
 #endif
-            int remainingWaitMilliseconds = Timeout.Infinite;
+            int monitorWaitMilliseconds = Timeout.Infinite;
 
             // Wait on the monitor as long as the count is zero
             while (m_currentCount == 0)
             {
                 // If cancelled, we throw. Trying to wait could lead to deadlock.
                 cancellationToken.ThrowIfCancellationRequested();
 
+                // Since Monitor.Wait will handle the actual wait and it accepts an int timeout,
+                // we may need to cap the timeout to int.MaxValue.
+                bool timeoutIsCapped = false;
                 if (millisecondsTimeout != Timeout.Infinite)
                 {
-                    remainingWaitMilliseconds = TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
+                    long remainingWaitMilliseconds = TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
                     if (remainingWaitMilliseconds <= 0)
                     {
                         // The thread has expires its timeout
                         return false;
                     }
+                    if (remainingWaitMilliseconds <= int.MaxValue)
+                    {
+                        monitorWaitMilliseconds = (int)remainingWaitMilliseconds;
+                    }
+                    else
+                    {
+                        timeoutIsCapped = true;
+                        monitorWaitMilliseconds = int.MaxValue;
+                    }
                 }
-                // ** the actual wait **
-                bool waitSuccessful = Monitor.Wait(m_lockObjAndDisposed, remainingWaitMilliseconds);
+
+
+                // The actual wait. If the timeout was capped and waitSuccessful is false, it doesn't imply
+                // a timeout, we are just limited by Monitor.Wait's maximum timeout value.
+                bool waitSuccessful = Monitor.Wait(m_lockObjAndDisposed, monitorWaitMilliseconds);
 
                 // This waiter has woken up and this needs to be reflected in the count of waiters pulsed to wake. Since we
                 // don't have thread-specific pulse state, there is not enough information to tell whether this thread woke up
@@ -485,7 +514,7 @@ private bool WaitUntilCountOrTimeout(int millisecondsTimeout, uint startTime, Ca
                     --m_countOfWaitersPulsedToWake;
                 }
 
-                if (!waitSuccessful)
+                if (!timeoutIsCapped && !waitSuccessful)
                 {
                     return false;
                 }
@@ -500,7 +529,7 @@ private bool WaitUntilCountOrTimeout(int millisecondsTimeout, uint startTime, Ca
         /// <returns>A task that will complete when the semaphore has been entered.</returns>
         public Task WaitAsync()
         {
-            return WaitAsync(Timeout.Infinite, default);
+            return WaitAsyncCore(Timeout.Infinite, default);
         }
 
         /// <summary>
@@ -516,7 +545,7 @@ public Task WaitAsync()
         /// </exception>
         public Task WaitAsync(CancellationToken cancellationToken)
         {
-            return WaitAsync(Timeout.Infinite, cancellationToken);
+            return WaitAsyncCore(Timeout.Infinite, cancellationToken);
         }
 
         /// <summary>
@@ -537,7 +566,7 @@ public Task WaitAsync(CancellationToken cancellationToken)
         /// </exception>
         public Task<bool> WaitAsync(int millisecondsTimeout)
         {
-            return WaitAsync(millisecondsTimeout, default);
+            return WaitAsyncCore(millisecondsTimeout, default);
         }
 
         /// <summary>
@@ -562,7 +591,16 @@ public Task<bool> WaitAsync(int millisecondsTimeout)
         /// </exception>
         public Task<bool> WaitAsync(TimeSpan timeout)
         {
-            return WaitAsync(timeout, default);
+            // Validate the timeout
+            long totalMilliseconds = (long)timeout.TotalMilliseconds;
+            if (totalMilliseconds < -1)
+            {
+                throw new ArgumentOutOfRangeException(
+                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
+            }
+
+            // Call wait with the timeout milliseconds
+            return WaitAsyncCore(totalMilliseconds, default);
         }
 
         /// <summary>
@@ -588,14 +626,14 @@ public Task<bool> WaitAsync(TimeSpan timeout, CancellationToken cancellationToke
         {
             // Validate the timeout
             long totalMilliseconds = (long)timeout.TotalMilliseconds;
-            if (totalMilliseconds < -1 || totalMilliseconds > int.MaxValue)
+            if (totalMilliseconds < -1)
             {
                 throw new ArgumentOutOfRangeException(
-                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
+                    nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
             }
 
             // Call wait with the timeout milliseconds
-            return WaitAsync((int)timeout.TotalMilliseconds, cancellationToken);
+            return WaitAsyncCore(totalMilliseconds, cancellationToken);
         }
 
         /// <summary>
@@ -618,14 +656,34 @@ public Task<bool> WaitAsync(TimeSpan timeout, CancellationToken cancellationToke
         /// </exception>
         public Task<bool> WaitAsync(int millisecondsTimeout, CancellationToken cancellationToken)
         {
-            CheckDispose();
-
             if (millisecondsTimeout < -1)
             {
                 throw new ArgumentOutOfRangeException(
                     nameof(millisecondsTimeout), millisecondsTimeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
             }
 
+            return WaitAsyncCore(millisecondsTimeout, cancellationToken);
+        }
+
+        /// <summary>
+        /// Asynchronously waits to enter the <see cref="SemaphoreSlim"/>,
+        /// using a 32-bit unsigned integer to measure the time interval,
+        /// while observing a <see cref="CancellationToken"/>.
+        /// </summary>
+        /// <param name="millisecondsTimeout">
+        /// The number of milliseconds to wait, or <see cref="Timeout.UnsignedInfinite"/> to wait indefinitely.
+        /// </param>
+        /// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe.</param>
+        /// <returns>
+        /// A task that will complete with a result of true if the current thread successfully entered
+        /// the <see cref="SemaphoreSlim"/>, otherwise with a result of false.
+        /// </returns>
+        /// <exception cref="ObjectDisposedException">The current instance has already been
+        /// disposed.</exception>
+        private Task<bool> WaitAsyncCore(long millisecondsTimeout, CancellationToken cancellationToken)
+        {
+            CheckDispose();
+
             // Bail early for cancellation
             if (cancellationToken.IsCancellationRequested)
                 return Task.FromCanceled<bool>(cancellationToken);
@@ -716,12 +774,14 @@ private bool RemoveAsyncWaiter(TaskNode task)
         /// <param name="millisecondsTimeout">The timeout.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>The task to return to the caller.</returns>
-        private async Task<bool> WaitUntilCountOrTimeoutAsync(TaskNode asyncWaiter, int millisecondsTimeout, CancellationToken cancellationToken)
+        private async Task<bool> WaitUntilCountOrTimeoutAsync(TaskNode asyncWaiter, long millisecondsTimeout, CancellationToken cancellationToken)
         {
             Debug.Assert(asyncWaiter is not null, "Waiter should have been constructed");
             Debug.Assert(Monitor.IsEntered(m_lockObjAndDisposed), "Requires the lock be held");
 
-            await ((Task)asyncWaiter.WaitAsync(TimeSpan.FromMilliseconds(millisecondsTimeout), cancellationToken)).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
+            await ((Task)asyncWaiter.WaitAsync(
+                TimeSpan.FromMilliseconds(millisecondsTimeout),
+                cancellationToken)).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
 
             if (cancellationToken.IsCancellationRequested)
             {

src/libraries/System.Private.CoreLib/src/System/Threading/SpinLock.cs

@@ -287,10 +287,10 @@ private void ContinueTryEnter(int millisecondsTimeout, ref bool lockTaken)
                     nameof(millisecondsTimeout), millisecondsTimeout, SR.SpinLock_TryEnter_ArgumentOutOfRange);
             }
 
-            uint startTime = 0;
+            long startTime = 0;
             if (millisecondsTimeout != Timeout.Infinite && millisecondsTimeout != 0)
             {
-                startTime = TimeoutHelper.GetTime();
+                startTime = Environment.TickCount64;
             }
 
             if (IsThreadOwnerTrackingEnabled)
@@ -404,7 +404,7 @@ private void DecrementWaiters()
         /// <summary>
         /// ContinueTryEnter for the thread tracking mode enabled
         /// </summary>
-        private void ContinueTryEnterWithThreadTracking(int millisecondsTimeout, uint startTime, ref bool lockTaken)
+        private void ContinueTryEnterWithThreadTracking(int millisecondsTimeout, long startTime, ref bool lockTaken)
         {
             Debug.Assert(IsThreadOwnerTrackingEnabled);