Fixed stabilitiy issues and resource leaking (On client disconnection…

…, the Pseudoconsole was kept open forever.)

backlog.md

@@ -1,7 +1,6 @@
 # gsudo Backlog
 
 - Chocolatey package / Scoop package / Release on github.
-- Third consecutive Ctrl-C should ask if the child process must be kept running or killed.
 - gsudo --nocache (service will quit immediately after process ends and will not elevate other commands with cached credentials) (find better --syntax)
 - Allow to specify other username. (RunAsUser verb)
 
@@ -15,6 +14,8 @@
 
 ## Completed
 
+- Third consecutive Ctrl-C or client disconnect kills the elevated process.
+- VT console extended keys (F1-F12, CTRL+?, HOME,PAGEUP)
 - WinPty/VT100 support: When in VT mode, processes are spawn using a PseudoConsole. Rendering could be done using Windows Console ENABLE_VIRTUAL_TERMINAL processing flag but it is pretty [unstable](https://github.com/microsoft/terminal/issues/3765). So it is disabled by default unless you are running inside ConEmu/Cmder which are VT100 ready terminals.
   VT Mode is enabled automatically if you run inside a ConEmu/Cmder or if you use `--vt` flag.
 
@@ -24,13 +25,13 @@
 
 - Configuration settings persistent storage.
 
-```
+``` console
     gsudo config (show all current user config)
     gsudo config {setting} (return current value)
     gsudo config {setting} {value} (save new value)
-        
+
     gsudo config CredentialsCacheDuration 0:0 (no cache)
     gsudo config CredentialsCacheDuration 5:00 (5 minutes)
     gsudo config Prompt "$P# " (elevated command prompt)
     gsudo config CredentialsCache disabled (disable service) -> not implemented
-```
+```

src/gsudo/Commands/RunCommand.cs

@@ -57,8 +57,6 @@ public async Task<int>
                 Environment.SetEnvironmentVariable("PROMPT", GlobalSettings.Prompt.Value);
             }
 
-            Logger.Instance.Log($"Using Console mode {elevationRequest.Mode}", LogLevel.Debug);
-
             if (ProcessExtensions.IsAdministrator() && !GlobalSettings.NewWindow)
             {
                 if (emptyArgs)
@@ -101,9 +99,10 @@ public async Task<int>
             }
             else // IsAdministrator() == false, or build in Debug Mode
             {
-                var cmd = CommandToRun.FirstOrDefault();
+                Logger.Instance.Log($"Using Console mode {elevationRequest.Mode}", LogLevel.Debug);
+                Logger.Instance.Log($"Caller ProcessId is {currentProcess.ParentProcessId()}", LogLevel.Debug);
 
-                Logger.Instance.Log($"Calling ProcessId is {currentProcess.ParentProcessId()}", LogLevel.Debug);
+                var cmd = CommandToRun.FirstOrDefault();
 
                 var rpcClient = GetClient(elevationRequest);
                 Rpc.Connection connection = null;
@@ -151,6 +150,13 @@ public async Task<int>
                 finally
                 {
                     connection?.Dispose();
+                    try
+                    {   
+                        // cleanup console before returning.
+                        Console.CursorVisible = true;
+                        Console.ResetColor();
+                    }
+                    catch { }
                 }
             }
 

src/gsudo/Commands/ServiceCommand.cs

@@ -61,7 +61,7 @@ private async Task AcceptConnection(Connection connection)
             catch (Exception e)
             {
                 Logger.Instance.Log(e.ToString(), LogLevel.Error);
-                connection.IsAlive = false;
+                connection.SignalDisconnected();
             }
         }
 

src/gsudo/Helpers/ProcessExtensions.cs

@@ -8,6 +8,7 @@
 using System.Security;
 using System.Security.Permissions;
 using System.Security.Principal;
+using System.Threading;
 
 namespace gsudo.Helpers
 {
@@ -188,5 +189,42 @@ protected override bool ReleaseHandle()
         }
 
         #endregion
+
+        public static void Terminate(this Process process)
+        {
+            if (process.HasExited) return;
+
+            Logger.Instance.Log($"Killing process {process.Id} {process.ProcessName}", LogLevel.Debug);
+
+            process.SendCtrlC(false);
+            process.CloseMainWindow();
+
+            process.WaitForExit(300);
+
+            if (!process.HasExited)
+            {
+                process.Kill();
+                /*
+                var p = Process.Start(new ProcessStartInfo()
+                {
+                    FileName = "taskkill",
+                    Arguments = $"/PID {process.Id} /T",    
+                    WindowStyle = ProcessWindowStyle.Hidden
+
+                });
+                p.WaitForExit();
+                */
+            }
+        }
+
+        /// <summary>
+        /// Get an AutoResetEvent that signals when the process exits
+        /// </summary>
+        public static AutoResetEvent GetWaitHandle(this Process process) =>
+            new AutoResetEvent(false)
+            {
+                SafeWaitHandle = new SafeWaitHandle(process.Handle, ownsHandle: false)
+            };
+
     }
 }

