mirror of
https://github.com/gui-cs/Terminal.Gui.git
synced 2025-12-26 15:57:56 +01:00
Implement Stopwatch-based timing in TimedEvents to fix x64 race condition
Co-authored-by: tig <585482+tig@users.noreply.github.com>
This commit is contained in:
@@ -1,11 +1,13 @@
|
||||
#nullable enable
|
||||
using System.Diagnostics;
|
||||
|
||||
namespace Terminal.Gui.App;
|
||||
|
||||
/// <summary>
|
||||
/// Manages scheduled timeouts (timed callbacks) for the application.
|
||||
/// <para>
|
||||
/// Allows scheduling of callbacks to be invoked after a specified delay, with optional repetition.
|
||||
/// Timeouts are stored in a sorted list by their scheduled execution time (UTC ticks).
|
||||
/// Timeouts are stored in a sorted list by their scheduled execution time (high-resolution ticks).
|
||||
/// Thread-safe for concurrent access.
|
||||
/// </para>
|
||||
/// <para>
|
||||
@@ -26,6 +28,10 @@ namespace Terminal.Gui.App;
|
||||
/// </list>
|
||||
/// </para>
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Uses <see cref="Stopwatch.GetTimestamp"/> for high-resolution timing instead of <see cref="DateTime.UtcNow"/>
|
||||
/// to provide microsecond-level precision and eliminate race conditions from timer resolution issues.
|
||||
/// </remarks>
|
||||
public class TimedEvents : ITimedEvents
|
||||
{
|
||||
internal SortedList<long, Timeout> _timeouts = new ();
|
||||
@@ -40,6 +46,18 @@ public class TimedEvents : ITimedEvents
|
||||
/// <inheritdoc/>
|
||||
public event EventHandler<TimeoutEventArgs>? Added;
|
||||
|
||||
/// <summary>
|
||||
/// Gets the current high-resolution timestamp in TimeSpan ticks.
|
||||
/// Uses <see cref="Stopwatch.GetTimestamp"/> for microsecond-level precision.
|
||||
/// </summary>
|
||||
/// <returns>Current timestamp in TimeSpan ticks (100-nanosecond units).</returns>
|
||||
private static long GetTimestampTicks ()
|
||||
{
|
||||
// Convert Stopwatch ticks to TimeSpan ticks (100-nanosecond units)
|
||||
// Stopwatch.Frequency gives ticks per second, so we need to scale appropriately
|
||||
return Stopwatch.GetTimestamp () * TimeSpan.TicksPerSecond / Stopwatch.Frequency;
|
||||
}
|
||||
|
||||
/// <inheritdoc/>
|
||||
public void RunTimers ()
|
||||
{
|
||||
@@ -92,7 +110,7 @@ public class TimedEvents : ITimedEvents
|
||||
/// <inheritdoc/>
|
||||
public bool CheckTimers (out int waitTimeout)
|
||||
{
|
||||
long now = DateTime.UtcNow.Ticks;
|
||||
long now = GetTimestampTicks ();
|
||||
|
||||
waitTimeout = 0;
|
||||
|
||||
@@ -125,7 +143,7 @@ public class TimedEvents : ITimedEvents
|
||||
{
|
||||
lock (_timeoutsLockToken)
|
||||
{
|
||||
long k = (DateTime.UtcNow + time).Ticks;
|
||||
long k = GetTimestampTicks () + time.Ticks;
|
||||
|
||||
// if user wants to run as soon as possible set timer such that it expires right away (no race conditions)
|
||||
if (time == TimeSpan.Zero)
|
||||
@@ -159,7 +177,7 @@ public class TimedEvents : ITimedEvents
|
||||
|
||||
private void RunTimersImpl ()
|
||||
{
|
||||
long now = DateTime.UtcNow.Ticks;
|
||||
long now = GetTimestampTicks ();
|
||||
SortedList<long, Timeout> copy;
|
||||
|
||||
// lock prevents new timeouts being added
|
||||
|
||||
@@ -243,7 +243,8 @@ public class MainLoopTests
|
||||
var ml = new MainLoop (new FakeMainLoop ());
|
||||
var ms = 100;
|
||||
|
||||
long originTicks = DateTime.UtcNow.Ticks;
|
||||
// Use Stopwatch ticks since TimedEvents now uses Stopwatch.GetTimestamp internally
|
||||
long originTicks = Stopwatch.GetTimestamp () * TimeSpan.TicksPerSecond / Stopwatch.Frequency;
|
||||
|
||||
var callbackCount = 0;
|
||||
|
||||
|
||||
140
Tests/UnitTests/Application/TimedEventsTests.cs
Normal file
140
Tests/UnitTests/Application/TimedEventsTests.cs
Normal file
@@ -0,0 +1,140 @@
|
||||
using System.Diagnostics;
|
||||
|
||||
namespace UnitTests.ApplicationTests;
|
||||
|
||||
/// <summary>
|
||||
/// Tests for TimedEvents class, focusing on high-resolution timing with Stopwatch.
|
||||
/// </summary>
|
||||
public class TimedEventsTests
|
||||
{
|
||||
[Fact]
|
||||
public void HighFrequency_Concurrent_Invocations_No_Lost_Timeouts ()
|
||||
{
|
||||
var timedEvents = new Terminal.Gui.App.TimedEvents ();
|
||||
var counter = 0;
|
||||
var expected = 1000;
|
||||
var completed = new ManualResetEventSlim (false);
|
||||
|
||||
// Add many timeouts with TimeSpan.Zero concurrently
|
||||
Parallel.For (0, expected, i =>
|
||||
{
|
||||
timedEvents.Add (TimeSpan.Zero, () =>
|
||||
{
|
||||
var current = Interlocked.Increment (ref counter);
|
||||
if (current == expected)
|
||||
{
|
||||
completed.Set ();
|
||||
}
|
||||
return false; // One-shot
|
||||
});
|
||||
});
|
||||
|
||||
// Run timers multiple times to ensure all are processed
|
||||
for (int i = 0; i < 10; i++)
|
||||
{
|
||||
timedEvents.RunTimers ();
|
||||
if (completed.IsSet)
|
||||
{
|
||||
break;
|
||||
}
|
||||
Thread.Sleep (10);
|
||||
}
|
||||
|
||||
Assert.Equal (expected, counter);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetTimestampTicks_Provides_High_Resolution ()
|
||||
{
|
||||
var timedEvents = new Terminal.Gui.App.TimedEvents ();
|
||||
|
||||
// Add multiple timeouts with TimeSpan.Zero rapidly
|
||||
var timestamps = new List<long> ();
|
||||
|
||||
for (int i = 0; i < 100; i++)
|
||||
{
|
||||
long capturedTimestamp = 0;
|
||||
timedEvents.Added += (s, e) =>
|
||||
{
|
||||
capturedTimestamp = e.Ticks;
|
||||
};
|
||||
|
||||
timedEvents.Add (TimeSpan.Zero, () => false);
|
||||
|
||||
if (capturedTimestamp > 0)
|
||||
{
|
||||
timestamps.Add (capturedTimestamp);
|
||||
}
|
||||
}
|
||||
|
||||
// Verify that we got unique timestamps (or very close)
|
||||
// With Stopwatch, we should have much better resolution than DateTime.UtcNow
|
||||
var uniqueTimestamps = timestamps.Distinct ().Count ();
|
||||
|
||||
// We should have mostly unique timestamps
|
||||
// Allow some duplicates due to extreme speed, but should be > 50% unique
|
||||
Assert.True (uniqueTimestamps > timestamps.Count / 2,
|
||||
$"Expected more unique timestamps. Got {uniqueTimestamps} unique out of {timestamps.Count} total");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TimeSpan_Zero_Executes_Immediately ()
|
||||
{
|
||||
var timedEvents = new Terminal.Gui.App.TimedEvents ();
|
||||
var executed = false;
|
||||
|
||||
timedEvents.Add (TimeSpan.Zero, () =>
|
||||
{
|
||||
executed = true;
|
||||
return false;
|
||||
});
|
||||
|
||||
// Should execute on first RunTimers call
|
||||
timedEvents.RunTimers ();
|
||||
|
||||
Assert.True (executed);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Multiple_TimeSpan_Zero_Timeouts_All_Execute ()
|
||||
{
|
||||
var timedEvents = new Terminal.Gui.App.TimedEvents ();
|
||||
var executeCount = 0;
|
||||
var expected = 100;
|
||||
|
||||
for (int i = 0; i < expected; i++)
|
||||
{
|
||||
timedEvents.Add (TimeSpan.Zero, () =>
|
||||
{
|
||||
Interlocked.Increment (ref executeCount);
|
||||
return false;
|
||||
});
|
||||
}
|
||||
|
||||
// Run timers once
|
||||
timedEvents.RunTimers ();
|
||||
|
||||
Assert.Equal (expected, executeCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Stopwatch_Based_Timing_More_Precise_Than_DateTime ()
|
||||
{
|
||||
// Measure resolution by sampling multiple times rapidly
|
||||
var datetimeSamples = new List<long> ();
|
||||
var stopwatchSamples = new List<long> ();
|
||||
|
||||
for (int i = 0; i < 1000; i++)
|
||||
{
|
||||
datetimeSamples.Add (DateTime.UtcNow.Ticks);
|
||||
stopwatchSamples.Add (Stopwatch.GetTimestamp () * TimeSpan.TicksPerSecond / Stopwatch.Frequency);
|
||||
}
|
||||
|
||||
var datetimeUnique = datetimeSamples.Distinct ().Count ();
|
||||
var stopwatchUnique = stopwatchSamples.Distinct ().Count ();
|
||||
|
||||
// Stopwatch should provide more unique values (better resolution)
|
||||
Assert.True (stopwatchUnique >= datetimeUnique,
|
||||
$"Stopwatch should have equal or better resolution. DateTime: {datetimeUnique}, Stopwatch: {stopwatchUnique}");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user