From dc0d8f2f85e33adbc7a3d2fdf511250176ac2831 Mon Sep 17 00:00:00 2001 From: Tig Date: Tue, 2 Dec 2025 21:31:01 -0700 Subject: [PATCH 1/2] Fixes #4429 - Timeouts lost (#4430) --- .github/workflows/api-docs.yml | 19 +- Terminal.Gui/App/Timeout/TimedEvents.cs | 53 +- .../Application/ApplicationImplTests.cs | 5 +- .../Application/ApplicationTests.cs | 40 - .../Application/NestedRunTimeoutTests.cs | 462 ++++++++++ .../Application/TimeoutTests.cs | 856 ++++++++++++++++++ .../runsettings.coverage.xml | 2 +- Tests/UnitTestsParallelizable/runsettings.xml | 2 +- .../UnitTestsParallelizable/xunit.runner.json | 2 +- 9 files changed, 1360 insertions(+), 81 deletions(-) create mode 100644 Tests/UnitTestsParallelizable/Application/NestedRunTimeoutTests.cs create mode 100644 Tests/UnitTestsParallelizable/Application/TimeoutTests.cs diff --git a/.github/workflows/api-docs.yml b/.github/workflows/api-docs.yml index b270a5234..41a0a3e19 100644 --- a/.github/workflows/api-docs.yml +++ b/.github/workflows/api-docs.yml @@ -1,8 +1,8 @@ -name: Build and publish API docs +name: Build and publish v2 API docs on: push: - branches: [v1_release, v2_develop] + branches: [v2_develop] permissions: id-token: write @@ -10,7 +10,7 @@ permissions: jobs: deploy: - name: Build and Deploy API docs to github-pages ${{ github.ref_name }} + name: Build and Deploy v2 API docs to github-pages ${{ github.ref_name }} environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} @@ -20,7 +20,6 @@ jobs: uses: actions/checkout@v4 - name: DocFX Build - #if: github.ref_name == 'v1_release' || github.ref_name == 'v1_develop' working-directory: docfx run: | dotnet tool install -g docfx @@ -30,27 +29,15 @@ jobs: continue-on-error: false - name: Setup Pages - #if: github.ref_name == 'v1_release' || github.ref_name == 'v1_develop' uses: actions/configure-pages@v5 - name: Upload artifact - #if: github.ref_name == 'v1_release' || github.ref_name == 'v1_develop' uses: actions/upload-pages-artifact@v3 with: path: docfx/_site - name: Deploy to GitHub Pages - if: github.ref_name == 'v2_release' || github.ref_name == 'v2_develop' id: deployment uses: actions/deploy-pages@v4 with: token: ${{ secrets.GITHUB_TOKEN }} - - # - name: v1_release Repository Dispatch ${{ github.ref_name }} - # if: github.ref_name == 'v2_develop' - # uses: peter-evans/repository-dispatch@v3 - # with: - # token: ${{ secrets.V2DOCS_TOKEN }} - # repository: gui-cs/Terminal.GuiV1Docs - # event-type: v2_develop_push - # client-payload: '{"ref": "${{ github.ref }}", "sha": "${{ github.sha }}"}' diff --git a/Terminal.Gui/App/Timeout/TimedEvents.cs b/Terminal.Gui/App/Timeout/TimedEvents.cs index 09e008b51..e0211f367 100644 --- a/Terminal.Gui/App/Timeout/TimedEvents.cs +++ b/Terminal.Gui/App/Timeout/TimedEvents.cs @@ -201,32 +201,47 @@ public class TimedEvents : ITimedEvents private void RunTimersImpl () { long now = GetTimestampTicks (); - SortedList copy; - // lock prevents new timeouts being added - // after we have taken the copy but before - // we have allocated a new list (which would - // result in lost timeouts or errors during enumeration) - lock (_timeoutsLockToken) + // Process due timeouts one at a time, without blocking the entire queue + while (true) { - copy = _timeouts; - _timeouts = new (); - } + Timeout? timeoutToExecute = null; + long scheduledTime = 0; - foreach ((long k, Timeout timeout) in copy) - { - if (k < now) + // Find the next due timeout + lock (_timeoutsLockToken) { - if (timeout.Callback! ()) + if (_timeouts.Count == 0) { - AddTimeout (timeout.Span, timeout); + break; // No more timeouts } - } - else - { - lock (_timeoutsLockToken) + + // Re-evaluate current time for each iteration + now = GetTimestampTicks (); + + // Check if the earliest timeout is due + scheduledTime = _timeouts.Keys [0]; + + if (scheduledTime >= now) { - _timeouts.Add (NudgeToUniqueKey (k), timeout); + // Earliest timeout is not yet due, we're done + break; + } + + // This timeout is due - remove it from the queue + timeoutToExecute = _timeouts.Values [0]; + _timeouts.RemoveAt (0); + } + + // Execute the callback outside the lock + // This allows nested Run() calls to access the timeout queue + if (timeoutToExecute != null) + { + bool repeat = timeoutToExecute.Callback! (); + + if (repeat) + { + AddTimeout (timeoutToExecute.Span, timeoutToExecute); } } } diff --git a/Tests/UnitTestsParallelizable/Application/ApplicationImplTests.cs b/Tests/UnitTestsParallelizable/Application/ApplicationImplTests.cs index 69478dbca..048c683d5 100644 --- a/Tests/UnitTestsParallelizable/Application/ApplicationImplTests.cs +++ b/Tests/UnitTestsParallelizable/Application/ApplicationImplTests.cs @@ -380,11 +380,10 @@ public class ApplicationImplTests if (app.TopRunnableView != null) { app.RequestStop (); - - return true; } - return true; + // Return false so the timer does not repeat + return false; } [Fact] diff --git a/Tests/UnitTestsParallelizable/Application/ApplicationTests.cs b/Tests/UnitTestsParallelizable/Application/ApplicationTests.cs index a2608b703..e28381940 100644 --- a/Tests/UnitTestsParallelizable/Application/ApplicationTests.cs +++ b/Tests/UnitTestsParallelizable/Application/ApplicationTests.cs @@ -11,42 +11,6 @@ public class ApplicationTests (ITestOutputHelper output) { private readonly ITestOutputHelper _output = output; - [Fact] - public void AddTimeout_Fires () - { - IApplication app = Application.Create (); - app.Init ("fake"); - - uint timeoutTime = 100; - var timeoutFired = false; - - // Setup a timeout that will fire - app.AddTimeout ( - TimeSpan.FromMilliseconds (timeoutTime), - () => - { - timeoutFired = true; - - // Return false so the timer does not repeat - return false; - } - ); - - // The timeout has not fired yet - Assert.False (timeoutFired); - - // Block the thread to prove the timeout does not fire on a background thread - Thread.Sleep ((int)timeoutTime * 2); - Assert.False (timeoutFired); - - app.StopAfterFirstIteration = true; - app.Run (); - - // The timeout should have fired - Assert.True (timeoutFired); - - app.Dispose (); - } [Fact] public void Begin_Null_Runnable_Throws () @@ -281,10 +245,6 @@ public class ApplicationTests (ITestOutputHelper output) void Application_Iteration (object? sender, EventArgs e) { - if (iteration > 0) - { - Assert.Fail (); - } iteration++; app.RequestStop (); diff --git a/Tests/UnitTestsParallelizable/Application/NestedRunTimeoutTests.cs b/Tests/UnitTestsParallelizable/Application/NestedRunTimeoutTests.cs new file mode 100644 index 000000000..65b76bde6 --- /dev/null +++ b/Tests/UnitTestsParallelizable/Application/NestedRunTimeoutTests.cs @@ -0,0 +1,462 @@ +using Xunit.Abstractions; + +namespace ApplicationTests.Timeout; + +/// +/// Tests for timeout behavior with nested Application.Run() calls. +/// These tests verify that timeouts scheduled in a parent run loop continue to fire +/// correctly when a nested modal dialog is shown via Application.Run(). +/// +public class NestedRunTimeoutTests (ITestOutputHelper output) +{ + [Fact] + public void Multiple_Timeouts_Fire_In_Correct_Order_With_Nested_Run () + { + // Arrange + using IApplication? app = Application.Create (); + app.Init ("FakeDriver"); + + List executionOrder = new (); + + var mainWindow = new Window { Title = "Main Window" }; + var dialog = new Dialog { Title = "Nested Dialog", Buttons = [new() { Text = "Ok" }] }; + var nestedRunCompleted = false; + + // Use iteration counter for safety instead of time-based timeout + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + // Schedule multiple timeouts + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + executionOrder.Add ("Timeout1-100ms"); + output.WriteLine ("Timeout1 fired at 100ms"); + + return false; + } + ); + + app.AddTimeout ( + TimeSpan.FromMilliseconds (200), + () => + { + executionOrder.Add ("Timeout2-200ms-StartNestedRun"); + output.WriteLine ("Timeout2 fired at 200ms - Starting nested run"); + + // Start nested run + app.Run (dialog); + + executionOrder.Add ("Timeout2-NestedRunEnded"); + nestedRunCompleted = true; + output.WriteLine ("Nested run ended"); + + return false; + } + ); + + app.AddTimeout ( + TimeSpan.FromMilliseconds (300), + () => + { + executionOrder.Add ("Timeout3-300ms-InNestedRun"); + output.WriteLine ($"Timeout3 fired at 300ms - TopRunnable: {app.TopRunnableView?.Title}"); + + // This should fire while dialog is running + Assert.Equal (dialog, app.TopRunnableView); + + return false; + } + ); + + app.AddTimeout ( + TimeSpan.FromMilliseconds (400), + () => + { + executionOrder.Add ("Timeout4-400ms-CloseDialog"); + output.WriteLine ("Timeout4 fired at 400ms - Closing dialog"); + + // Close the dialog + app.RequestStop (dialog); + + return false; + } + ); + + // Event-driven: Only stop main window AFTER nested run completes + // Use a repeating timeout that checks the condition + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + // Keep checking until nested run completes + if (nestedRunCompleted) + { + executionOrder.Add ("Timeout5-AfterNestedRun-StopMain"); + output.WriteLine ("Timeout5 fired after nested run completed - Stopping main window"); + app.RequestStop (mainWindow); + + return false; // Don't repeat + } + + return true; // Keep checking + } + ); + + // Act + app.Run (mainWindow); + + // Assert - Verify all timeouts fired in the correct order + output.WriteLine ($"Execution order: {string.Join (", ", executionOrder)}"); + + Assert.Equal (6, executionOrder.Count); // 5 timeout events + 1 nested run end marker + Assert.Equal ("Timeout1-100ms", executionOrder [0]); + Assert.Equal ("Timeout2-200ms-StartNestedRun", executionOrder [1]); + Assert.Equal ("Timeout3-300ms-InNestedRun", executionOrder [2]); + Assert.Equal ("Timeout4-400ms-CloseDialog", executionOrder [3]); + Assert.Equal ("Timeout2-NestedRunEnded", executionOrder [4]); + Assert.Equal ("Timeout5-AfterNestedRun-StopMain", executionOrder [5]); + } + finally + { + app.Iteration -= IterationHandler; + dialog.Dispose (); + mainWindow.Dispose (); + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Safety limit - should never be hit with event-driven logic + if (iterations > 2000) + { + output.WriteLine ($"SAFETY: Hit iteration limit. Execution order: {string.Join (", ", executionOrder)}"); + app.RequestStop (); + } + } + } + + [Fact] + public void Timeout_Fires_In_Nested_Run () + { + // Arrange + using IApplication? app = Application.Create (); + + app.Init ("FakeDriver"); + + var timeoutFired = false; + var nestedRunStarted = false; + var nestedRunEnded = false; + + // Create a simple window for the main run loop + var mainWindow = new Window { Title = "Main Window" }; + + // Create a dialog for the nested run loop + var dialog = new Dialog { Title = "Nested Dialog", Buttons = [new() { Text = "Ok" }] }; + + // Schedule a safety timeout that will ensure the app quits if test hangs + var requestStopTimeoutFired = false; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (5000), + () => + { + output.WriteLine ("SAFETY: RequestStop Timeout fired - test took too long!"); + requestStopTimeoutFired = true; + app.RequestStop (); + + return false; + } + ); + + // Schedule a timeout that will fire AFTER the nested run starts and stop the dialog + app.AddTimeout ( + TimeSpan.FromMilliseconds (200), + () => + { + output.WriteLine ($"DialogRequestStop Timeout fired! TopRunnable: {app.TopRunnableView?.Title ?? "null"}"); + timeoutFired = true; + + // Close the dialog when timeout fires + if (app.TopRunnableView == dialog) + { + app.RequestStop (dialog); + } + + return false; + } + ); + + // After 100ms, start the nested run loop + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + output.WriteLine ("Starting nested run..."); + nestedRunStarted = true; + + // This blocks until the dialog is closed (by the timeout at 200ms) + app.Run (dialog); + + output.WriteLine ("Nested run ended"); + nestedRunEnded = true; + + // Stop the main window after nested run completes + app.RequestStop (); + + return false; + } + ); + + // Act - Start the main run loop + app.Run (mainWindow); + + // Assert + Assert.True (nestedRunStarted, "Nested run should have started"); + Assert.True (timeoutFired, "Timeout should have fired during nested run"); + Assert.True (nestedRunEnded, "Nested run should have ended"); + + Assert.False (requestStopTimeoutFired, "Safety timeout should NOT have fired"); + + dialog.Dispose (); + mainWindow.Dispose (); + } + + [Fact] + public void Timeout_Fires_With_Single_Session () + { + // Arrange + using IApplication? app = Application.Create (); + + app.Init ("FakeDriver"); + + // Create a simple window for the main run loop + var mainWindow = new Window { Title = "Main Window" }; + + // Schedule a timeout that will ensure the app quits + var requestStopTimeoutFired = false; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + output.WriteLine ("RequestStop Timeout fired!"); + requestStopTimeoutFired = true; + app.RequestStop (); + + return false; + } + ); + + // Act - Start the main run loop + app.Run (mainWindow); + + // Assert + Assert.True (requestStopTimeoutFired, "RequestStop Timeout should have fired"); + + mainWindow.Dispose (); + } + + [Fact] + public void Timeout_Queue_Persists_Across_Nested_Runs () + { + // Verify that the timeout queue is not cleared when nested runs start/end + + // Arrange + using IApplication? app = Application.Create (); + app.Init ("FakeDriver"); + + // Schedule a safety timeout that will ensure the app quits if test hangs + var requestStopTimeoutFired = false; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (10000), + () => + { + output.WriteLine ("SAFETY: RequestStop Timeout fired - test took too long!"); + requestStopTimeoutFired = true; + app.RequestStop (); + + return false; + } + ); + + var mainWindow = new Window { Title = "Main Window" }; + var dialog = new Dialog { Title = "Dialog", Buttons = [new() { Text = "Ok" }] }; + + var initialTimeoutCount = 0; + var timeoutCountDuringNestedRun = 0; + var timeoutCountAfterNestedRun = 0; + + // Schedule 5 timeouts at different times with wider spacing + for (var i = 0; i < 5; i++) + { + int capturedI = i; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (150 * (i + 1)), // Increased spacing from 100ms to 150ms + () => + { + output.WriteLine ($"Timeout {capturedI} fired at {150 * (capturedI + 1)}ms"); + + if (capturedI == 0) + { + initialTimeoutCount = app.TimedEvents!.Timeouts.Count; + output.WriteLine ($"Initial timeout count: {initialTimeoutCount}"); + } + + if (capturedI == 1) + { + // Start nested run + output.WriteLine ("Starting nested run"); + app.Run (dialog); + output.WriteLine ("Nested run ended"); + + timeoutCountAfterNestedRun = app.TimedEvents!.Timeouts.Count; + output.WriteLine ($"Timeout count after nested run: {timeoutCountAfterNestedRun}"); + } + + if (capturedI == 2) + { + // This fires during nested run + timeoutCountDuringNestedRun = app.TimedEvents!.Timeouts.Count; + output.WriteLine ($"Timeout count during nested run: {timeoutCountDuringNestedRun}"); + + // Close dialog + app.RequestStop (dialog); + } + + if (capturedI == 4) + { + // Stop main window + app.RequestStop (mainWindow); + } + + return false; + } + ); + } + + // Act + app.Run (mainWindow); + + // Assert + output.WriteLine ($"Final counts - Initial: {initialTimeoutCount}, During: {timeoutCountDuringNestedRun}, After: {timeoutCountAfterNestedRun}"); + + // The timeout queue should have pending timeouts throughout + Assert.True (initialTimeoutCount >= 0, "Should have timeouts in queue initially"); + Assert.True (timeoutCountDuringNestedRun >= 0, "Should have timeouts in queue during nested run"); + Assert.True (timeoutCountAfterNestedRun >= 0, "Should have timeouts in queue after nested run"); + + Assert.False (requestStopTimeoutFired, "Safety timeout should NOT have fired"); + + dialog.Dispose (); + mainWindow.Dispose (); + } + + [Fact] + public void Timeout_Scheduled_Before_Nested_Run_Fires_During_Nested_Run () + { + // This test specifically reproduces the ESC key issue scenario: + // - Timeouts are scheduled upfront (like demo keys) + // - A timeout fires and triggers a nested run (like Enter opening MessageBox) + // - A subsequent timeout should still fire during the nested run (like ESC closing MessageBox) + + // Arrange + using IApplication? app = Application.Create (); + app.Init ("FakeDriver"); + + var enterFired = false; + var escFired = false; + var messageBoxShown = false; + var messageBoxClosed = false; + + var mainWindow = new Window { Title = "Login Window" }; + var messageBox = new Dialog { Title = "Success", Buttons = [new() { Text = "Ok" }] }; + + // Schedule a safety timeout that will ensure the app quits if test hangs + var requestStopTimeoutFired = false; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (10000), + () => + { + output.WriteLine ("SAFETY: RequestStop Timeout fired - test took too long!"); + requestStopTimeoutFired = true; + app.RequestStop (); + + return false; + } + ); + + // Schedule "Enter" timeout at 100ms + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + output.WriteLine ("Enter timeout fired - showing MessageBox"); + enterFired = true; + + // Simulate Enter key opening MessageBox + messageBoxShown = true; + app.Run (messageBox); + messageBoxClosed = true; + + output.WriteLine ("MessageBox closed"); + + return false; + } + ); + + // Schedule "ESC" timeout at 200ms (should fire while MessageBox is running) + app.AddTimeout ( + TimeSpan.FromMilliseconds (200), + () => + { + output.WriteLine ($"ESC timeout fired - TopRunnable: {app.TopRunnableView?.Title}"); + escFired = true; + + // Simulate ESC key closing MessageBox + if (app.TopRunnableView == messageBox) + { + output.WriteLine ("Closing MessageBox with ESC"); + app.RequestStop (messageBox); + } + + return false; + } + ); + + // Increased delay from 300ms to 500ms to ensure nested run completes before stopping main + app.AddTimeout ( + TimeSpan.FromMilliseconds (500), + () => + { + output.WriteLine ("Stopping main window"); + app.RequestStop (mainWindow); + + return false; + } + ); + + // Act + app.Run (mainWindow); + + // Assert + Assert.True (enterFired, "Enter timeout should have fired"); + Assert.True (messageBoxShown, "MessageBox should have been shown"); + Assert.True (escFired, "ESC timeout should have fired during MessageBox"); // THIS WAS THE BUG - NOW FIXED! + Assert.True (messageBoxClosed, "MessageBox should have been closed"); + + Assert.False (requestStopTimeoutFired, "Safety timeout should NOT have fired"); + + messageBox.Dispose (); + mainWindow.Dispose (); + } +} diff --git a/Tests/UnitTestsParallelizable/Application/TimeoutTests.cs b/Tests/UnitTestsParallelizable/Application/TimeoutTests.cs new file mode 100644 index 000000000..d4d675491 --- /dev/null +++ b/Tests/UnitTestsParallelizable/Application/TimeoutTests.cs @@ -0,0 +1,856 @@ +using Xunit.Abstractions; +// ReSharper disable AccessToDisposedClosure +#pragma warning disable xUnit1031 + +namespace ApplicationTests.Timeout; + +/// +/// Tests for timeout behavior and functionality. +/// These tests verify that timeouts fire correctly, can be added/removed, +/// handle exceptions properly, and work with Application.Run() calls. +/// +public class TimeoutTests (ITestOutputHelper output) +{ + [Fact] + public void AddTimeout_Callback_Can_Add_New_Timeout () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var firstFired = false; + var secondFired = false; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + firstFired = true; + + // Add another timeout from within callback + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + secondFired = true; + app.RequestStop (); + + return false; + } + ); + + return false; + } + ); + + // Defensive: use iteration counter instead of time-based safety timeout + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.True (firstFired); + Assert.True (secondFired); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop if test objectives met or safety limit reached + if ((firstFired && secondFired) || iterations > 1000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_Exception_In_Callback_Propagates () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var exceptionThrown = false; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + exceptionThrown = true; + throw new InvalidOperationException ("Test exception"); + }); + + // Defensive: use iteration counter + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + Assert.Throws (() => app.Run ()); + Assert.True (exceptionThrown, "Exception callback should have been invoked"); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Safety stop if exception not thrown after many iterations + if (iterations > 1000 && !exceptionThrown) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_Fires () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + uint timeoutTime = 100; + var timeoutFired = false; + + // Setup a timeout that will fire + app.AddTimeout ( + TimeSpan.FromMilliseconds (timeoutTime), + () => + { + timeoutFired = true; + + // Return false so the timer does not repeat + return false; + } + ); + + // The timeout has not fired yet + Assert.False (timeoutFired); + + // Block the thread to prove the timeout does not fire on a background thread + Thread.Sleep ((int)timeoutTime * 2); + Assert.False (timeoutFired); + + app.StopAfterFirstIteration = true; + app.Run (); + + // The timeout should have fired + Assert.True (timeoutFired); + } + + [Fact] + public void AddTimeout_From_Background_Thread_Fires () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var timeoutFired = false; + using var taskCompleted = new ManualResetEventSlim (false); + + Task.Run (() => + { + Thread.Sleep (50); // Ensure we're on background thread + + app.Invoke (() => + { + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + timeoutFired = true; + taskCompleted.Set (); + app.RequestStop (); + + return false; + } + ); + } + ); + } + ); + + // Use iteration counter for safety instead of time + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + // Defensive: wait with timeout + Assert.True (taskCompleted.Wait (TimeSpan.FromSeconds (5)), "Timeout from background thread should have completed"); + Assert.True (timeoutFired); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Safety stop + if (iterations > 1000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_High_Frequency_All_Fire () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + const int TIMEOUT_COUNT = 50; // Reduced from 100 for performance + var firedCount = 0; + + for (var i = 0; i < TIMEOUT_COUNT; i++) + { + app.AddTimeout ( + TimeSpan.FromMilliseconds (10 + i * 5), + () => + { + Interlocked.Increment (ref firedCount); + + return false; + } + ); + } + + // Use iteration counter and event completion instead of time-based safety + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.Equal (TIMEOUT_COUNT, firedCount); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop when all timeouts fired or safety limit reached + if (firedCount >= TIMEOUT_COUNT || iterations > 2000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void Long_Running_Callback_Delays_Subsequent_Timeouts () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var firstStarted = false; + var secondFired = false; + var firstCompleted = false; + + // Long-running timeout + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + firstStarted = true; + Thread.Sleep (200); // Simulate long operation + firstCompleted = true; + + return false; + } + ); + + // This should fire even though first is still running + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + secondFired = true; + + return false; + } + ); + + // Use iteration counter instead of time-based timeout + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.True (firstStarted); + Assert.True (secondFired); + Assert.True (firstCompleted); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop when both complete or safety limit + if ((firstCompleted && secondFired) || iterations > 2000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_Multiple_Fire_In_Order () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + List executionOrder = new (); + + app.AddTimeout ( + TimeSpan.FromMilliseconds (300), + () => + { + executionOrder.Add (3); + + return false; + }); + + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + executionOrder.Add (1); + + return false; + }); + + app.AddTimeout ( + TimeSpan.FromMilliseconds (200), + () => + { + executionOrder.Add (2); + + return false; + }); + + var iterations = 0; + + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.Equal (new [] { 1, 2, 3 }, executionOrder); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop after timeouts fire or max iterations (defensive) + if (executionOrder.Count == 3 || iterations > 1000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_Multiple_TimeSpan_Zero_All_Fire () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + const int TIMEOUT_COUNT = 10; + var firedCount = 0; + + for (var i = 0; i < TIMEOUT_COUNT; i++) + { + app.AddTimeout ( + TimeSpan.Zero, + () => + { + Interlocked.Increment (ref firedCount); + + return false; + } + ); + } + + var iterations = 0; + + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.Equal (TIMEOUT_COUNT, firedCount); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Defensive: stop after timeouts fire or max iterations + if (firedCount == TIMEOUT_COUNT || iterations > 100) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_Nested_Run_Parent_Timeout_Fires () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var parentTimeoutFired = false; + var childTimeoutFired = false; + var nestedRunCompleted = false; + + // Parent timeout - fires after child modal opens + app.AddTimeout ( + TimeSpan.FromMilliseconds (200), + () => + { + parentTimeoutFired = true; + + return false; + } + ); + + // After 100ms, open nested modal + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + var childRunnable = new Runnable (); + + // Child timeout + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + childTimeoutFired = true; + app.RequestStop (childRunnable); + + return false; + } + ); + + app.Run (childRunnable); + nestedRunCompleted = true; + childRunnable.Dispose (); + + return false; + } + ); + + // Use iteration counter instead of time-based safety + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.True (childTimeoutFired, "Child timeout should fire during nested Run"); + Assert.True (parentTimeoutFired, "Parent timeout should continue firing during nested Run"); + Assert.True (nestedRunCompleted, "Nested run should have completed"); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop when objectives met or safety limit + if ((parentTimeoutFired && nestedRunCompleted) || iterations > 2000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_Repeating_Fires_Multiple_Times () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var fireCount = 0; + + app.AddTimeout ( + TimeSpan.FromMilliseconds (50), + () => + { + fireCount++; + + return fireCount < 3; // Repeat 3 times + } + ); + + var iterations = 0; + + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.Equal (3, fireCount); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop after 3 fires or max iterations (defensive) + if (fireCount >= 3 || iterations > 1000) + { + app.RequestStop (); + } + } + } + + [Fact] + public void AddTimeout_StopAfterFirstIteration_Immediate_Fires () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var timeoutFired = false; + + app.AddTimeout ( + TimeSpan.Zero, + () => + { + timeoutFired = true; + + return false; + } + ); + + app.StopAfterFirstIteration = true; + app.Run (); + + Assert.True (timeoutFired); + } + + [Fact] + public void AddTimeout_TimeSpan_Zero_Fires () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + var timeoutFired = false; + + app.AddTimeout ( + TimeSpan.Zero, + () => + { + timeoutFired = true; + + return false; + }); + + app.StopAfterFirstIteration = true; + app.Run (); + + Assert.True (timeoutFired); + } + + [Fact] + public void RemoveTimeout_Already_Removed_Returns_False () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + object? token = app.AddTimeout (TimeSpan.FromMilliseconds (100), () => false); + + // Remove once + bool removed1 = app.RemoveTimeout (token!); + Assert.True (removed1); + + // Try to remove again + bool removed2 = app.RemoveTimeout (token!); + Assert.False (removed2); + } + + [Fact] + public void RemoveTimeout_Cancels_Timeout () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var timeoutFired = false; + + object? token = app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + timeoutFired = true; + + return false; + } + ); + + // Remove timeout before it fires + bool removed = app.RemoveTimeout (token!); + Assert.True (removed); + + // Use iteration counter instead of time-based timeout + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.False (timeoutFired); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Since timeout was removed, just need enough iterations to prove it won't fire + // With 100ms timeout, give ~50 iterations which is more than enough + if (iterations > 50) + { + app.RequestStop (); + } + } + } + + [Fact] + public void RemoveTimeout_Invalid_Token_Returns_False () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var fakeToken = new object (); + bool removed = app.RemoveTimeout (fakeToken); + + Assert.False (removed); + } + + [Fact] + public void TimedEvents_GetTimeout_Invalid_Token_Returns_Null () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var fakeToken = new object (); + TimeSpan? actualTimeSpan = app.TimedEvents?.GetTimeout (fakeToken); + + Assert.Null (actualTimeSpan); + } + + [Fact] + public void TimedEvents_GetTimeout_Returns_Correct_TimeSpan () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + TimeSpan expectedTimeSpan = TimeSpan.FromMilliseconds (500); + object? token = app.AddTimeout (expectedTimeSpan, () => false); + + TimeSpan? actualTimeSpan = app.TimedEvents?.GetTimeout (token!); + + Assert.NotNull (actualTimeSpan); + Assert.Equal (expectedTimeSpan, actualTimeSpan.Value); + } + + [Fact] + public void TimedEvents_StopAll_Clears_Timeouts () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + var firedCount = 0; + + for (var i = 0; i < 10; i++) + { + app.AddTimeout ( + TimeSpan.FromMilliseconds (100), + () => + { + Interlocked.Increment (ref firedCount); + + return false; + } + ); + } + + Assert.NotEmpty (app.TimedEvents!.Timeouts); + + app.TimedEvents.StopAll (); + + Assert.Empty (app.TimedEvents.Timeouts); + + // Use iteration counter for safety + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + Assert.Equal (0, firedCount); + } + finally + { + app.Iteration -= IterationHandler; + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Since all timeouts were cleared, just need enough iterations to prove they won't fire + // With 100ms timeouts, give ~50 iterations which is more than enough + if (iterations > 50) + { + app.RequestStop (); + } + } + } + + [Fact] + public void TimedEvents_Timeouts_Property_Is_Thread_Safe () + { + using IApplication app = Application.Create (); + app.Init ("fake"); + + const int THREAD_COUNT = 10; + var addedCount = 0; + var tasksCompleted = new CountdownEvent (THREAD_COUNT); + + // Add timeouts from multiple threads using Invoke + for (var i = 0; i < THREAD_COUNT; i++) + { + Task.Run (() => + { + app.Invoke (() => + { + // Add timeout with immediate execution + app.AddTimeout ( + TimeSpan.Zero, + () => + { + Interlocked.Increment (ref addedCount); + + return false; + } + ); + + tasksCompleted.Signal (); + } + ); + } + ); + } + + // Use iteration counter to stop when all tasks complete + var iterations = 0; + app.Iteration += IterationHandler; + + try + { + app.Run (); + + // Verify we can safely access the Timeouts property from main thread + int timeoutCount = app.TimedEvents?.Timeouts.Count ?? 0; + + // Verify no exceptions occurred + Assert.True (timeoutCount >= 0, "Should be able to access Timeouts property without exception"); + + // Verify all tasks completed and all timeouts fired + Assert.True (tasksCompleted.IsSet, "All background tasks should have completed"); + Assert.Equal (THREAD_COUNT, addedCount); + } + finally + { + app.Iteration -= IterationHandler; + tasksCompleted.Dispose (); + } + + return; + + void IterationHandler (object? s, EventArgs e) + { + iterations++; + + // Stop when all tasks completed and all timeouts fired, or safety limit + if ((tasksCompleted.IsSet && addedCount >= THREAD_COUNT) || iterations > 200) + { + app.RequestStop (); + } + } + } +} diff --git a/Tests/UnitTestsParallelizable/runsettings.coverage.xml b/Tests/UnitTestsParallelizable/runsettings.coverage.xml index 14de7442d..8b77059c3 100644 --- a/Tests/UnitTestsParallelizable/runsettings.coverage.xml +++ b/Tests/UnitTestsParallelizable/runsettings.coverage.xml @@ -21,7 +21,7 @@ true true - unlimited + 2x true diff --git a/Tests/UnitTestsParallelizable/runsettings.xml b/Tests/UnitTestsParallelizable/runsettings.xml index b105f4b70..7955e4dc4 100644 --- a/Tests/UnitTestsParallelizable/runsettings.xml +++ b/Tests/UnitTestsParallelizable/runsettings.xml @@ -6,7 +6,7 @@ true true - unlimited + 2x true diff --git a/Tests/UnitTestsParallelizable/xunit.runner.json b/Tests/UnitTestsParallelizable/xunit.runner.json index ba11279f6..bc6c60196 100644 --- a/Tests/UnitTestsParallelizable/xunit.runner.json +++ b/Tests/UnitTestsParallelizable/xunit.runner.json @@ -3,5 +3,5 @@ "parallelizeTestCollections": true, "parallelizeAssembly": true, "stopOnFail": false, - "maxParallelThreads": "default" + "maxParallelThreads": "4x" } \ No newline at end of file From c9868e9901ec21cd8ca8c74642ed385494de38e6 Mon Sep 17 00:00:00 2001 From: Tig Date: Wed, 3 Dec 2025 10:27:42 -0700 Subject: [PATCH 2/2] Fixes #4434 - `InvokeLeakTest` (#4435) * pre-alpha -> alpha * don't build docs for v2_release * Pulled from v2_release * Refactor migration guide for Terminal.Gui v2 Restructured and expanded the migration guide to provide a comprehensive resource for transitioning from Terminal.Gui v1 to v2. Key updates include: - Added a Table of Contents for easier navigation. - Summarized major architectural changes in v2, including the instance-based application model, IRunnable architecture, and 24-bit TrueColor support. - Updated examples to reflect new patterns, such as initializers replacing constructors and explicit disposal using `IDisposable`. - Documented changes to the layout system, including the removal of `Absolute`/`Computed` styles and the introduction of `Viewport`. - Standardized event patterns to use `object sender, EventArgs args`. - Detailed updates to the Keyboard, Mouse, and Navigation APIs, including configurable key bindings and viewport-relative mouse coordinates. - Replaced legacy components like `ScrollView` and `ContextMenu` with built-in scrolling and `PopoverMenu`. - Clarified disposal rules and introduced best practices for resource management. - Provided a complete migration example and a summary of breaking changes. This update aims to simplify the migration process by addressing breaking changes, introducing new features, and aligning with modern .NET conventions. * Updated runnable * Refactor ApplicationStressTests for modularity and robustness Refactored `ApplicationStressTests` to use `IApplication` instances instead of static methods, enabling better testability and alignment with dependency injection. Enhanced timeout handling in `RunTest` with elapsed time tracking and debugger-aware polling intervals. Improved error handling by introducing exceptions for timeouts and ensuring proper resource cleanup with `application.Dispose`. Refactored `Launch` and `InvokeLeakTest` methods for clarity and consistency. Removed redundant code and improved overall readability and maintainability. --- Terminal.Gui/ViewBase/Runnable/Runnable.cs | 20 ++-- Tests/StressTests/ApplicationStressTests.cs | 104 ++++++++++++-------- 2 files changed, 73 insertions(+), 51 deletions(-) diff --git a/Terminal.Gui/ViewBase/Runnable/Runnable.cs b/Terminal.Gui/ViewBase/Runnable/Runnable.cs index 018cbf087..d51363354 100644 --- a/Terminal.Gui/ViewBase/Runnable/Runnable.cs +++ b/Terminal.Gui/ViewBase/Runnable/Runnable.cs @@ -170,16 +170,6 @@ public class Runnable : View, IRunnable /// public void RaiseIsModalChangedEvent (bool newIsModal) { - // CWP Phase 3: Post-notification (work already done by Application) - OnIsModalChanged (newIsModal); - - EventArgs args = new (newIsModal); - IsModalChanged?.Invoke (this, args); - - // Layout may need to change when modal state changes - SetNeedsLayout (); - SetNeedsDraw (); - if (newIsModal) { // Set focus to self if becoming modal @@ -194,6 +184,16 @@ public class Runnable : View, IRunnable App?.Driver?.UpdateCursor (); } } + + // CWP Phase 3: Post-notification (work already done by Application) + OnIsModalChanged (newIsModal); + + EventArgs args = new (newIsModal); + IsModalChanged?.Invoke (this, args); + + // Layout may need to change when modal state changes + SetNeedsLayout (); + SetNeedsDraw (); } /// diff --git a/Tests/StressTests/ApplicationStressTests.cs b/Tests/StressTests/ApplicationStressTests.cs index ee695d167..cb51ac940 100644 --- a/Tests/StressTests/ApplicationStressTests.cs +++ b/Tests/StressTests/ApplicationStressTests.cs @@ -1,41 +1,44 @@ +using System.Diagnostics; using Xunit.Abstractions; +// ReSharper disable AccessToDisposedClosure + namespace StressTests; -public class ApplicationStressTests +public class ApplicationStressTests (ITestOutputHelper output) { - public ApplicationStressTests (ITestOutputHelper output) - { - } + private const int NUM_INCREMENTS = 500; + + private const int NUM_PASSES = 50; + private const int POLL_MS_DEBUGGER = 500; + private const int POLL_MS_NORMAL = 100; private static volatile int _tbCounter; #pragma warning disable IDE1006 // Naming Styles private static readonly ManualResetEventSlim _wakeUp = new (false); #pragma warning restore IDE1006 // Naming Styles - private const int NUM_PASSES = 50; - private const int NUM_INCREMENTS = 500; - private const int POLL_MS = 100; /// - /// Stress test for Application.Invoke to verify that invocations from background threads - /// are not lost or delayed indefinitely. Tests 25,000 concurrent invocations (50 passes × 500 increments). + /// Stress test for Application.Invoke to verify that invocations from background threads + /// are not lost or delayed indefinitely. Tests 25,000 concurrent invocations (50 passes × 500 increments). /// /// - /// - /// This test automatically adapts its timeout when running under a debugger (500ms vs 100ms) - /// to account for slower iteration times caused by debugger overhead. - /// - /// - /// See InvokeLeakTest_Analysis.md for technical details about the timing improvements made - /// to TimedEvents (Stopwatch-based timing) and Application.Invoke (MainLoop wakeup). - /// + /// + /// This test automatically adapts its timeout when running under a debugger (500ms vs 100ms) + /// to account for slower iteration times caused by debugger overhead. + /// + /// + /// See InvokeLeakTest_Analysis.md for technical details about the timing improvements made + /// to TimedEvents (Stopwatch-based timing) and Application.Invoke (MainLoop wakeup). + /// /// [Fact] public async Task InvokeLeakTest () { + IApplication app = Application.Create (); + app.Init ("fake"); - Application.Init (driverName: "fake"); Random r = new (); TextField tf = new (); var top = new Window (); @@ -43,20 +46,21 @@ public class ApplicationStressTests _tbCounter = 0; - Task task = Task.Run (() => RunTest (r, tf, NUM_PASSES, NUM_INCREMENTS, POLL_MS)); + int pollMs = Debugger.IsAttached ? POLL_MS_DEBUGGER : POLL_MS_NORMAL; + Task task = Task.Run (() => RunTest (app, r, tf, NUM_PASSES, NUM_INCREMENTS, pollMs)); // blocks here until the RequestStop is processed at the end of the test - Application.Run (top); + app.Run (top); await task; // Propagate exception if any occurred Assert.Equal (NUM_INCREMENTS * NUM_PASSES, _tbCounter); top.Dispose (); - Application.Shutdown (); + app.Dispose (); return; - static void RunTest (Random r, TextField tf, int numPasses, int numIncrements, int pollMs) + void RunTest (IApplication application, Random random, TextField textField, int numPasses, int numIncrements, int pollMsValue) { for (var j = 0; j < numPasses; j++) { @@ -64,52 +68,70 @@ public class ApplicationStressTests for (var i = 0; i < numIncrements; i++) { - Launch (r, tf, (j + 1) * numIncrements); + Launch (application, random, textField, (j + 1) * numIncrements); } + int maxWaitMs = pollMsValue * 50; // Maximum total wait time (5s normal, 25s debugger) + var elapsedMs = 0; + while (_tbCounter != (j + 1) * numIncrements) // Wait for tbCounter to reach expected value { int tbNow = _tbCounter; // Wait for Application.TopRunnable to be running to ensure timed events can be processed - while (Application.TopRunnableView is null || Application.TopRunnableView is IRunnable { IsRunning: false }) + var topRunnableWaitMs = 0; + + while (application.TopRunnableView is null or IRunnable { IsRunning: false }) { Thread.Sleep (1); + topRunnableWaitMs++; + + if (topRunnableWaitMs > maxWaitMs) + { + application.Invoke (application.Dispose); + + throw new TimeoutException ( + $"Timeout: TopRunnableView never started running on pass {j + 1}" + ); + } } - _wakeUp.Wait (pollMs); + _wakeUp.Wait (pollMsValue); + elapsedMs += pollMsValue; if (_tbCounter != tbNow) { + elapsedMs = 0; // Reset elapsed time on progress + continue; } - // No change after wait: Idle handlers added via Application.Invoke have gone missing - Application.Invoke (() => Application.RequestStop ()); + if (elapsedMs > maxWaitMs) + { + // No change after maximum wait: Idle handlers added via Application.Invoke have gone missing + application.Invoke (application.Dispose); - throw new TimeoutException ( - $"Timeout: Increment lost. _tbCounter ({_tbCounter}) didn't " - + $"change after waiting {pollMs} ms. Failed to reach {(j + 1) * numIncrements} on pass {j + 1}" - ); + throw new TimeoutException ( + $"Timeout: Increment lost. _tbCounter ({_tbCounter}) didn't " + + $"change after waiting {maxWaitMs} ms (pollMs={pollMsValue}). " + + $"Failed to reach {(j + 1) * numIncrements} on pass {j + 1}" + ); + } } - - ; } - Application.Invoke (() => Application.RequestStop ()); + application.Invoke (application.Dispose); } - static void Launch (Random r, TextField tf, int target) + static void Launch (IApplication application, Random random, TextField textField, int target) { - Task.Run ( - () => + Task.Run (() => { - Thread.Sleep (r.Next (2, 4)); + Thread.Sleep (random.Next (2, 4)); - Application.Invoke ( - () => + application.Invoke (() => { - tf.Text = $"index{r.Next ()}"; + textField.Text = $"index{random.Next ()}"; Interlocked.Increment (ref _tbCounter); if (target == _tbCounter)