src/gsudo/Helpers/ProcessFactory.cs

@@ -142,11 +142,11 @@ public static string FindExecutableInPath(string exe)
             return null;
         }
 
-        public static PseudoConsole.Process StartPseudoConsole(string command, IntPtr attributes, IntPtr hPC, string startFolder)
+        public static PseudoConsole.PseudoConsoleProcess StartPseudoConsole(string command, IntPtr attributes, IntPtr hPC, string startFolder)
         {
             var startupInfo = ConfigureProcessThread(hPC, attributes);
             var processInfo = RunProcess(ref startupInfo, command, startFolder);
-            return new PseudoConsole.Process(startupInfo, processInfo);
+            return new PseudoConsole.PseudoConsoleProcess(startupInfo, processInfo);
         }
 
         private static STARTUPINFOEX ConfigureProcessThread(IntPtr hPC, IntPtr attributes)

src/gsudo/ProcessHosts/PipedProcessHost.cs

@@ -1,10 +1,10 @@
 using gsudo.Helpers;
-using gsudo.Native;
 using gsudo.Rpc;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace gsudo.ProcessHosts
@@ -32,11 +32,8 @@ public async Task Start(Connection connection, ElevationRequest request)
                 var t4 = new StreamReader(connection.ControlStream, GlobalSettings.Encoding).ConsumeOutput((s) => HandleControl(s, process));
 
                 int i = 0;
-
-                while (!process.WaitForExit(0) && connection.IsAlive)
-                {
-                    await Task.Delay(10).ConfigureAwait(false);
-                }
+
+                WaitHandle.WaitAny(new WaitHandle[] { process.GetWaitHandle(), connection.DisconnectedWaitHandle });
 
                 if (process.HasExited && connection.IsAlive)
                 {
@@ -47,10 +44,6 @@ public async Task Start(Connection connection, ElevationRequest request)
                     await Task.WhenAll(t1, t2).ConfigureAwait(false);
                     await connection.ControlStream.WriteAsync($"{Constants.TOKEN_EXITCODE}{process.ExitCode}{Constants.TOKEN_EXITCODE}").ConfigureAwait(false);
                 }
-                else
-                {
-                    TerminateHostedProcess();
-                }
 
                 await connection.FlushAndCloseAll().ConfigureAwait(false);
             }
@@ -63,6 +56,10 @@ public async Task Start(Connection connection, ElevationRequest request)
             }
             finally
             {
+                if (process != null && !process.HasExited)
+                {
+                    process?.Terminate();
+                }
                 process?.Dispose();
             }
         }
@@ -84,30 +81,6 @@ private bool ShouldWait(StreamReader streamReader)
             }
         }
 
-        private void TerminateHostedProcess()
-        {
-            Logger.Instance.Log($"Killing process {process.Id} {process.ProcessName}", LogLevel.Debug);
-
-            if (process.HasExited) return;
-
-            process.SendCtrlC(true);
-
-            if (process.CloseMainWindow())
-                process.WaitForExit(100);
-
-            //if (!process.HasExited)
-            //{
-            //    var p = Process.Start(new ProcessStartInfo()
-            //    {
-            //        FileName = "taskkill",
-            //        Arguments = $"/PID {process.Id} /T",
-            //        WindowStyle = ProcessWindowStyle.Hidden
-
-            //    });
-            //    p.WaitForExit();
-            //}   
-        }
-
         private async Task WriteToProcessStdIn(string s, Process process)
         {
             if (lastInboundMessage == null)
@@ -166,7 +139,7 @@ private async Task WriteToPipe(string s)
                     s = s.Substring(c);
                     lastInboundMessage = lastInboundMessage.Substring(c);
                 }
-                if (GlobalSettings.Debug && !string.IsNullOrEmpty(s)) Logger.Instance.Log($"Last input command was: {s}", LogLevel.Debug);
+                //if (GlobalSettings.Debug && !string.IsNullOrEmpty(s)) Logger.Instance.Log($"Last input command was: {s}", LogLevel.Debug);
 
             }
             if (string.IsNullOrEmpty(s)) return; // suppress chars n s;

src/gsudo/ProcessHosts/VTProcessHost.cs

@@ -21,6 +21,7 @@ public async Task Start(Connection connection, ElevationRequest request)
             int? exitCode;
             Task t1 = null, t2 = null, t3=null;
             _connection = connection;
