From a6d064ae8030d1b92b8bc19126967ff475a86705 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Oct 2025 17:25:25 +0000 Subject: [PATCH] Implement Stopwatch-based timing in TimedEvents to fix x64 race condition Co-authored-by: tig <585482+tig@users.noreply.github.com> --- Terminal.Gui/App/Timeout/TimedEvents.cs | 26 +++- Tests/UnitTests/Application/MainLoopTests.cs | 3 +- .../UnitTests/Application/TimedEventsTests.cs | 140 ++++++++++++++++++ 3 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 Tests/UnitTests/Application/TimedEventsTests.cs diff --git a/Terminal.Gui/App/Timeout/TimedEvents.cs b/Terminal.Gui/App/Timeout/TimedEvents.cs index b77e0520d..3137b9604 100644 --- a/Terminal.Gui/App/Timeout/TimedEvents.cs +++ b/Terminal.Gui/App/Timeout/TimedEvents.cs @@ -1,11 +1,13 @@ #nullable enable +using System.Diagnostics; + namespace Terminal.Gui.App; /// /// Manages scheduled timeouts (timed callbacks) for the application. /// /// 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. /// /// @@ -26,6 +28,10 @@ namespace Terminal.Gui.App; /// /// /// +/// +/// Uses for high-resolution timing instead of +/// to provide microsecond-level precision and eliminate race conditions from timer resolution issues. +/// public class TimedEvents : ITimedEvents { internal SortedList _timeouts = new (); @@ -40,6 +46,18 @@ public class TimedEvents : ITimedEvents /// public event EventHandler? Added; + /// + /// Gets the current high-resolution timestamp in TimeSpan ticks. + /// Uses for microsecond-level precision. + /// + /// Current timestamp in TimeSpan ticks (100-nanosecond units). + 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; + } + /// public void RunTimers () { @@ -92,7 +110,7 @@ public class TimedEvents : ITimedEvents /// 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 copy; // lock prevents new timeouts being added diff --git a/Tests/UnitTests/Application/MainLoopTests.cs b/Tests/UnitTests/Application/MainLoopTests.cs index f2caa1069..43797c8ba 100644 --- a/Tests/UnitTests/Application/MainLoopTests.cs +++ b/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; diff --git a/Tests/UnitTests/Application/TimedEventsTests.cs b/Tests/UnitTests/Application/TimedEventsTests.cs new file mode 100644 index 000000000..c09014bc5 --- /dev/null +++ b/Tests/UnitTests/Application/TimedEventsTests.cs @@ -0,0 +1,140 @@ +using System.Diagnostics; + +namespace UnitTests.ApplicationTests; + +/// +/// Tests for TimedEvents class, focusing on high-resolution timing with Stopwatch. +/// +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 (); + + 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 (); + var stopwatchSamples = new List (); + + 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}"); + } +}