From 1aba50a8e95798e1f4fdb8bed0980c68f120da96 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 29 Sep 2026 12:55:38 +0000 Subject: [PATCH 1/2] fix: end the run after repeated failed reads instead of spinning forever [patch] With stdin redirected, Console.ReadKey throws InvalidOperationException on every call. The input loop treated that as recoverable and retried at once, so RunAsync spun a core at ~200k reads a second and never returned. Failed reads are now counted, and after MaxConsecutiveReadFailures in a row the exception propagates out of RunAsync, which restores the cursor as usual. A successful read resets the count, and errors from handling input are not counted. Fixes ktsu-dev/TUI#143 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VCL6GX9wshgqiBeq6z98ZH --- TUI.Test/UIApplicationReadFailureTests.cs | 102 ++++++++++++++++++++++ TUI/Services/UIApplication.cs | 26 ++++++ 2 files changed, 128 insertions(+) create mode 100644 TUI.Test/UIApplicationReadFailureTests.cs diff --git a/TUI.Test/UIApplicationReadFailureTests.cs b/TUI.Test/UIApplicationReadFailureTests.cs new file mode 100644 index 0000000..e8782e9 --- /dev/null +++ b/TUI.Test/UIApplicationReadFailureTests.cs @@ -0,0 +1,102 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Contracts; +using ktsu.TUI.Core.Models; +using ktsu.TUI.Core.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests for how treats a console provider whose reads fail, such as +/// when stdin is redirected (ktsu-dev/TUI#143). +/// +[TestClass] +public sealed class UIApplicationReadFailureTests +{ + /// + /// How long a run is given to end before it is declared stuck. + /// + private static readonly TimeSpan StepTimeout = TimeSpan.FromSeconds(10); + + /// + /// Gets or sets the test context MSTest injects. + /// + public TestContext TestContext { get; set; } = null!; + + /// + /// A read that can never succeed ends the run with the provider's exception after a bounded + /// number of attempts, instead of retrying it in a tight loop forever. + /// + [TestMethod] + public async Task AReadThatAlwaysFailsEndsTheRun() + { + FailingReadConsoleProvider provider = new(failures: int.MaxValue); + UIApplication app = new(provider) { InterruptSource = new FakeInterruptSource() }; + + // Started on the thread pool because a read that fails synchronously never yields, and the + // run is cancelled once the timeout passes, so a regression fails the test rather than + // hanging it or leaving a run spinning in the test host. + using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.CancellationToken); + Task run = Task.Run(() => app.RunAsync(timeout.Token), TestContext.CancellationToken); + Task finished = await Task.WhenAny(run, Task.Delay(StepTimeout, TestContext.CancellationToken)).ConfigureAwait(false); + await timeout.CancelAsync().ConfigureAwait(false); + + Assert.AreSame(run, finished, "A read that always fails should end the run"); + await Assert.ThrowsExactlyAsync(() => run).ConfigureAwait(false); + Assert.IsLessThanOrEqualTo(100, provider.ReadCount, "The failing read should be retried a bounded number of times"); + Assert.IsTrue(provider.CursorVisible, "The cursor must be restored when the run ends on a read failure"); + } + + /// + /// An occasional failed read is still tolerated: the run carries on and ends normally. + /// + [TestMethod] + public async Task AFewFailedReadsDoNotEndTheRun() + { + FailingReadConsoleProvider provider = new(failures: 2); + UIApplication app = new(provider) { InterruptSource = new FakeInterruptSource() }; + + using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.CancellationToken); + Task run = Task.Run(() => app.RunAsync(timeout.Token), TestContext.CancellationToken); + Task finished = await Task.WhenAny(run, Task.Delay(StepTimeout, TestContext.CancellationToken)).ConfigureAwait(false); + await timeout.CancelAsync().ConfigureAwait(false); + + Assert.AreSame(run, finished, "The run should end on the exit key that follows the failed reads"); + await run.ConfigureAwait(false); + Assert.AreEqual(3, provider.ReadCount); + } + + /// + /// A provider whose first failures reads fail the way Console.ReadKey does with + /// redirected input, and whose next read is an exit key. + /// + private sealed class FailingReadConsoleProvider(int failures) : IConsoleProvider + { + private int readCount; + + public int ReadCount => Volatile.Read(ref readCount); + + public bool CursorVisible { get; private set; } = true; + + public Dimensions Dimensions => new(80, 24); + + public void Clear() { } + + public void Render(IUIElement element, Position position) => element?.Render(this); + + public void WriteAt(string text, Position position, TextStyle? style = null) { } + + public Task ReadInputAsync() + { + int count = Interlocked.Increment(ref readCount); + return count <= failures + ? Task.FromException(new InvalidOperationException("Cannot read keys when console input has been redirected.")) + : Task.FromResult(InputResult.Exit()); + } + + public void SetCursorVisibility(bool visible) => CursorVisible = visible; + + public void SetCursorPosition(Position position) { } + } +} diff --git a/TUI/Services/UIApplication.cs b/TUI/Services/UIApplication.cs index 7842282..cc2b9ec 100644 --- a/TUI/Services/UIApplication.cs +++ b/TUI/Services/UIApplication.cs @@ -93,6 +93,16 @@ public class UIApplication(IConsoleProvider consoleProvider, ILogger internal TimeSpan ResizePollInterval { get; init; } = TimeSpan.FromMilliseconds(100); + /// + /// How many reads in a row may fail before the input loop gives up and ends the run + /// + /// + /// An occasional failed read is tolerated, but a read that can never succeed, such as + /// Console.ReadKey with stdin redirected, would otherwise be retried in a tight loop + /// forever at full CPU (ktsu-dev/TUI#143). + /// + internal const int MaxConsecutiveReadFailures = 10; + /// /// The terminal size the current layout was computed for, or null before the first render /// @@ -338,9 +348,13 @@ public async Task ProcessInputAsync(CancellationToken cancellationToken = defaul // keypress, and starting a fresh read each time it woke would leave several reads racing // for the next key. Task? pendingRead = null; + int consecutiveReadFailures = 0; while (!cancellationToken.IsCancellationRequested && IsRunning) { + // Tells the catch blocks below whether the failure came from reading or from handling + bool readSucceeded = false; + try { pendingRead ??= ConsoleProvider.ReadInputAsync(); @@ -377,6 +391,8 @@ public async Task ProcessInputAsync(CancellationToken cancellationToken = defaul Task completedRead = pendingRead; pendingRead = null; Models.InputResult input = await completedRead.ConfigureAwait(false); + readSucceeded = true; + consecutiveReadFailures = 0; if (_logger != null) { @@ -419,6 +435,11 @@ public async Task ProcessInputAsync(CancellationToken cancellationToken = defaul LogInputProcessingError(_logger, ex); } + if (!readSucceeded && ++consecutiveReadFailures >= MaxConsecutiveReadFailures) + { + throw; + } + // Continue processing for recoverable errors } catch (ArgumentException ex) @@ -428,6 +449,11 @@ public async Task ProcessInputAsync(CancellationToken cancellationToken = defaul LogInputProcessingError(_logger, ex); } + if (!readSucceeded && ++consecutiveReadFailures >= MaxConsecutiveReadFailures) + { + throw; + } + // Continue processing for recoverable errors } catch (OutOfMemoryException) From 45c38d9dc1a10af4a5584d6df8c08048a1662322 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Tue, 29 Sep 2026 13:24:02 +0000 Subject: [PATCH 2/2] test: cover the ArgumentException read-failure path [patch] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VCL6GX9wshgqiBeq6z98ZH --- TUI.Test/UIApplicationReadFailureTests.cs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/TUI.Test/UIApplicationReadFailureTests.cs b/TUI.Test/UIApplicationReadFailureTests.cs index e8782e9..bac1ea8 100644 --- a/TUI.Test/UIApplicationReadFailureTests.cs +++ b/TUI.Test/UIApplicationReadFailureTests.cs @@ -48,6 +48,25 @@ public async Task AReadThatAlwaysFailsEndsTheRun() Assert.IsTrue(provider.CursorVisible, "The cursor must be restored when the run ends on a read failure"); } + /// + /// The other exception the loop treats as recoverable is bounded the same way. + /// + [TestMethod] + public async Task AReadThatAlwaysThrowsArgumentExceptionEndsTheRun() + { + FailingReadConsoleProvider provider = new(failures: int.MaxValue, () => new ArgumentException("Unreadable input.")); + UIApplication app = new(provider) { InterruptSource = new FakeInterruptSource() }; + + using CancellationTokenSource timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.CancellationToken); + Task run = Task.Run(() => app.RunAsync(timeout.Token), TestContext.CancellationToken); + Task finished = await Task.WhenAny(run, Task.Delay(StepTimeout, TestContext.CancellationToken)).ConfigureAwait(false); + await timeout.CancelAsync().ConfigureAwait(false); + + Assert.AreSame(run, finished, "A read that always fails should end the run"); + await Assert.ThrowsExactlyAsync(() => run).ConfigureAwait(false); + Assert.AreEqual(UIApplication.MaxConsecutiveReadFailures, provider.ReadCount); + } + /// /// An occasional failed read is still tolerated: the run carries on and ends normally. /// @@ -71,7 +90,7 @@ public async Task AFewFailedReadsDoNotEndTheRun() /// A provider whose first failures reads fail the way Console.ReadKey does with /// redirected input, and whose next read is an exit key. /// - private sealed class FailingReadConsoleProvider(int failures) : IConsoleProvider + private sealed class FailingReadConsoleProvider(int failures, Func? failure = null) : IConsoleProvider { private int readCount; @@ -91,7 +110,7 @@ public Task ReadInputAsync() { int count = Interlocked.Increment(ref readCount); return count <= failures - ? Task.FromException(new InvalidOperationException("Cannot read keys when console input has been redirected.")) + ? Task.FromException(failure?.Invoke() ?? new InvalidOperationException("Cannot read keys when console input has been redirected.")) : Task.FromResult(InputResult.Exit()); }