Skip to content

Commit f25c3c9

Browse files
committed
Address PR feedback
1 parent 68040cb commit f25c3c9

File tree

6 files changed

+36
-77
lines changed

6 files changed

+36
-77
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3411,7 +3411,7 @@
34113411
<value>The timeout must represent a value between -1 and Int32.MaxValue, inclusive.</value>
34123412
</data>
34133413
<data name="SemaphoreSlim_Wait_TimeSpanTimeoutWrong" xml:space="preserve">
3414-
<value>The timeout must represent a value between -1 and the maximum allowed timer duration.</value>
3414+
<value>The timeout must be greater than or equal to -1.</value>
34153415
</data>
34163416
<data name="Serialization_BadParameterInfo" xml:space="preserve">
34173417
<value>Non existent ParameterInfo. Position bigger than member's parameters length.</value>

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
520520
// period of time. The timeout adjustments only take effect when and if we actually
521521
// decide to block in the kernel below.
522522

523-
startTime = TimeoutHelper.GetTime();
523+
startTime = (uint)Environment.TickCount;
524524
bNeedTimeoutAdjustment = true;
525525
}
526526

@@ -558,7 +558,8 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
558558
// update timeout (delays in wait commencement are due to spinning and/or spurious wakeups from other waits being canceled)
559559
if (bNeedTimeoutAdjustment)
560560
{
561-
realMillisecondsTimeout = TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
561+
// TimeoutHelper.UpdateTimeOut returns a long but the value is capped as millisecondsTimeout is an int.
562+
realMillisecondsTimeout = (int)TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
562563
if (realMillisecondsTimeout <= 0)
563564
return false;
564565
}

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public void Wait()
178178
if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
179179
#endif
180180
// Call wait with infinite timeout
181-
Wait(Timeout.UnsignedInfinite, CancellationToken.None);
181+
Wait(Timeout.Infinite, CancellationToken.None);
182182
}
183183

184184
/// <summary>
@@ -198,7 +198,7 @@ public void Wait(CancellationToken cancellationToken)
198198
if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
199199
#endif
200200
// Call wait with infinite timeout
201-
Wait(Timeout.UnsignedInfinite, cancellationToken);
201+
Wait(Timeout.Infinite, cancellationToken);
202202
}
203203

204204
/// <summary>
@@ -221,14 +221,14 @@ public bool Wait(TimeSpan timeout)
221221
#endif
222222
// Validate the timeout
223223
long totalMilliseconds = (long)timeout.TotalMilliseconds;
224-
if (totalMilliseconds < -1 || totalMilliseconds > Timer.MaxSupportedTimeout)
224+
if (totalMilliseconds < -1)
225225
{
226226
throw new ArgumentOutOfRangeException(
227227
nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
228228
}
229229

230230
// Call wait with the timeout milliseconds
231-
return Wait(totalMilliseconds == Timeout.Infinite ? Timeout.UnsignedInfinite : (uint)totalMilliseconds, CancellationToken.None);
231+
return Wait(totalMilliseconds, CancellationToken.None);
232232
}
233233

234234
/// <summary>
@@ -255,14 +255,14 @@ public bool Wait(TimeSpan timeout, CancellationToken cancellationToken)
255255
#endif
256256
// Validate the timeout
257257
long totalMilliseconds = (long)timeout.TotalMilliseconds;
258-
if (totalMilliseconds < -1 || totalMilliseconds > Timer.MaxSupportedTimeout)
258+
if (totalMilliseconds < -1)
259259
{
260260
throw new ArgumentOutOfRangeException(
261261
nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
262262
}
263263

264264
// Call wait with the timeout milliseconds
265-
return Wait(totalMilliseconds == Timeout.Infinite ? Timeout.UnsignedInfinite : (uint)totalMilliseconds, cancellationToken);
265+
return Wait(totalMilliseconds, cancellationToken);
266266
}
267267

268268
/// <summary>
@@ -309,7 +309,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
309309
nameof(millisecondsTimeout), millisecondsTimeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
310310
}
311311

312-
return Wait(millisecondsTimeout == Timeout.Infinite ? Timeout.UnsignedInfinite : (uint)millisecondsTimeout, cancellationToken);
312+
return Wait(millisecondsTimeout, cancellationToken);
313313
}
314314

