From e4f3b97da09c83f9d0257edd5d49a2a406f35083 Mon Sep 17 00:00:00 2001 From: Tigger Kindel Date: Thu, 5 Oct 2023 09:14:51 -0600 Subject: [PATCH] Fixed WindowsDriver mem leak --- Terminal.Gui/Application.cs | 2 +- .../CursesDriver/UnixMainLoop.cs | 8 +++ .../ConsoleDrivers/FakeDriver/FakeMainLoop.cs | 5 ++ Terminal.Gui/ConsoleDrivers/NetDriver.cs | 15 ++++- Terminal.Gui/ConsoleDrivers/WindowsDriver.cs | 58 +++++++++++-------- Terminal.Gui/MainLoop.cs | 51 +++++++++++----- UnitTests/Application/MainLoopTests.cs | 4 +- 7 files changed, 100 insertions(+), 43 deletions(-) diff --git a/Terminal.Gui/Application.cs b/Terminal.Gui/Application.cs index 52198afad..3cb0cd808 100644 --- a/Terminal.Gui/Application.cs +++ b/Terminal.Gui/Application.cs @@ -242,7 +242,7 @@ namespace Terminal.Gui { // BUGBUG: OverlappedTop is not cleared here, but it should be? - MainLoop?.Stop (); + MainLoop?.Dispose (); MainLoop = null; Driver?.End (); Driver = null; diff --git a/Terminal.Gui/ConsoleDrivers/CursesDriver/UnixMainLoop.cs b/Terminal.Gui/ConsoleDrivers/CursesDriver/UnixMainLoop.cs index 85e81ca8f..9a276b07c 100644 --- a/Terminal.Gui/ConsoleDrivers/CursesDriver/UnixMainLoop.cs +++ b/Terminal.Gui/ConsoleDrivers/CursesDriver/UnixMainLoop.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Runtime.InteropServices; +using static Terminal.Gui.NetEvents; namespace Terminal.Gui { /// @@ -205,5 +206,12 @@ namespace Terminal.Gui { } } } + + public void TearDown () + { + _descriptorWatchers?.Clear (); + + _mainLoop = null; + } } } diff --git a/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeMainLoop.cs b/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeMainLoop.cs index 012cb67ee..0279a27d9 100644 --- a/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeMainLoop.cs +++ b/Terminal.Gui/ConsoleDrivers/FakeDriver/FakeMainLoop.cs @@ -1,4 +1,5 @@ using System; +using static Terminal.Gui.NetEvents; namespace Terminal.Gui; @@ -33,5 +34,9 @@ internal class FakeMainLoop : IMainLoopDriver { KeyPressed?.Invoke (FakeConsole.MockKeyPresses.Pop ()); } } + + public void TearDown () + { + } } diff --git a/Terminal.Gui/ConsoleDrivers/NetDriver.cs b/Terminal.Gui/ConsoleDrivers/NetDriver.cs index e1f4cc06b..18315d1a3 100644 --- a/Terminal.Gui/ConsoleDrivers/NetDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/NetDriver.cs @@ -10,6 +10,7 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using System.Text; +using static Terminal.Gui.NetEvents; namespace Terminal.Gui; class NetWinVTConsole { @@ -1394,8 +1395,20 @@ internal class NetMainLoop : IMainLoopDriver { ProcessInput?.Invoke (_inputResult.Dequeue ().Value); } } + public void TearDown () { - //throw new NotImplementedException (); + _inputResult?.Clear (); + + _tokenSource?.Cancel (); + _tokenSource?.Dispose (); + + _keyReady?.Dispose (); + + _waitForProbe?.Dispose (); + + _netEvents?.Dispose (); + + _mainLoop = null; } } \ No newline at end of file diff --git a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs b/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs index 7f94c2081..56c2742ff 100644 --- a/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs +++ b/Terminal.Gui/ConsoleDrivers/WindowsDriver.cs @@ -1739,7 +1739,7 @@ internal class WindowsDriver : ConsoleDriver { //_mainLoop.Dispose (); } _mainLoop = null; - + WinConsole?.Cleanup (); WinConsole = null; @@ -1774,7 +1774,8 @@ internal class WindowsMainLoop : IMainLoopDriver { bool _winChanged; Size _windowSize; CancellationTokenSource _eventReadyTokenSource = new CancellationTokenSource (); - + CancellationTokenSource _inputHandlerTokenSource = new CancellationTokenSource (); + // The records that we keep fetching readonly Queue _resultQueue = new Queue (); @@ -1790,22 +1791,30 @@ internal class WindowsMainLoop : IMainLoopDriver { public WindowsMainLoop (ConsoleDriver consoleDriver = null) { - _consoleDriver = consoleDriver ?? throw new ArgumentNullException (nameof(consoleDriver)); + _consoleDriver = consoleDriver ?? throw new ArgumentNullException (nameof (consoleDriver)); _winConsole = ((WindowsDriver)consoleDriver).WinConsole; } void IMainLoopDriver.Setup (MainLoop mainLoop) { _mainLoop = mainLoop; - Task.Run (WindowsInputHandler); + Task.Run (WindowsInputHandler, _inputHandlerTokenSource.Token); Task.Run (CheckWinChange); } - + void WindowsInputHandler () { while (_mainLoop != null) { - _waitForProbe.Wait (); - _waitForProbe.Reset (); + try { + if (!_inputHandlerTokenSource.IsCancellationRequested) { + _waitForProbe.Wait (_inputHandlerTokenSource.Token); + } + + } catch (OperationCanceledException) { + return; + } finally { + _waitForProbe.Reset (); + } if (_resultQueue?.Count == 0) { _resultQueue.Enqueue (_winConsole.ReadConsoleInput ()); @@ -1820,7 +1829,7 @@ internal class WindowsMainLoop : IMainLoopDriver { while (_mainLoop != null) { _winChange.Wait (); _winChange.Reset (); - + while (_mainLoop != null) { Task.Delay (500).Wait (); _windowSize = _winConsole.GetConsoleBufferWindow (out _); @@ -1829,18 +1838,17 @@ internal class WindowsMainLoop : IMainLoopDriver { break; } } - + _winChanged = true; _eventReady.Set (); } } - + void IMainLoopDriver.Wakeup () { - //tokenSource.Cancel (); _eventReady.Set (); } - + bool IMainLoopDriver.EventsPending () { _waitForProbe.Set (); @@ -1869,8 +1877,6 @@ internal class WindowsMainLoop : IMainLoopDriver { _eventReadyTokenSource = new CancellationTokenSource (); return true; } - - void IMainLoopDriver.Iteration () { @@ -1886,17 +1892,21 @@ internal class WindowsMainLoop : IMainLoopDriver { WinChanged?.Invoke (this, new SizeChangedEventArgs (_windowSize)); } } - - //public void Dispose () - //{ - // _eventReadyTokenSource?.Cancel (); - // _eventReadyTokenSource?.Dispose (); - // _eventReady?.Dispose (); - // _mainLoop = null; - // _winChange?.Dispose (); - // _waitForProbe?.Dispose (); - //} + void IMainLoopDriver.TearDown () + { + _inputHandlerTokenSource?.Cancel (); + _inputHandlerTokenSource?.Dispose (); + + _eventReadyTokenSource?.Cancel (); + _eventReadyTokenSource?.Dispose (); + _eventReady?.Dispose (); + + _winChange?.Dispose (); + _waitForProbe?.Dispose (); + + _mainLoop = null; + } } class WindowsClipboard : ClipboardBase { diff --git a/Terminal.Gui/MainLoop.cs b/Terminal.Gui/MainLoop.cs index 11038b033..c210bc507 100644 --- a/Terminal.Gui/MainLoop.cs +++ b/Terminal.Gui/MainLoop.cs @@ -16,6 +16,9 @@ namespace Terminal.Gui { /// /// Initializes the , gets the calling main loop for the initialization. /// + /// + /// Call to release resources. + /// /// Main loop. void Setup (MainLoop mainLoop); @@ -34,6 +37,11 @@ namespace Terminal.Gui { /// The iteration function. /// void Iteration (); + + /// + /// Tears down the driver. Releases resources created in . + /// + void TearDown (); } /// @@ -43,7 +51,7 @@ namespace Terminal.Gui { /// Monitoring of file descriptors is only available on Unix, there /// does not seem to be a way of supporting this on Windows. /// - public class MainLoop { + public class MainLoop : IDisposable { internal SortedList _timeouts = new SortedList (); readonly object _timeoutsLockToken = new object (); @@ -76,7 +84,7 @@ namespace Terminal.Gui { /// The current in use. /// /// The main loop driver. - public IMainLoopDriver MainLoopDriver { get; } + public IMainLoopDriver MainLoopDriver { get; private set; } /// /// Invoked when a new timeout is added. To be used in the case @@ -87,6 +95,9 @@ namespace Terminal.Gui { /// /// Creates a new MainLoop. /// + /// + /// Use to release resources. + /// /// The instance /// (one of the implementations FakeMainLoop, UnixMainLoop, NetMainLoop or WindowsMainLoop). public MainLoop (IMainLoopDriver driver) @@ -294,17 +305,6 @@ namespace Terminal.Gui { bool _running; - // BUGBUG: Stop is only called from MainLoopUnitTests.cs. As a result, the mainloop - // will never exit during other unit tests or normal execution. - /// - /// Stops the mainloop. - /// - public void Stop () - { - _running = false; - MainLoopDriver.Wakeup (); - } - /// /// Determines whether there are pending events to be processed. /// @@ -325,7 +325,7 @@ namespace Terminal.Gui { /// Use this to process all pending events (timers, idle handlers and file watches). /// /// - /// while (main.EvenstPending ()) RunIteration (); + /// while (main.EventsPending ()) RunIteration (); /// /// public void RunIteration () @@ -335,10 +335,10 @@ namespace Terminal.Gui { RunTimers (); } } - + MainLoopDriver.Iteration (); - bool runIdle = false; + var runIdle = false; lock (_idleHandlersLock) { runIdle = _idleHandlers.Count > 0; } @@ -361,5 +361,24 @@ namespace Terminal.Gui { } _running = prev; } + + /// + /// Stops the main loop driver and calls . + /// + public void Stop () + { + _running = false; + MainLoopDriver.Wakeup (); + } + + /// + public void Dispose () + { + GC.SuppressFinalize (this); + Stop (); + _running = false; + MainLoopDriver?.TearDown (); + MainLoopDriver = null; + } } } diff --git a/UnitTests/Application/MainLoopTests.cs b/UnitTests/Application/MainLoopTests.cs index fffa9fa3a..7b15faad5 100644 --- a/UnitTests/Application/MainLoopTests.cs +++ b/UnitTests/Application/MainLoopTests.cs @@ -613,8 +613,10 @@ namespace Terminal.Gui.ApplicationTests { Application.MainLoop.Invoke (() => { tf.Text = $"index{r.Next ()}"; Interlocked.Increment (ref tbCounter); - if (target == tbCounter) // On last increment wake up the check + if (target == tbCounter) { + // On last increment wake up the check _wakeUp.Set (); + } }); }); }