From 19007644074ef04fa733f19f330f4bd3b3780130 Mon Sep 17 00:00:00 2001
From: NextTurn <45985406+NextTurn@users.noreply.github.com>
Date: Mon, 10 Aug 2020 00:00:00 +0800
Subject: [PATCH] Fix service crashes

---
 src/WinSW.Core/Util/ProcessExtensions.cs | 26 ++++++++++++++++--------
 src/WinSW.Tests/CommandLineTests.cs      |  8 ++++++++
 src/WinSW/ServiceMessages.cs             |  8 ++++++++
 src/WinSW/WrapperService.cs              | 13 +++++++++---
 4 files changed, 43 insertions(+), 12 deletions(-)
 create mode 100644 src/WinSW/ServiceMessages.cs

diff --git a/src/WinSW.Core/Util/ProcessExtensions.cs b/src/WinSW.Core/Util/ProcessExtensions.cs
index f0c9b5f..9d0bcd4 100644
--- a/src/WinSW.Core/Util/ProcessExtensions.cs
+++ b/src/WinSW.Core/Util/ProcessExtensions.cs
@@ -193,20 +193,28 @@ namespace WinSW.Util
             if (!AttachConsole(process.Id))
             {
                 int error = Marshal.GetLastWin32Error();
-                Log.Debug("Failed to attach to console. " + error switch
+                switch (error)
                 {
-                    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
-                });
+                    // The process does not have a console.
+                    case Errors.ERROR_INVALID_HANDLE:
+                        return false;
 
-                return error == Errors.ERROR_INVALID_PARAMETER ? (bool?)null : false;
+                    // The process has exited.
+                    case Errors.ERROR_INVALID_PARAMETER:
+                        return null;
+
+                    // The calling process is already attached to a console.
+                    case Errors.ERROR_ACCESS_DENIED:
+                    default:
+                        Log.Warn("Failed to attach to console. " + new Win32Exception(error).Message);
+                        return false;
+                }
             }
 
-            _ = SetConsoleCtrlHandler(null, true);
+            // Don't call GenerateConsoleCtrlEvent immediately after SetConsoleCtrlHandler.
+            // A delay was observed as of Windows 10, version 2004 and Windows Server 2019.
             _ = GenerateConsoleCtrlEvent(CtrlEvents.CTRL_C_EVENT, 0);
-            _ = SetConsoleCtrlHandler(null, false);
+
             bool succeeded = FreeConsole();
             Debug.Assert(succeeded);
 
diff --git a/src/WinSW.Tests/CommandLineTests.cs b/src/WinSW.Tests/CommandLineTests.cs
index befffd7..b7596a2 100644
--- a/src/WinSW.Tests/CommandLineTests.cs
+++ b/src/WinSW.Tests/CommandLineTests.cs
@@ -38,6 +38,10 @@ namespace WinSW.Tests
                         Assert.Equal(ServiceControllerStatus.Running, controller.Status);
                         Assert.True(controller.CanStop);
 
+                        Assert.EndsWith(
+                            ServiceMessages.StartedSuccessfully + Environment.NewLine,
+                            File.ReadAllText(Path.ChangeExtension(config.FullPath, ".wrapper.log")));
+
                         if (Environment.GetEnvironmentVariable("System.DefinitionId") != null)
                         {
                             session = new InterProcessCodeCoverageSession(Helper.Name);
@@ -48,6 +52,10 @@ namespace WinSW.Tests
                         _ = Helper.Test(new[] { "stop", config.FullPath }, config);
                         controller.Refresh();
                         Assert.Equal(ServiceControllerStatus.Stopped, controller.Status);
+
+                        Assert.EndsWith(
+                            ServiceMessages.StoppedSuccessfully + Environment.NewLine,
+                            File.ReadAllText(Path.ChangeExtension(config.FullPath, ".wrapper.log")));
                     }
                 }
                 finally
diff --git a/src/WinSW/ServiceMessages.cs b/src/WinSW/ServiceMessages.cs
new file mode 100644
index 0000000..f37f2a3
--- /dev/null
+++ b/src/WinSW/ServiceMessages.cs
@@ -0,0 +1,8 @@
+namespace WinSW
+{
+    internal static class ServiceMessages
+    {
+        internal const string StartedSuccessfully = "Service started successfully.";
+        internal const string StoppedSuccessfully = "Service stopped successfully.";
+    }
+}
diff --git a/src/WinSW/WrapperService.cs b/src/WinSW/WrapperService.cs
index 6fd51c7..ddb7b7b 100644
--- a/src/WinSW/WrapperService.cs
+++ b/src/WinSW/WrapperService.cs
@@ -11,6 +11,7 @@ using WinSW.Extensions;
 using WinSW.Logging;
 using WinSW.Native;
 using WinSW.Util;
+using Messages = WinSW.ServiceMessages;
 
 namespace WinSW
 {
@@ -206,7 +207,7 @@ namespace WinSW
             try
             {
                 this.DoStart();
-                this.LogMinimal("Service started successfully.");
+                this.LogMinimal(Messages.StartedSuccessfully);
             }
             catch (Exception e)
             {
@@ -220,7 +221,7 @@ namespace WinSW
             try
             {
                 this.DoStop();
-                this.LogMinimal("Service stopped successfully.");
+                this.LogMinimal(Messages.StoppedSuccessfully);
             }
             catch (Exception e)
             {
@@ -257,6 +258,8 @@ namespace WinSW
         {
             bool succeeded = ConsoleApis.FreeConsole();
             Debug.Assert(succeeded);
+            succeeded = ConsoleApis.SetConsoleCtrlHandler(null, true);
+            Debug.Assert(succeeded);
 
             this.HandleFileCopies();
 
@@ -561,7 +564,9 @@ namespace WinSW
                 }
             }
 
-            bool succeeded = ConsoleApis.AllocConsole();
+            bool succeeded = ConsoleApis.AllocConsole(); // inherited
+            Debug.Assert(succeeded);
+            succeeded = ConsoleApis.SetConsoleCtrlHandler(null, false); // inherited
             Debug.Assert(succeeded);
 
             Process process;
@@ -573,6 +578,8 @@ namespace WinSW
             {
                 succeeded = ConsoleApis.FreeConsole();
                 Debug.Assert(succeeded);
+                succeeded = ConsoleApis.SetConsoleCtrlHandler(null, true);
+                Debug.Assert(succeeded);
             }
 
             Log.Info($"Started process {process.Format()}.");