Browse Source

Implement Stopwatch-based timing in TimedEvents to fix x64 race condition

Co-authored-by: tig <[email protected]>
copilot-swe-agent[bot] 1 month ago
parent
commit
a6d064a

+ 22 - 4
Terminal.Gui/App/Timeout/TimedEvents.cs

@@ -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

+ 2 - 1
Tests/UnitTests/Application/MainLoopTests.cs

@@ -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 - 0
Tests/UnitTests/Application/TimedEventsTests.cs

@@ -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}");
+    }
+}