+            System.Diagnostics.Process runningProcess = null;
             try
             {
                 string command = request.FileName + " " + request.Arguments;
@@ -31,6 +32,8 @@ public async Task Start(Connection connection, ElevationRequest request)
                     {
                         using (var process = ProcessFactory.StartPseudoConsole(command, PseudoConsole.PseudoConsole.PseudoConsoleThreadAttribute, pseudoConsole.Handle, request.StartFolder))
                         {
+                            runningProcess = System.Diagnostics.Process.GetProcessById(process.ProcessInfo.dwProcessId);
+
                             // copy all pseudoconsole output to stdout
                             t1 = Task.Run(() => CopyPipeToOutput(outputPipe.ReadSide));
                             // prompt for stdin input and send the result to the pseudoconsole
@@ -43,12 +46,9 @@ public async Task Start(Connection connection, ElevationRequest request)
                             // var t3 = new StreamReader(pipe, Globals.Encoding).ConsumeOutput((s) => WriteToStdInput(s, process));
 
                             OnClose(() => DisposeResources(process, pseudoConsole, outputPipe, inputPipe));
-                            WaitForExit(process).WaitOne();
 
-                            if (connection.IsAlive)
-                            {
-                                await Task.Delay(10).ConfigureAwait(false);
-                            }
+                            WaitHandle.WaitAny(new WaitHandle[] { process.GetWaitHandle(), connection.DisconnectedWaitHandle });
+
                             exitCode = process.GetExitCode();
                         }
                     }
@@ -68,6 +68,11 @@ public async Task Start(Connection connection, ElevationRequest request)
                 await connection.FlushAndCloseAll().ConfigureAwait(false);
                 return;
             }
+            finally
+            {
+                if (runningProcess != null && !runningProcess.HasExited)
+                    runningProcess.Terminate();
+            }
         }
 
         /// <summary>
@@ -111,42 +116,43 @@ private async Task CopyPipeToOutput(SafeFileHandle outputReadSide)
 
             if (GlobalSettings.Debug)
             {
-                streamWriter = new StreamWriter(new FileStream("VTProcessHost.debug.txt", FileMode.Create, FileAccess.Write), new System.Text.UTF8Encoding(true));
+                try
+                {
+                    streamWriter = new StreamWriter(new FileStream("VTProcessHost.debug.txt", FileMode.Create, FileAccess.Write), new System.Text.UTF8Encoding(true));
+                }
+                catch { /* if debug stream fails, lets go on. */ }
             }
 
-            using (var pseudoConsoleOutput = new FileStream(outputReadSide, FileAccess.Read))
+            try
             {
-                byte[] buffer = new byte[10240];
-                int cch;
-
-                while ((cch = await pseudoConsoleOutput.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false)) > 0)
+                using (var pseudoConsoleOutput = new FileStream(outputReadSide, FileAccess.Read))
                 {
-                    var s = GlobalSettings.Encoding.GetString(buffer, 0, cch);
-                    await _connection.DataStream.WriteAsync(s).ConfigureAwait(false);
-
-                    streamWriter?.Write(s);
-                    streamWriter?.Flush();
-
-                    if (GlobalSettings.Debug)
-                        Console.Write(s
-                            .Replace('\a',' ') //  no bell sounds please
-                            .Replace("\r", "\\r")
-                            .Replace("\n", "\\n")
-                            );
+                    byte[] buffer = new byte[10240];
+                    int cch;
+
+                    while ((cch = await pseudoConsoleOutput.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false)) > 0)
+                    {
+                        var s = GlobalSettings.Encoding.GetString(buffer, 0, cch);
+                        await _connection.DataStream.WriteAsync(s).ConfigureAwait(false);
+
+                        streamWriter?.Write(s);
+                        streamWriter?.Flush();
+
+                        if (GlobalSettings.Debug)
+                            Console.Write(s
+                                .Replace('\a', ' ') //  no bell sounds please
+                                .Replace("\r", "\\r")
+                                .Replace("\n", "\\n")
+                                );
+                    }
                 }
             }
-            streamWriter?.Close();
+            finally
+            { 
+                streamWriter?.Close();
+            }
         }
-
-        /// <summary>
-        /// Get an AutoResetEvent that signals when the process exits
-        /// </summary>
-        private static AutoResetEvent WaitForExit(Process process) =>
-            new AutoResetEvent(false)
-            {
-                SafeWaitHandle = new SafeWaitHandle(process.ProcessInfo.hProcess, ownsHandle: false)
-            };
-
+
         /// <summary>
         /// Set a callback for when the terminal is closed (e.g. via the "X" window decoration button).
         /// Intended for resource cleanup logic.
@@ -163,7 +169,7 @@ private static void OnClose(Action handler)
             }, true);
         }
 
-        private void DisposeResources(params IDisposable[] disposables)
+        private static void DisposeResources(params IDisposable[] disposables)
         {
             foreach (var disposable in disposables)
             {

src/gsudo/Program.cs

@@ -3,9 +3,7 @@
 using gsudo.Helpers;
 using System;
 using System.Collections.Generic;
-using System.IO;
 using System.Linq;
-using System.Reflection;
 using System.Threading.Tasks;
 
 namespace gsudo