Improve window thread termination (#5781)

## Change
Shut down the window thread as part of the `TerminationSignalHandler`
destruction.
Properly unregister the window class after destroying the window on the
thread.
Signal the shutdown via a `WM_DESTROY`.

Author: JohnMcPMS

src/AppInstallerCLICore/Commands/TestCommand.cpp

@@ -13,6 +13,7 @@
 #include "Workflows/MSStoreInstallerHandler.h"
 #include <winget/RepositorySource.h>
 #include <winrt/Microsoft.Management.Configuration.h>
+#include <winrt/Windows.ApplicationModel.Core.h>
 
 using namespace AppInstaller::CLI::Workflow;
 using namespace AppInstaller::Utility::literals;
@@ -74,7 +75,7 @@ namespace AppInstaller::CLI
                     NULL,
                     ENDSESSION_CLOSEAPP,
                     (SMTO_ABORTIFHUNG | SMTO_ERRORONEXIT),
-                    5000,
+                    10000,
                     NULL));
             }
 
@@ -277,6 +278,41 @@ namespace AppInstaller::CLI
                 return module ? module->GetObjectCount() : 0;
             }
         };
+
+        struct TestTerminateTerminationSignalHandler final : public Command
+        {
+            TestTerminateTerminationSignalHandler(std::string_view parent) : Command("term-signal-handler", {}, parent, Visibility::Hidden) {}
+
+            Resource::LocString ShortDescription() const override
+            {
+                return "Test TerminationSignalHandler thread"_lis;
+            }
+
+            Resource::LocString LongDescription() const override
+            {
+                return "Forces the TerminationSignalHandler static object to be destroyed so that the thread behavior can be observed."_lis;
+            }
+
+        protected:
+            void ExecuteInternal(Execution::Context& context) const override
+            {
+                // Destroy the one created by standard execution
+                // We join on the window thread, so if this never exits we have failed the test.
+                winrt::Windows::ApplicationModel::Core::CoreApplication::Properties().TryRemove(L"WindowsPackageManager.TerminationSignalHandler");
+
+                // Create a new instance
+                auto instance = ShutdownMonitoring::TerminationSignalHandler::Instance();
+                
+                if (instance->GetWindowHandle() == nullptr)
+                {
+                    LogAndReport(context, "Didn't get a window handle");
+                }
+                else
+                {
+                    LogAndReport(context, "Got a window handle");
+                }
+            }
+        };
     }
 
     std::vector<std::unique_ptr<Command>> TestCommand::GetCommands() const
@@ -287,6 +323,7 @@ namespace AppInstaller::CLI
                 std::make_unique<TestConfigurationExportCommand>(FullName()),
                 std::make_unique<TestConfigurationFindUnitProcessorsCommand>(FullName()),
                 std::make_unique<TestCanUnloadNowCommand>(FullName()),
+                std::make_unique<TestTerminateTerminationSignalHandler>(FullName()),
             });
     }
 

src/AppInstallerCLICore/ShutdownMonitoring.cpp

@@ -7,6 +7,8 @@
 #include <AppInstallerRuntime.h>
 #include <winget/COMStaticStorage.h>
 
+using namespace std::chrono_literals;
+
 namespace AppInstaller::ShutdownMonitoring
 {
     std::shared_ptr<TerminationSignalHandler> TerminationSignalHandler::Instance()
@@ -102,7 +104,13 @@ namespace AppInstaller::ShutdownMonitoring
         // std::thread requires that any managed thread (joinable) be joined or detached before destructing
         if (m_windowThread.joinable())
         {
-            m_windowThread.detach();
+            if (m_windowHandle)
+            {
+                // Inform the thread that it should stop.
+                PostMessageW(m_windowHandle.get(), WM_DESTROY, 0, 0);
+            }
+
+            m_windowThread.join();
         }
     }
 
@@ -214,6 +222,12 @@ namespace AppInstaller::ShutdownMonitoring
             return;
         }
 
+        // Unregister the window class on exiting the thread
+        auto classUnregister = wil::scope_exit([&]()
+            {
+                UnregisterClassW(windowClass, hInstance);
+            });
+
         m_windowHandle = wil::unique_hwnd(CreateWindow(
             windowClass,
             L"WingetMessageOnlyWindow",
@@ -227,26 +241,38 @@ namespace AppInstaller::ShutdownMonitoring
             hInstance,
             NULL)); /* lpParam */
 
-        if (m_windowHandle == nullptr)
+        HWND windowHandle = m_windowHandle.get();
+        if (windowHandle == nullptr)
         {
             LOG_LAST_ERROR_MSG("Failed creating window");
             return;
         }
 
-        ShowWindow(m_windowHandle.get(), SW_HIDE);
+        // We must destroy the window first so that the class unregister can succeed
+        auto destroyWindow = wil::scope_exit([&]()
+            {
+                DestroyWindow(windowHandle);
+            });
+
+        ShowWindow(windowHandle, SW_HIDE);
 
         // Force message queue to be created.
         MSG msg;
         PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE);
         m_messageQueueReady.SetEvent();
 
-        // Message loop
+        // Message loop, we send WM_DESTROY to terminate it
         BOOL getMessageResult;
-        while ((getMessageResult = GetMessage(&msg, m_windowHandle.get(), 0, 0)) != 0)
+        while ((getMessageResult = GetMessage(&msg, windowHandle, 0, 0)) != 0)
         {
             if (getMessageResult == -1)
             {
                 LOG_LAST_ERROR();
+                break;
+            }
+            else if (msg.message == WM_DESTROY)
+            {
+                break;
             }
             else
             {

src/AppInstallerCLIE2ETests/AppShutdownTests.cs

@@ -121,5 +121,15 @@ public void CanUnloadNowTest()
             Assert.True(lines[3].Contains("Internal objects: 0"));
             Assert.True(lines[4].Contains("External objects: 0"));
         }
+
+        /// <summary>
+        /// Runs winget test term-signal-handler to check for proper thread termination.
+        /// </summary>
+        [Test]
+        public void TermSignalHandler()
+        {
+            var result = TestCommon.RunAICLICommand("test", "term-signal-handler --verbose");
+            Assert.True(result.StdOut.Contains("Got a window handle"));
+        }
     }
 }