From f9e0126cf8baad61e9009b47d04aad2e06c20821 Mon Sep 17 00:00:00 2001
From: NextTurn <45985406+NextTurn@users.noreply.github.com>
Date: Mon, 16 Mar 2020 00:00:00 +0800
Subject: [PATCH] Improve signal-related logging

---
 src/Core/WinSWCore/Native/Errors.cs      |  3 ++
 src/Core/WinSWCore/Util/ProcessHelper.cs | 17 +++++-----
 src/Core/WinSWCore/Util/SigIntHelper.cs  | 40 -----------------------
 src/Core/WinSWCore/Util/SignalHelper.cs  | 41 ++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 48 deletions(-)
 delete mode 100644 src/Core/WinSWCore/Util/SigIntHelper.cs
 create mode 100644 src/Core/WinSWCore/Util/SignalHelper.cs

diff --git a/src/Core/WinSWCore/Native/Errors.cs b/src/Core/WinSWCore/Native/Errors.cs
index 82c929a..c52dbe9 100644
--- a/src/Core/WinSWCore/Native/Errors.cs
+++ b/src/Core/WinSWCore/Native/Errors.cs
@@ -2,6 +2,9 @@
 {
     internal static class Errors
     {
+        internal const int ERROR_ACCESS_DENIED = 5;
+        internal const int ERROR_INVALID_HANDLE = 6;
+        internal const int ERROR_INVALID_PARAMETER = 7;
         internal const int ERROR_CANCELLED = 1223;
     }
 }
diff --git a/src/Core/WinSWCore/Util/ProcessHelper.cs b/src/Core/WinSWCore/Util/ProcessHelper.cs
index 51b4ce7..0c22adf 100644
--- a/src/Core/WinSWCore/Util/ProcessHelper.cs
+++ b/src/Core/WinSWCore/Util/ProcessHelper.cs
@@ -64,17 +64,18 @@ namespace winsw.Util
                 return;
             }
 
-            Logger.Info("Send SIGINT " + pid);
-            bool successful = SigIntHelper.SendSIGINTToProcess(proc, stopTimeout);
-            if (successful)
-            {
-                Logger.Info("SIGINT to " + pid + " successful");
-            }
-            else
+            // (bool sent, bool exited)
+            KeyValuePair<bool, bool> result = SignalHelper.SendCtrlCToProcess(proc, stopTimeout);
+            bool exited = result.Value;
+            if (!exited)
             {
                 try
                 {
-                    Logger.Warn("SIGINT to " + pid + " failed - Killing as fallback");
+                    bool sent = result.Key;
+                    if (sent)
+                    {
+                        Logger.Warn("Process " + pid + " did not respond to Ctrl+C signal - Killing as fallback");
+                    }
                     proc.Kill();
                 }
                 catch (Exception ex)
diff --git a/src/Core/WinSWCore/Util/SigIntHelper.cs b/src/Core/WinSWCore/Util/SigIntHelper.cs
deleted file mode 100644
index 8a6feab..0000000
--- a/src/Core/WinSWCore/Util/SigIntHelper.cs
+++ /dev/null
@@ -1,40 +0,0 @@
-using System;
-using System.Diagnostics;
-using System.Runtime.InteropServices;
-using log4net;
-using winsw.Native;
-
-namespace winsw.Util
-{
-    public static class SigIntHelper
-    {
-        private static readonly ILog Logger = LogManager.GetLogger(typeof(SigIntHelper));
-
-        /// <summary>
-        /// Uses the native funciton "AttachConsole" to attach the thread to the executing process to try to trigger a CTRL_C event (SIGINT).  If the application
-        /// doesn't honor the event and shut down gracefully, the. wait period will time out after 15 seconds.
-        /// </summary>
-        /// <param name="process">The process to attach to and send the SIGINT</param>
-        /// <returns>True if the process shut down successfully to the SIGINT, false if it did not.</returns>
-        public static bool SendSIGINTToProcess(Process process, TimeSpan shutdownTimeout)
-        {
-            if (!ConsoleApis.AttachConsole(process.Id))
-            {
-                Logger.Warn("Failed to attach to console. Error code: " + Marshal.GetLastWin32Error());
-                return false;
-            }
-
-            // Disable Ctrl-C handling for our program
-            _ = ConsoleApis.SetConsoleCtrlHandler(null, true);
-            _ = ConsoleApis.GenerateConsoleCtrlEvent(ConsoleApis.CtrlEvents.CTRL_C_EVENT, 0);
-
-            process.WaitForExit((int)shutdownTimeout.TotalMilliseconds);
-
-            // Detach from console. Causes child console process to be automatically closed.
-            bool succeeded = ConsoleApis.FreeConsole();
-            Debug.Assert(succeeded);
-
-            return process.HasExited;
-        }
-    }
-}
diff --git a/src/Core/WinSWCore/Util/SignalHelper.cs b/src/Core/WinSWCore/Util/SignalHelper.cs
new file mode 100644
index 0000000..cffc249
--- /dev/null
+++ b/src/Core/WinSWCore/Util/SignalHelper.cs
@@ -0,0 +1,41 @@
+using System;
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.Diagnostics;
+using System.Runtime.InteropServices;
+using log4net;
+using winsw.Native;
+
+namespace winsw.Util
+{
+    internal static class SignalHelper
+    {
+        private static readonly ILog Logger = LogManager.GetLogger(typeof(SignalHelper));
+
+        // (bool sent, bool exited)
+        internal static KeyValuePair<bool, bool> SendCtrlCToProcess(Process process, TimeSpan shutdownTimeout)
+        {
+            if (!ConsoleApis.AttachConsole(process.Id))
+            {
+                int error = Marshal.GetLastWin32Error();
+                Logger.Warn("Failed to attach to console. " + error switch
+                {
+                    Errors.ERROR_ACCESS_DENIED => "WinSW is already attached to a console.", // TODO: test mode
+                    Errors.ERROR_INVALID_HANDLE => "The process does not have a console.",
+                    Errors.ERROR_INVALID_PARAMETER => "The process has exited.",
+                    _ => new Win32Exception(error).Message // unreachable
+                });
+
+                return new KeyValuePair<bool, bool>(false, error == Errors.ERROR_INVALID_PARAMETER);
+            }
+
+            _ = ConsoleApis.SetConsoleCtrlHandler(null, true);
+            _ = ConsoleApis.GenerateConsoleCtrlEvent(ConsoleApis.CtrlEvents.CTRL_C_EVENT, 0);
+            _ = ConsoleApis.SetConsoleCtrlHandler(null, false);
+            bool succeeded = ConsoleApis.FreeConsole();
+            Debug.Assert(succeeded);
+
+            return new KeyValuePair<bool, bool>(true, process.WaitForExit((int)shutdownTimeout.TotalMilliseconds));
+        }
+    }
+}