315315
/// <summary>
@@ -323,7 +323,7 @@ public bool Wait(int millisecondsTimeout, CancellationToken cancellationToken)
323323
/// <returns>true if the current thread successfully entered the <see cref="SemaphoreSlim"/>; otherwise, false.</returns>
324324
/// <exception cref="OperationCanceledException"><paramref name="cancellationToken"/> was canceled.</exception>
325325
[UnsupportedOSPlatform("browser")]
326-
private bool Wait(uint millisecondsTimeout, CancellationToken cancellationToken)
326+
private bool Wait(long millisecondsTimeout, CancellationToken cancellationToken)
327327
{
328328
CheckDispose();
329329
#if FEATURE_WASM_MANAGED_THREADS
@@ -339,9 +339,9 @@ private bool Wait(uint millisecondsTimeout, CancellationToken cancellationToken)
339339
}
340340

341341
long startTime = 0;
342-
if (millisecondsTimeout != Timeout.UnsignedInfinite && millisecondsTimeout > 0)
342+
if (millisecondsTimeout != Timeout.Infinite && millisecondsTimeout > 0)
343343
{
344-
startTime = TimeoutHelper.GetTime64();
344+
startTime = Environment.TickCount64;
345345
}
346346

347347
bool waitSuccessful = false;
@@ -465,7 +465,7 @@ private bool Wait(uint millisecondsTimeout, CancellationToken cancellationToken)
465465
/// <param name="cancellationToken">The CancellationToken to observe.</param>
466466
/// <returns>true if the monitor received a signal, false if the timeout expired</returns>
467467
[UnsupportedOSPlatform("browser")]
468-
private bool WaitUntilCountOrTimeout(uint millisecondsTimeout, long startTime, CancellationToken cancellationToken)
468+
private bool WaitUntilCountOrTimeout(long millisecondsTimeout, long startTime, CancellationToken cancellationToken)
469469
{
470470
#if TARGET_WASI
471471
if (OperatingSystem.IsWasi()) throw new PlatformNotSupportedException(); // TODO remove with https://github.com/dotnet/runtime/pull/107185
@@ -481,7 +481,7 @@ private bool WaitUntilCountOrTimeout(uint millisecondsTimeout, long startTime, C
481481
// Since Monitor.Wait will handle the actual wait and it accepts an int timeout,
482482
// we may need to cap the timeout to int.MaxValue.
483483
bool timeoutIsCapped = false;
484-
if (millisecondsTimeout != Timeout.UnsignedInfinite)
484+
if (millisecondsTimeout != Timeout.Infinite)
485485
{
486486
long remainingWaitMilliseconds = TimeoutHelper.UpdateTimeOut(startTime, millisecondsTimeout);
487487
if (remainingWaitMilliseconds <= 0)
@@ -531,7 +531,7 @@ private bool WaitUntilCountOrTimeout(uint millisecondsTimeout, long startTime, C
531531
/// <returns>A task that will complete when the semaphore has been entered.</returns>
532532
public Task WaitAsync()
533533
{
534-
return WaitAsync(Timeout.UnsignedInfinite, default);
534+
return WaitAsync(Timeout.Infinite, default);
535535
}
536536

537537
/// <summary>
@@ -547,7 +547,7 @@ public Task WaitAsync()
547547
/// </exception>
548548
public Task WaitAsync(CancellationToken cancellationToken)
549549
{
550-
return WaitAsync(Timeout.UnsignedInfinite, cancellationToken);
550+
return WaitAsync(Timeout.Infinite, cancellationToken);
551551
}
552552

553553
/// <summary>
@@ -619,14 +619,14 @@ public Task<bool> WaitAsync(TimeSpan timeout, CancellationToken cancellationToke
619619
{
620620
// Validate the timeout
621621
long totalMilliseconds = (long)timeout.TotalMilliseconds;
622-
if (totalMilliseconds < -1 || totalMilliseconds > Timer.MaxSupportedTimeout)
622+
if (totalMilliseconds < -1)
623623
{
624624
throw new ArgumentOutOfRangeException(
625625
nameof(timeout), timeout, SR.SemaphoreSlim_Wait_TimeSpanTimeoutWrong);
626626
}
627627

628628
// Call wait with the timeout milliseconds
629-
return WaitAsync(totalMilliseconds == Timeout.Infinite ? Timeout.UnsignedInfinite : (uint)totalMilliseconds, cancellationToken);
629+
return WaitAsync(totalMilliseconds, cancellationToken);
630630
}
631631

632632
/// <summary>
@@ -655,7 +655,7 @@ public Task<bool> WaitAsync(int millisecondsTimeout, CancellationToken cancellat
655655
nameof(millisecondsTimeout), millisecondsTimeout, SR.SemaphoreSlim_Wait_TimeoutWrong);
656656
}
657657

658-
return WaitAsync(millisecondsTimeout == Timeout.Infinite ? Timeout.UnsignedInfinite : (uint)millisecondsTimeout, cancellationToken);
658+
return WaitAsync(millisecondsTimeout, cancellationToken);
659659
}
660660

661661
/// <summary>
@@ -673,7 +673,7 @@ public Task<bool> WaitAsync(int millisecondsTimeout, CancellationToken cancellat
673673
/// </returns>
674674
/// <exception cref="ObjectDisposedException">The current instance has already been
675675
/// disposed.</exception>
676-
private Task<bool> WaitAsync(uint millisecondsTimeout, CancellationToken cancellationToken)
676+
private Task<bool> WaitAsync(long millisecondsTimeout, CancellationToken cancellationToken)
677677
{
678678
CheckDispose();
679679

@@ -702,7 +702,7 @@ private Task<bool> WaitAsync(uint millisecondsTimeout, CancellationToken cancell
702702
{
703703
Debug.Assert(m_currentCount == 0, "m_currentCount should never be negative");
704704
TaskNode asyncWaiter = CreateAndAddAsyncWaiter();
705-
return (millisecondsTimeout == Timeout.UnsignedInfinite && !cancellationToken.CanBeCanceled) ?
705+
return (millisecondsTimeout == Timeout.Infinite && !cancellationToken.CanBeCanceled) ?
706706
asyncWaiter :
707707
WaitUntilCountOrTimeoutAsync(asyncWaiter, millisecondsTimeout, cancellationToken);
708708
}
@@ -767,13 +767,13 @@ private bool RemoveAsyncWaiter(TaskNode task)
767767
/// <param name="millisecondsTimeout">The timeout.</param>
768768
/// <param name="cancellationToken">The cancellation token.</param>
769769
/// <returns>The task to return to the caller.</returns>
770-
private async Task<bool> WaitUntilCountOrTimeoutAsync(TaskNode asyncWaiter, uint millisecondsTimeout, CancellationToken cancellationToken)
770+
private async Task<bool> WaitUntilCountOrTimeoutAsync(TaskNode asyncWaiter, long millisecondsTimeout, CancellationToken cancellationToken)
771771
{
772772
Debug.Assert(asyncWaiter is not null, "Waiter should have been constructed");
773773
Debug.Assert(Monitor.IsEntered(m_lockObjAndDisposed), "Requires the lock be held");
774774

775775
await ((Task)asyncWaiter.WaitAsync(
776-
TimeSpan.FromMilliseconds(millisecondsTimeout == Timeout.UnsignedInfinite ? (long)Timeout.Infinite : (long)millisecondsTimeout),
776+
TimeSpan.FromMilliseconds(millisecondsTimeout),
777777
cancellationToken)).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
778778

779779
if (cancellationToken.IsCancellationRequested)

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,10 @@ private void ContinueTryEnter(int millisecondsTimeout, ref bool lockTaken)
287287
nameof(millisecondsTimeout), millisecondsTimeout, SR.SpinLock_TryEnter_ArgumentOutOfRange);
288288
}
289289

290-
uint startTime = 0;
290+
int startTime = 0;
291291
if (millisecondsTimeout != Timeout.Infinite && millisecondsTimeout != 0)
292292
{
293-
startTime = TimeoutHelper.GetTime();
293+
startTime = Environment.TickCount;
294294
}
295295

296296
if (IsThreadOwnerTrackingEnabled)
@@ -404,7 +404,7 @@ private void DecrementWaiters()
404404
/// <summary>
405405
/// ContinueTryEnter for the thread tracking mode enabled
406406
/// </summary>
407-
private void ContinueTryEnterWithThreadTracking(int millisecondsTimeout, uint startTime, ref bool lockTaken)
407+
private void ContinueTryEnterWithThreadTracking(int millisecondsTimeout, int startTime, ref bool lockTaken)
408408
{
409409
Debug.Assert(IsThreadOwnerTrackingEnabled);
410410

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,10 @@ public static bool SpinUntil(Func<bool> condition, int millisecondsTimeout)
312312
nameof(millisecondsTimeout), millisecondsTimeout, SR.SpinWait_SpinUntil_TimeoutWrong);
313313
}
314314
ArgumentNullException.ThrowIfNull(condition);
315-
uint startTime = 0;
315+
int startTime = 0;
316316
if (millisecondsTimeout != 0 && millisecondsTimeout != Timeout.Infinite)
317317
{
318-
startTime = TimeoutHelper.GetTime();
318+
startTime = Environment.TickCount;
319319
}
320320
SpinWait spinner = default;
321321
while (!condition())
@@ -329,7 +329,7 @@ public static bool SpinUntil(Func<bool> condition, int millisecondsTimeout)
329329

330330
if (millisecondsTimeout != Timeout.Infinite && spinner.NextSpinWillYield)
331331
{
332-
if (millisecondsTimeout <= (TimeoutHelper.GetTime() - startTime))
332+
if (millisecondsTimeout <= (uint)(Environment.TickCount - startTime))
333333
{
334334
return false;
335335
}

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

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,77 +6,35 @@
66
namespace System.Threading
77
{
88
/// <summary>
9-
/// A helper class to capture a start time using <see cref="Environment.TickCount"/> as a time in milliseconds.
10-
/// Also updates a given timeout by subtracting the current time from the start time.
9+
/// A helper class to update a given timeout by subtracting the current time from the start time.
1110
/// </summary>
1211
internal static class TimeoutHelper
1312
{
14-
/// <summary>
15-
/// Returns <see cref="Environment.TickCount"/> as a start time in milliseconds as a <see cref="uint"/>.
16-
/// <see cref="Environment.TickCount"/> rolls over from positive to negative every ~25 days, then ~25 days to back to positive again.
17-
/// <see cref="uint"/> is used to ignore the sign and double the range to 50 days.
18-
/// </summary>
19-
public static uint GetTime()
20-
{
21-
return (uint)Environment.TickCount;
22-
}
23-
24-
/// <summary>
25-
/// Returns <see cref="Environment.TickCount64"/> as a start time in milliseconds.
26-
/// </summary>
27-
public static long GetTime64()
28-
{
29-
return Environment.TickCount64;
30-
}
31-
3213
/// <summary>
3314
/// Helper function to measure and update the elapsed time
3415
/// </summary>
3516
/// <param name="startTime"> The first time (in milliseconds) observed when the wait started</param>
3617
/// <param name="originalWaitMillisecondsTimeout">The original wait timeout in milliseconds</param>
3718
/// <returns>The new wait time in milliseconds</returns>
38-
public static int UpdateTimeOut(uint startTime, int originalWaitMillisecondsTimeout)
19+
public static long UpdateTimeOut(long startTime, long originalWaitMillisecondsTimeout)
3920
{
4021
// The function must be called in case the time out is not infinite
4122
Debug.Assert(originalWaitMillisecondsTimeout != Timeout.Infinite);
4223

43-
uint elapsedMilliseconds = (GetTime() - startTime);
24+
ulong elapsedMilliseconds = (ulong)(Environment.TickCount64 - startTime);
4425

45-
// Check the elapsed milliseconds is greater than max int because this property is uint
46-
if (elapsedMilliseconds > int.MaxValue)
26+
if (elapsedMilliseconds > long.MaxValue)
4727
{
4828
return 0;
4929
}
5030

51-
// Subtract the elapsed time from the current wait time
52-
int currentWaitTimeout = originalWaitMillisecondsTimeout - (int)elapsedMilliseconds;
31+
long currentWaitTimeout = originalWaitMillisecondsTimeout - (long)elapsedMilliseconds;
5332
if (currentWaitTimeout <= 0)
5433
{
5534
return 0;
5635
}
5736

5837
return currentWaitTimeout;
5938
}
60-
61-
/// <summary>
62-
/// Helper function to measure and update the elapsed time
63-
/// </summary>
64-
/// <param name="startTime"> The first time (in milliseconds) observed when the wait started</param>
65-
/// <param name="originalWaitMillisecondsTimeout">The original wait timeout in milliseconds</param>
66-
/// <returns>The new wait time in milliseconds</returns>
67-
public static long UpdateTimeOut(long startTime, uint originalWaitMillisecondsTimeout)
68-
{
69-
// The function must be called in case the time out is not infinite
70-
Debug.Assert(originalWaitMillisecondsTimeout != Timeout.UnsignedInfinite);
71-
72-
long elapsedMilliseconds = GetTime64() - startTime;
73-
74-
if (originalWaitMillisecondsTimeout <= elapsedMilliseconds)
75-
{
76-
return 0;
77-
}
78-
79-
return originalWaitMillisecondsTimeout - elapsedMilliseconds;
80-
}
8139
}
8240
}

0 commit comments

Comments
 (0)