diff --git a/Terminal.Gui/ConsoleDrivers/AnsiResponseParser/AnsiRequestScheduler.cs b/Terminal.Gui/ConsoleDrivers/AnsiResponseParser/AnsiRequestScheduler.cs index 20e223e215..3d1f08a1a4 100644 --- a/Terminal.Gui/ConsoleDrivers/AnsiResponseParser/AnsiRequestScheduler.cs +++ b/Terminal.Gui/ConsoleDrivers/AnsiResponseParser/AnsiRequestScheduler.cs @@ -19,7 +19,7 @@ internal class AnsiRequestScheduler /// internal Func Now { get; set; } - private readonly List> _queuedRequests = new (); + private readonly HashSet> _queuedRequests = new (); internal IReadOnlyCollection QueuedRequests => _queuedRequests.Select (r => r.Item1).ToList (); @@ -68,6 +68,10 @@ public AnsiRequestScheduler (IAnsiResponseParser parser, Func? now = n /// /// if request was sent immediately. if it was queued. public bool SendOrSchedule (AnsiEscapeSequenceRequest request) + { + return SendOrSchedule (request, true); + } + private bool SendOrSchedule (AnsiEscapeSequenceRequest request,bool addToQueue) { if (CanSend (request, out ReasonCannotSend reason)) { @@ -91,7 +95,10 @@ public bool SendOrSchedule (AnsiEscapeSequenceRequest request) } } - _queuedRequests.Add (Tuple.Create (request, Now ())); + if (addToQueue) + { + _queuedRequests.Add (Tuple.Create (request, Now ())); + } return false; } @@ -137,14 +144,17 @@ public bool RunSchedule (bool force = false) return false; } - Tuple? opportunity = _queuedRequests.FirstOrDefault (r => CanSend (r.Item1, out _)); + // Get oldest request + Tuple? opportunity = _queuedRequests.MinBy (r=>r.Item2); if (opportunity != null) { - _queuedRequests.Remove (opportunity); - Send (opportunity.Item1); - - return true; + // Give it another go + if (SendOrSchedule (opportunity.Item1, false)) + { + _queuedRequests.Remove (opportunity); + return true; + } } return false; diff --git a/UnitTests/ConsoleDrivers/AnsiRequestSchedulerTests.cs b/UnitTests/ConsoleDrivers/AnsiRequestSchedulerTests.cs index 2526ee534c..d184c8bae2 100644 --- a/UnitTests/ConsoleDrivers/AnsiRequestSchedulerTests.cs +++ b/UnitTests/ConsoleDrivers/AnsiRequestSchedulerTests.cs @@ -144,6 +144,53 @@ public void RunSchedule_ThrottleExceeded_QueueRequest () _parserMock.Verify (); } + + [Fact] + public void EvictStaleRequests_RemovesStaleRequest_AfterTimeout () + { + // Arrange + var request1 = new AnsiEscapeSequenceRequest + { + Request = "\u001b[0c", + Terminator = "c", + ResponseReceived = r => { } + }; + + // Send + _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Once); + _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny> (), false)).Verifiable (Times.Exactly (2)); + + Assert.True (_scheduler.SendOrSchedule (request1)); + + // Parser already has an ongoing request for "c" + _parserMock.Setup (p => p.IsExpecting ("c")).Returns (true).Verifiable (Times.Exactly (2)); + + // Cannot send because there is already outstanding request + Assert.False(_scheduler.SendOrSchedule (request1)); + Assert.Single (_scheduler.QueuedRequests); + + // Simulate request going stale + SetTime (5001); // Exceeds stale timeout + + // Parser should be told to give up on this one (evicted) + _parserMock.Setup (p => p.StopExpecting ("c", false)) + .Callback (() => + { + // When we tell parser to evict - it should now tell us it is no longer expecting + _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Once); + }).Verifiable (); + + // When we send again the evicted one should be + var evicted = _scheduler.RunSchedule (); + + Assert.True (evicted); // Stale request should be evicted + Assert.Empty (_scheduler.QueuedRequests); + + // Assert + _parserMock.Verify (); + } + + private void SetTime (int milliseconds) { // This simulates the passing of time by setting the Now function to return a specific time.