commit 7d4f839752f42e87304205084c654b5ff67cd35a parent aeb5284c1193784e39d1828b5a761847be6ce998 Author: Ruben Guerrero <rubengu@microsoft.com> Date: Mon, 10 Jul 2023 10:21:34 -0700 Support winget installing AppInstaller (#3377) When winget is updating AppInstaller (aka. itself) via winget install 9NBLGGH4NNS1 -s msstore --force if appears the installation failed but in reality, it got updated successfully. winget install 9NBLGGH4NNS1 -s msstore --force Verifying/Requesting package acquisition... Starting package install... ███████████████████████████ 90% Failed to install or upgrade Microsoft Store package. Error code: 0x80073d02 The failure means ERROR_PACKAGES_IN_USE and from my last PR #3299 I believe that error will go away because of --force, but a new one will occur. I need a real AppInstaller package to confirm this but because reasons I cannot build it right now. Regardless, the correct fix it to listen to messages from the lifetime manager. When the system is updating an app, the app lifetime manager will ask any the app to terminate if its running. It does it by: Sending a WM_QUERYENDSESSION message to any app window. Sending a CTRL-C signal. This means that winget.exe needs to start listening to window messages because there's no real difference between the app lifetime manager and a user doing CTRL-C. This PR creates a hidden window and start the message loop. When WM_QUERYENDSESSION is received, we will ask to cancel any progress (as it currently does with CTRL-C) stating the reason as AppShutdown. By design, we are not going to cancel installations as maybe they get completed before they terminate us. Any other operation will be cancelled. If the context gets cancelled with an AppShutdown reason while installing AppInstaller we will just set the progress as 100% and be done with it as we are going to get terminated soon. Diffstat:
31 files changed, 590 insertions(+), 83 deletions(-)
diff --git a/.github/actions/spelling/excludes.txt b/.github/actions/spelling/excludes.txt @@ -90,4 +90,4 @@ # Because it doesn't handle argument -Words well ^tools/CorrelationTestbed/.*\.ps1$ ^tools/COMTrace/ComTrace.wprp$ -ignore$- \ No newline at end of file +ignore$ diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt @@ -1,4 +1,5 @@ abcd +ABORTIFHUNG accepteula adjacents adml @@ -17,6 +18,8 @@ apiset appinstallertest applic appname +appshutdown +APPTERMINATION argumentlist ARMNT arp @@ -65,6 +68,7 @@ cgi cinq CLASSNOTREG CLIE +CLOSEAPP cloudapp cls clsctx @@ -111,8 +115,10 @@ ecfrbrowse EFGH EFile endregion +ENDSESSION EQU errmsg +ERRORONEXIT ESRB etest etl @@ -162,6 +168,7 @@ hmodule Howto hre hresults +hwnd IARP IAttachment ICONDIR @@ -216,33 +223,38 @@ liv liwpx localizationpriority LOWORD +LPARAM LPBYTE LPCWSTR LPDWORD +lpfn LPGRPICONDIR LPGRPICONDIRENTRY LPICONDIR LPICONDIRENTRY LPICONIMAGE lpitemidlist -LPW -maclachlan LPSTR +lpsz +LPW LPWCH LPWSTR +LRESULT LSTATUS LTDA luffy Luffytaro +maclachlan malware mapview +Maxed maxvalue maybenull -Maxed MBH mdmp MDs megamorf +meme midl minexample minidump @@ -276,12 +288,14 @@ NETSDK Newtonsoft NNS NOAGGREGATION +NOCLOSE NOCRLF NOEXPAND NOLINKINFO nonetwork NONFOLDERS nonterminated +NOREMOVE normer NOSEARCH NOSEPARATOR @@ -297,9 +311,10 @@ objbase objidl ofile ools -osfhandle OPTOUT +osfhandle Outptr +OVERLAPPEDWINDOW packageinuse packageinusebyapplication PACL @@ -342,6 +357,7 @@ pvm pwabuilder PWAs PWSTR +QUERYENDSESSION qword rebootinitiated rebootrequiredforinstall @@ -364,8 +380,8 @@ riid roblox ronomon rosoft -roy rowids +roy RRF rrr runspace @@ -388,6 +404,7 @@ Sideload SIGNATUREHASH Sku SLAPI +SMTO sortof sourceforge spamming @@ -460,6 +477,7 @@ VERSIE vns vsconfig vstest +wcex webpages Webserver websites @@ -481,6 +499,9 @@ winreg winrtact winstring withstarts +Wnd +WNDCLASSEX +WPARAM wpr wprp wputenv diff --git a/doc/windows/package-manager/winget/returnCodes.md b/doc/windows/package-manager/winget/returnCodes.md @@ -117,6 +117,7 @@ ms.localizationpriority: medium | 0x8A150067 | -1978335129 | APPINSTALLER_CLI_ERROR_NOT_ALL_QUERIES_FOUND_SINGLE | One or more queries did not return exactly one match | | 0x8A150068 | -1978335128 | APPINSTALLER_CLI_ERROR_PACKAGE_IS_PINNED | The package has a pin that prevents upgrade. | | 0x8A150069 | -1978335127 | APPINSTALLER_CLI_ERROR_PACKAGE_IS_STUB | The package currently installed is the stub package | +| 0x8A15006A | -1978335126 | APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED | Application shutdown signal received | ## Install errors. diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -358,6 +358,7 @@ <ClInclude Include="Commands\InstallCommand.h" /> <ClInclude Include="Commands\RootCommand.h" /> <ClInclude Include="Commands\SourceCommand.h" /> + <ClInclude Include="Commands\TestCommand.h" /> <ClInclude Include="Commands\UninstallCommand.h" /> <ClInclude Include="Commands\UpgradeCommand.h" /> <ClInclude Include="Commands\ValidateCommand.h" /> @@ -414,6 +415,7 @@ <ClCompile Include="Commands\DebugCommand.cpp" /> <ClCompile Include="Commands\ImportCommand.cpp" /> <ClCompile Include="Commands\PinCommand.cpp" /> + <ClCompile Include="Commands\TestCommand.cpp" /> <ClCompile Include="ConfigurationContext.cpp" /> <ClCompile Include="ConfigurationSetProcessorFactoryRemoting.cpp" /> <ClCompile Include="ContextOrchestrator.cpp" /> diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters @@ -221,6 +221,9 @@ <ClInclude Include="Commands\DebugCommand.h"> <Filter>Commands</Filter> </ClInclude> + <ClInclude Include="Commands\TestCommand.h"> + <Filter>Commands</Filter> + </ClInclude> </ItemGroup> <ItemGroup> <ClCompile Include="pch.cpp"> @@ -409,6 +412,9 @@ <ClCompile Include="Commands\DebugCommand.cpp"> <Filter>Commands</Filter> </ClCompile> + <ClCompile Include="Commands\TestCommand.cpp"> + <Filter>Commands</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> diff --git a/src/AppInstallerCLICore/Commands/RootCommand.cpp b/src/AppInstallerCLICore/Commands/RootCommand.cpp @@ -22,6 +22,7 @@ #include "PinCommand.h" #include "ConfigureCommand.h" #include "DebugCommand.h" +#include "TestCommand.h" #include "Resources.h" #include "TableOutput.h" @@ -179,6 +180,9 @@ namespace AppInstaller::CLI #if _DEBUG std::make_unique<DebugCommand>(FullName()), #endif +#ifndef AICLI_DISABLE_TEST_HOOKS + std::make_unique<TestCommand>(FullName()), +#endif }); } diff --git a/src/AppInstallerCLICore/Commands/TestCommand.cpp b/src/AppInstallerCLICore/Commands/TestCommand.cpp @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" + +#ifndef AICLI_DISABLE_TEST_HOOKS + +#include "TestCommand.h" + +namespace AppInstaller::CLI +{ + namespace + { + void LogAndReport(Execution::Context& context, const std::string& message) + { + AICLI_LOG(CLI, Info, << message); + context.Reporter.Info() << message << std::endl; + } + } + + std::vector<std::unique_ptr<Command>> TestCommand::GetCommands() const + { + return InitializeFromMoveOnly<std::vector<std::unique_ptr<Command>>>({ + std::make_unique<TestAppShutdownCommand>(FullName()), + }); + } + + void TestCommand::ExecuteInternal(Execution::Context& context) const + { + UNREFERENCED_PARAMETER(context); + Sleep(INFINITE); + } + + Resource::LocString TestCommand::ShortDescription() const + { + return Utility::LocIndString("Waits infinitely"sv); + } + + Resource::LocString TestCommand::LongDescription() const + { + return Utility::LocIndString("Waits infinitely. Use this if you want winget to wait forever while something is going on"sv); + } + + std::vector<Argument> TestAppShutdownCommand::GetArguments() const + { + return { + Argument::ForType(Execution::Args::Type::Force) + }; + } + + void TestAppShutdownCommand::ExecuteInternal(Execution::Context& context) const + { + auto windowHandle = Execution::GetWindowHandle(); + + if (windowHandle == NULL) + { + LogAndReport(context, "Window was not created"); + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR); + } + + if (context.Args.Contains(Execution::Args::Type::Force)) + { + LogAndReport(context, "Sending WM_QUERYENDSESSION message"); + THROW_LAST_ERROR_IF(!SendMessageTimeout( + windowHandle, + WM_QUERYENDSESSION, + NULL, + ENDSESSION_CLOSEAPP, + (SMTO_ABORTIFHUNG | SMTO_ERRORONEXIT), + 5000, + NULL)); + } + + LogAndReport(context, "Waiting for app shutdown event"); + bool result = Execution::WaitForAppShutdownEvent(); + if (!result) + { + LogAndReport(context, "Failed getting app shutdown event"); + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR); + } + + LogAndReport(context, "Succeeded waiting for app shutdown event"); + } + + Resource::LocString TestAppShutdownCommand::ShortDescription() const + { + return Utility::LocIndString("Test command to verify appshutdown event."sv); + } + + Resource::LocString TestAppShutdownCommand::LongDescription() const + { + return Utility::LocIndString("Test command for appshutdown. Verifies the window was created and waits for the app shutdown event"sv); + } + +} + +#endif diff --git a/src/AppInstallerCLICore/Commands/TestCommand.h b/src/AppInstallerCLICore/Commands/TestCommand.h @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Command.h" + +#ifndef AICLI_DISABLE_TEST_HOOKS + +namespace AppInstaller::CLI +{ + // Command: winget test + // Convenient command for debugging. Waits infinitely. + // Use this if you want to debug things that happen out side of workflows or modify locally to do whatever you need. + struct TestCommand final : public Command + { + TestCommand(std::string_view parent) : Command("test", {}, parent, Visibility::Hidden) {} + + std::vector<std::unique_ptr<Command>> GetCommands() const override; + + Resource::LocString ShortDescription() const override; + Resource::LocString LongDescription() const override; + + protected: + void ExecuteInternal(Execution::Context& context) const override; + }; + + // Command: winget test appshutdown + // Verifies the window was created and waits for the app shutdown event. + // Used in E2E. + struct TestAppShutdownCommand final : public Command + { + TestAppShutdownCommand(std::string_view parent) : Command("appshutdown", {}, parent, Visibility::Hidden) {} + + std::vector<Argument> GetArguments() const override; + + Resource::LocString ShortDescription() const override; + Resource::LocString LongDescription() const override; + + protected: + void ExecuteInternal(Execution::Context& context) const override; + }; +} + +#endif diff --git a/src/AppInstallerCLICore/ContextOrchestrator.cpp b/src/AppInstallerCLICore/ContextOrchestrator.cpp @@ -115,7 +115,7 @@ namespace AppInstaller::CLI::Execution void ContextOrchestrator::CancelQueueItem(const OrchestratorQueueItem& item) { // Always cancel the item, even if it isn't running yet, to get the terminationHR set correctly. - item.GetContext().Cancel(false, true); + item.GetContext().Cancel(CancelReason::Abort, true); RemoveItemInState(item, OrchestratorQueueItemState::Queued); } @@ -271,7 +271,7 @@ namespace AppInstaller::CLI::Execution command->ValidateArguments(item->GetContext().Args); - item->GetContext().EnableCtrlHandler(); + item->GetContext().EnableSignalTerminationHandler(); ::AppInstaller::CLI::ExecuteWithoutLoggingSuccess(item->GetContext(), command.get()); } @@ -284,7 +284,7 @@ namespace AppInstaller::CLI::Execution item->GetContext().SetTerminationHR(exceptionHR); } - item->GetContext().EnableCtrlHandler(false); + item->GetContext().EnableSignalTerminationHandler(false); if (FAILED(item->GetContext().GetTerminationHR()) || item->IsComplete()) { diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp @@ -63,7 +63,6 @@ namespace AppInstaller::CLI Execution::Context context{ std::cout, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); - context.EnableCtrlHandler(); // Enable all logging for this phase; we will update once we have the arguments Logging::Log().EnableChannel(Logging::Channel::All); @@ -85,6 +84,8 @@ namespace AppInstaller::CLI Logging::FileLogger::BeginCleanup(); } + context.EnableSignalTerminationHandler(); + context << Workflow::ReportExecutionStage(Workflow::ExecutionStage::ParseArgs); // Convert incoming wide char args to UTF8 diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -12,12 +12,12 @@ namespace AppInstaller::CLI::Execution namespace { - // Type to contain the CTRL signal handler. - struct CtrlHandler + // Type to contain the CTRL signal and window messages handler. + struct SignalTerminationHandler { - static CtrlHandler& Instance() + static SignalTerminationHandler& Instance() { - static CtrlHandler s_instance; + static SignalTerminationHandler s_instance; return s_instance; } @@ -39,10 +39,53 @@ namespace AppInstaller::CLI::Execution m_contexts.erase(itr); } + void StartAppShutdown() + { + // Lifetime manager sends CTRL-C after the WM_QUERYENDSESSION is processed. + // If we disable the CTRL-C handler, the default handler will kill us. + TerminateContexts(CancelReason::AppShutdown, true); + +#ifndef AICLI_DISABLE_TEST_HOOKS + m_appShutdownEvent.SetEvent(); +#endif + } + +#ifndef AICLI_DISABLE_TEST_HOOKS + HWND GetWindowHandle() { return m_windowHandle.get(); } + + bool WaitForAppShutdownEvent() + { + return m_appShutdownEvent.wait(60000); + } +#endif + private: - CtrlHandler() + SignalTerminationHandler() { + // Create message only window. + m_messageQueueReady.create(); + m_windowThread = std::thread(&SignalTerminationHandler::CreateWindowAndStartMessageLoop, this); + if (!m_messageQueueReady.wait(100)) + { + AICLI_LOG(CLI, Warning, << "Timeout creating winget window"); + } + + // Set up ctrl-c handler. LOG_IF_WIN32_BOOL_FALSE(SetConsoleCtrlHandler(StaticCtrlHandlerFunction, TRUE)); + +#ifndef AICLI_DISABLE_TEST_HOOKS + m_appShutdownEvent.create(); +#endif + } + + ~SignalTerminationHandler() + { + // At this point the thread is gone, but it will get angry + // if there's no call to join. + if (m_windowThread.joinable()) + { + m_windowThread.join(); + } } static BOOL WINAPI StaticCtrlHandlerFunction(DWORD ctrlType) @@ -50,19 +93,43 @@ namespace AppInstaller::CLI::Execution return Instance().CtrlHandlerFunction(ctrlType); } + static LRESULT WINAPI WindowMessageProcedure(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) + { + AICLI_LOG(CLI, Verbose, << "Received window message type: " << uMsg); + switch (uMsg) + { + case WM_QUERYENDSESSION: + SignalTerminationHandler::Instance().StartAppShutdown(); + return TRUE; + case WM_ENDSESSION: + case WM_CLOSE: + DestroyWindow(hWnd); + break; + case WM_DESTROY: + PostQuitMessage(0); + break; + default: + return DefWindowProc(hWnd, uMsg, wParam, lParam); + } + return FALSE; + } + BOOL CtrlHandlerFunction(DWORD ctrlType) { + // TODO: Move this to be logged per active context when we have thread static globals + AICLI_LOG(CLI, Info, << "Got CTRL type: " << ctrlType); + switch (ctrlType) { case CTRL_C_EVENT: case CTRL_BREAK_EVENT: - return TerminateContexts(ctrlType, false); + return TerminateContexts(CancelReason::CtrlCSignal, false); // According to MSDN, we should never receive these due to having gdi32/user32 loaded in our process. // But handle them as a force terminate anyway. case CTRL_CLOSE_EVENT: case CTRL_LOGOFF_EVENT: case CTRL_SHUTDOWN_EVENT: - return TerminateContexts(ctrlType, true); + return TerminateContexts(CancelReason::CtrlCSignal, true); default: return FALSE; } @@ -70,7 +137,7 @@ namespace AppInstaller::CLI::Execution // Terminates the currently attached contexts. // Returns FALSE if no contexts attached; TRUE otherwise. - BOOL TerminateContexts(DWORD ctrlType, bool force) + BOOL TerminateContexts(CancelReason reason, bool force) { if (m_contexts.empty()) { @@ -79,43 +146,113 @@ namespace AppInstaller::CLI::Execution { std::lock_guard<std::mutex> lock{ m_contextsLock }; - - // TODO: Move this to be logged per active context when we have thread static globals - AICLI_LOG(CLI, Info, << "Got CTRL type: " << ctrlType); - for (auto& context : m_contexts) { - context->Cancel(true, force); + context->Cancel(reason, force); } } return TRUE; } + void CreateWindowAndStartMessageLoop() + { + PCWSTR windowClass = L"wingetWindow"; + HINSTANCE hInstance = GetModuleHandle(NULL); + if (hInstance == NULL) + { + LOG_LAST_ERROR_MSG("Failed getting module handle"); + return; + } + + WNDCLASSEX wcex = {}; + wcex.cbSize = sizeof(wcex); + + wcex.style = CS_NOCLOSE; + wcex.lpfnWndProc = SignalTerminationHandler::WindowMessageProcedure; + wcex.cbClsExtra = 0; + wcex.cbWndExtra = 0; + wcex.hInstance = hInstance; + wcex.lpszClassName = windowClass; + + if (!RegisterClassEx(&wcex)) + { + LOG_LAST_ERROR_MSG("Failed registering window class"); + return; + } + + m_windowHandle = wil::unique_hwnd(CreateWindow( + windowClass, + L"WingetMessageOnlyWindow", + WS_OVERLAPPEDWINDOW, + 0, /* x */ + 0, /* y */ + 0, /* nWidth */ + 0, /* nHeight */ + NULL, /* hWndParent */ + NULL, /* hMenu */ + hInstance, + NULL)); /* lpParam */ + + if (m_windowHandle == nullptr) + { + LOG_LAST_ERROR_MSG("Failed creating window"); + return; + } + + ShowWindow(m_windowHandle.get(), SW_HIDE); + + // Force message queue to be created. + MSG msg; + PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE); + m_messageQueueReady.SetEvent(); + + // Message loop + BOOL getMessageResult; + while ((getMessageResult = GetMessage(&msg, m_windowHandle.get(), 0, 0)) != 0) + { + if (getMessageResult == -1) + { + LOG_LAST_ERROR(); + } + else + { + DispatchMessage(&msg); + } + } + } + +#ifndef AICLI_DISABLE_TEST_HOOKS + wil::unique_event m_appShutdownEvent; +#endif + std::mutex m_contextsLock; std::vector<Context*> m_contexts; + wil::unique_event m_messageQueueReady; + wil::unique_hwnd m_windowHandle; + std::thread m_windowThread; }; - void SetCtrlHandlerContext(bool add, Context* context) + void SetSignalTerminationHandlerContext(bool add, Context* context) { THROW_HR_IF(E_POINTER, context == nullptr); if (add) { - CtrlHandler::Instance().AddContext(context); + SignalTerminationHandler::Instance().AddContext(context); } else { - CtrlHandler::Instance().RemoveContext(context); + SignalTerminationHandler::Instance().RemoveContext(context); } } } Context::~Context() { - if (m_disableCtrlHandlerOnExit) + if (m_disableSignalTerminationHandlerOnExit) { - EnableCtrlHandler(false); + EnableSignalTerminationHandler(false); } } @@ -125,9 +262,9 @@ namespace AppInstaller::CLI::Execution clone->m_flags = m_flags; clone->m_executingCommand = m_executingCommand; // If the parent is hooked up to the CTRL signal, have the clone be as well - if (m_disableCtrlHandlerOnExit) + if (m_disableSignalTerminationHandlerOnExit) { - clone->EnableCtrlHandler(); + clone->EnableSignalTerminationHandler(); } CopyArgsToSubContext(clone.get()); return clone; @@ -149,10 +286,10 @@ namespace AppInstaller::CLI::Execution } } - void Context::EnableCtrlHandler(bool enabled) + void Context::EnableSignalTerminationHandler(bool enabled) { - SetCtrlHandlerContext(enabled, this); - m_disableCtrlHandlerOnExit = enabled; + SetSignalTerminationHandlerContext(enabled, this); + m_disableSignalTerminationHandlerOnExit = enabled; } void Context::UpdateForArgs() @@ -193,11 +330,18 @@ namespace AppInstaller::CLI::Execution std::exit(hr); } } + else if (hr == APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED) + { + AICLI_LOG(CLI, Info, << "Got app termination signal"); + hr = E_ABORT; + } Logging::Telemetry().LogCommandTermination(hr, file, line); - m_isTerminated = true; - m_terminationHR = hr; + if (!m_isTerminated) + { + SetTerminationHR(hr); + } } void Context::SetTerminationHR(HRESULT hr) @@ -206,10 +350,20 @@ namespace AppInstaller::CLI::Execution m_isTerminated = true; } - void Context::Cancel(bool exitIfStuck, bool bypassUser) + void Context::Cancel(CancelReason reason, bool bypassUser) { - Terminate(exitIfStuck ? APPINSTALLER_CLI_ERROR_CTRL_SIGNAL_RECEIVED : E_ABORT); - Reporter.CancelInProgressTask(bypassUser); + HRESULT hr = E_ABORT; + if (reason == CancelReason::CtrlCSignal) + { + hr = APPINSTALLER_CLI_ERROR_CTRL_SIGNAL_RECEIVED; + } + else if (reason == CancelReason::AppShutdown) + { + hr = APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED; + } + + Terminate(hr); + Reporter.CancelInProgressTask(bypassUser, reason); } void Context::SetExecutionStage(Workflow::ExecutionStage stage) @@ -242,5 +396,15 @@ namespace AppInstaller::CLI::Execution { return (m_shouldExecuteWorkflowTask ? m_shouldExecuteWorkflowTask(task) : true); } + + HWND GetWindowHandle() + { + return SignalTerminationHandler::Instance().GetWindowHandle(); + } + + bool WaitForAppShutdownEvent() + { + return SignalTerminationHandler::Instance().WaitForAppShutdownEvent(); + } #endif } diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h @@ -69,6 +69,12 @@ namespace AppInstaller::CLI::Execution DEFINE_ENUM_FLAG_OPERATORS(ContextFlag); +#ifndef AICLI_DISABLE_TEST_HOOKS + HWND GetWindowHandle(); + + bool WaitForAppShutdownEvent(); +#endif + // The context within which all commands execute. // Contains input/output via Execution::Reporter and // arguments via Execution::Args. @@ -92,8 +98,8 @@ namespace AppInstaller::CLI::Execution // Creates a child of this context. virtual std::unique_ptr<Context> CreateSubContext(); - // Enables reception of CTRL signals. - void EnableCtrlHandler(bool enabled = true); + // Enables reception of CTRL signals and window messages. + void EnableSignalTerminationHandler(bool enabled = true); // Applies changes based on the parsed args. void UpdateForArgs(); @@ -114,9 +120,9 @@ namespace AppInstaller::CLI::Execution void SetTerminationHR(HRESULT hr); // Cancel the context; this terminates it as well as informing any in progress task to stop cooperatively. - // Multiple attempts with exitIfStuck == true may cause the process to simply exit. + // Multiple attempts with CancelReason::CancelSignal may cause the process to simply exit. // The bypassUser indicates whether the user should be asked for cancellation (does not currently have any effect). - void Cancel(bool exitIfStuck = false, bool bypassUser = false); + void Cancel(CancelReason reason, bool bypassUser = false); // Gets context flags ContextFlag GetFlags() const @@ -164,7 +170,7 @@ namespace AppInstaller::CLI::Execution std::function<bool(const Workflow::WorkflowTask&)> m_shouldExecuteWorkflowTask; private: - DestructionToken m_disableCtrlHandlerOnExit = false; + DestructionToken m_disableSignalTerminationHandlerOnExit = false; bool m_isTerminated = false; HRESULT m_terminationHR = S_OK; size_t m_CtrlSignalCount = 0; diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -294,7 +294,7 @@ namespace AppInstaller::CLI::Execution m_progressCallback = callback; } - void Reporter::CancelInProgressTask(bool force) + void Reporter::CancelInProgressTask(bool force, CancelReason reason) { // TODO: Maybe ask the user if they really want to cancel? UNREFERENCED_PARAMETER(force); @@ -302,8 +302,11 @@ namespace AppInstaller::CLI::Execution ProgressCallback* callback = m_progressCallback.load(); if (callback) { - callback->SetProgressMessage(Resource::String::CancellingOperation()); - callback->Cancel(); + if (!callback->IsCancelledBy(CancelReason::Any)) + { + callback->SetProgressMessage(Resource::String::CancellingOperation()); + callback->Cancel(reason); + } } } diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h @@ -156,7 +156,7 @@ namespace AppInstaller::CLI::Execution void SetProgressCallback(ProgressCallback* callback); // Cancels the in progress task. - void CancelInProgressTask(bool force); + void CancelInProgressTask(bool force, CancelReason reason); void CloseOutputStream(bool forceDisable = false); diff --git a/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp @@ -9,15 +9,12 @@ namespace AppInstaller::CLI::Workflow { using namespace AppInstaller::MSStore; using namespace AppInstaller::SelfManagement; - using namespace std::string_view_literals; using namespace winrt::Windows::Foundation; using namespace winrt::Windows::Foundation::Collections; using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl; namespace { - static constexpr std::wstring_view s_AppInstallerProductId = L"9NBLGGH4NNS1"sv; - Utility::LocIndString GetErrorCodeString(const HRESULT errorCode) { std::ostringstream ssError; @@ -179,7 +176,7 @@ namespace AppInstaller::CLI::Workflow { context.Reporter.Info() << Resource::String::ConfigurationEnablingMessage << std::endl; bool bypassStorePolicy = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::BypassIsStoreClientBlockedPolicyCheck); - AppInstallerUpdate(false, bypassStorePolicy, context.Reporter); + AppInstallerUpdate(false, bypassStorePolicy, context); } else { diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -41,7 +41,7 @@ namespace AppInstaller::CLI::Workflow wil::unique_process_handle process{ execInfo.hProcess }; // Wait for installation to finish - while (!progress.IsCancelled()) + while (!progress.IsCancelledBy(CancelReason::User)) { DWORD waitResult = WaitForSingleObject(process.get(), 250); if (waitResult == WAIT_OBJECT_0) @@ -54,7 +54,7 @@ namespace AppInstaller::CLI::Workflow } } - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { return {}; } diff --git a/src/AppInstallerCLIE2ETests/AppShutdownTests.cs b/src/AppInstallerCLIE2ETests/AppShutdownTests.cs @@ -0,0 +1,115 @@ +// ----------------------------------------------------------------------------- +// <copyright file="AppShutdownTests.cs" company="Microsoft Corporation"> +// Copyright (c) Microsoft Corporation. Licensed under the MIT License. +// </copyright> +// ----------------------------------------------------------------------------- + +namespace AppInstallerCLIE2ETests +{ + using System; + using System.IO; + using System.Threading; + using System.Threading.Tasks; + using System.Xml; + using NUnit.Framework; + + /// <summary> + /// `test appshutdown` command tests. + /// </summary> + public class AppShutdownTests : BaseCommand + { + /// <summary> + /// Runs winget test appshutdown and register the application to force a WM_QUERYENDSESSION message. + /// </summary> + [Test] + [Ignore("This test won't work on Window Server")] + public void RegisterApplicationTest() + { + if (!TestCommon.PackagedContext) + { + return; + } + + if (string.IsNullOrEmpty(TestCommon.AICLIPackagePath)) + { + throw new NullReferenceException("AICLIPackagePath"); + } + + var appxManifest = Path.Combine(TestCommon.AICLIPackagePath, "AppxManifest.xml"); + if (!File.Exists(appxManifest)) + { + throw new FileNotFoundException(appxManifest); + } + + // In order to registering the application we need a higher version number and pass the force app shutdown flag. + // Doing it the long way. + var xmlDoc = new XmlDocument(); + XmlNamespaceManager namespaces = new XmlNamespaceManager(xmlDoc.NameTable); + namespaces.AddNamespace("n", "http://schemas.microsoft.com/appx/manifest/foundation/windows10"); + xmlDoc.Load(appxManifest); + var identityNode = xmlDoc.SelectSingleNode("/n:Package/n:Identity", namespaces); + if (identityNode == null) + { + throw new NullReferenceException("Identity node"); + } + + var versionAttribute = identityNode.Attributes["Version"]; + if (versionAttribute == null) + { + throw new NullReferenceException("Version attribute"); + } + + var ogVersion = new Version(versionAttribute.Value); + var newVersion = new Version(ogVersion.Major, ogVersion.Minor, ogVersion.Build, ogVersion.Revision + 1); + versionAttribute.Value = newVersion.ToString(); + xmlDoc.Save(appxManifest); + + // This just waits for the app termination event. + var testCmdTask = new Task<TestCommon.RunCommandResult>(() => + { + return TestCommon.RunAICLICommandViaInvokeCommandInDesktopPackage("test", "appshutdown", timeOut: 300000, throwOnTimeout: false); + }); + + // Register the app with the updated version. + var registerTask = new Task<bool>(() => + { + return TestCommon.InstallMsixRegister(TestCommon.AICLIPackagePath, true, false); + }); + + // Give it a little time. + testCmdTask.Start(); + Thread.Sleep(30000); + registerTask.Start(); + + Task.WaitAll(new Task[] { testCmdTask, registerTask }, 360000); + + // Assert.True(registerTask.Result); + TestContext.Out.Write(testCmdTask.Result.StdOut); + + // The ctrl-c command terminates the batch file before the exit code file gets created. + // Look for the output. + Assert.True(testCmdTask.Result.StdOut.Contains("Succeeded waiting for app shutdown event")); + } + + /// <summary> + /// Runs winget test appshutdown --force. + /// </summary> + [Test] + public void RegisterApplicationTest_Force() + { + if (!TestCommon.PackagedContext) + { + return; + } + + if (string.IsNullOrEmpty(TestCommon.AICLIPackagePath)) + { + throw new NullReferenceException("AICLIPackagePath"); + } + + var result = TestCommon.RunAICLICommandViaInvokeCommandInDesktopPackage("test", "appshutdown --force", timeOut: 300000, throwOnTimeout: false); + TestContext.Out.Write(result.StdOut); + Assert.True(result.StdOut.Contains("Succeeded waiting for app shutdown event")); + } + } +}+ \ No newline at end of file diff --git a/src/AppInstallerCLIE2ETests/TestCommon.cs b/src/AppInstallerCLIE2ETests/TestCommon.cs @@ -203,8 +203,9 @@ namespace AppInstallerCLIE2ETests /// <param name="parameters">Parameters.</param> /// <param name="stdIn">Optional std in.</param> /// <param name="timeOut">Optional timeout.</param> + /// <param name="throwOnTimeout">Throw on timeout.</param> /// <returns>The result of the command.</returns> - public static RunCommandResult RunAICLICommandViaInvokeCommandInDesktopPackage(string command, string parameters, string stdIn = null, int timeOut = 60000) + public static RunCommandResult RunAICLICommandViaInvokeCommandInDesktopPackage(string command, string parameters, string stdIn = null, int timeOut = 60000, bool throwOnTimeout = true) { string cmdCommandPiped = string.Empty; if (!string.IsNullOrEmpty(stdIn)) @@ -240,7 +241,7 @@ namespace AppInstallerCLIE2ETests waitedTime += 1000; } - if (waitedTime >= timeOut) + if (waitedTime >= timeOut && throwOnTimeout) { throw new TimeoutException($"Packaged winget command run timed out: {command} {parameters}"); } @@ -420,11 +421,20 @@ namespace AppInstallerCLIE2ETests /// Install and register msix package via appx manifest. /// </summary> /// <param name="packagePath">Path to package.</param> + /// <param name="forceShutdown">Force shutdown.</param> + /// <param name="throwOnFailure">Throw on failure.</param> /// <returns>True if installed correctly.</returns> - public static bool InstallMsixRegister(string packagePath) + public static bool InstallMsixRegister(string packagePath, bool forceShutdown = false, bool throwOnFailure = true) { string manifestFile = Path.Combine(packagePath, "AppxManifest.xml"); - return RunCommand("powershell", $"Add-AppxPackage -Register \"{manifestFile}\"", throwOnFailure: true); + + var command = $"Add-AppxPackage -Register \"{manifestFile}\""; + if (forceShutdown) + { + command += " -ForceTargetApplicationShutdown"; + } + + return RunCommand("powershell", command, throwOnFailure: throwOnFailure); } /// <summary> diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp @@ -176,7 +176,7 @@ namespace TestCommon { } - bool TestProgress::IsCancelled() + bool TestProgress::IsCancelledBy(AppInstaller::CancelReason) { return false; } diff --git a/src/AppInstallerCLITests/TestCommon.h b/src/AppInstallerCLITests/TestCommon.h @@ -105,7 +105,8 @@ namespace TestCommon void EndProgress(bool) override; - bool IsCancelled() override; + bool IsCancelledBy(AppInstaller::CancelReason) override; + CancelFunctionRemoval SetCancellationFunction(std::function<void()>&& f) override; std::function<void(uint64_t, uint64_t, AppInstaller::ProgressType)> m_OnProgress; diff --git a/src/AppInstallerCommonCore/DODownloader.cpp b/src/AppInstallerCommonCore/DODownloader.cpp @@ -258,7 +258,7 @@ namespace AppInstaller::Utility std::optional<UINT64> initialTransferAmount; bool transferChange = false; - while (!m_progress.IsCancelled()) + while (!m_progress.IsCancelledBy(CancelReason::Any)) { if (!transferChange) { @@ -273,7 +273,7 @@ namespace AppInstaller::Utility } // Since we just finished a wait, check for cancellation before handling anything else - if (m_progress.IsCancelled()) + if (m_progress.IsCancelledBy(CancelReason::Any)) { return false; } @@ -387,7 +387,7 @@ namespace AppInstaller::Utility }); // Check to handle cancellation between Start and SetCancellationFunction - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { AICLI_LOG(Core, Info, << "Download cancelled."); download.Cancel(); diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp @@ -93,7 +93,7 @@ namespace AppInstaller::Utility do { - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { AICLI_LOG(Core, Info, << "Download cancelled."); return {}; diff --git a/src/AppInstallerCommonCore/MSStore.cpp b/src/AppInstallerCommonCore/MSStore.cpp @@ -220,7 +220,7 @@ namespace AppInstaller::MSStore progress.OnProgress(currentProgress, overallProgressMax, ProgressType::Percent); } - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::User)) { for (auto const& installItem : installItems) { @@ -228,6 +228,21 @@ namespace AppInstaller::MSStore } } + // If app shutdown then we have 30s to keep installing, keep going and hope for the best. + else if (progress.IsCancelledBy(CancelReason::AppShutdown)) + { + for (auto const& installItem : installItems) + { + // Insert spiderman meme. + if (installItem.ProductId() == std::wstring{ s_AppInstallerProductId }) + { + AICLI_LOG(Core, Info, << "Asked to shutdown while installing AppInstaller."); + progress.OnProgress(overallProgressMax, overallProgressMax, ProgressType::Percent); + return S_OK; + } + } + } + Sleep(100); } diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp @@ -59,7 +59,7 @@ namespace AppInstaller::Msix UINT64 totalBytesRead = 0; - while (!progress.IsCancelled()) + while (!progress.IsCancelledBy(CancelReason::Any)) { ULONG bytesRead = 0; HRESULT hr = stream->Read(buffer.get(), bufferSize, &bytesRead); @@ -134,7 +134,7 @@ namespace AppInstaller::Msix UINT64 totalBytesRead = 0; - while (!progress.IsCancelled()) + while (!progress.IsCancelledBy(CancelReason::Any)) { ULONG bytesRead = 0; HRESULT hr = stream->Read(buffer.get(), bufferSize, &bytesRead); diff --git a/src/AppInstallerCommonCore/Progress.cpp b/src/AppInstallerCommonCore/Progress.cpp @@ -45,9 +45,10 @@ namespace AppInstaller } }; - bool ProgressCallback::IsCancelled() + bool ProgressCallback::IsCancelledBy(CancelReason cancelReasons) { - return m_cancelled.load(); + THROW_HR_IF(E_UNEXPECTED, cancelReasons == CancelReason::None); + return WI_IsAnyFlagSet(cancelReasons, m_cancelReason); } [[nodiscard]] IProgressCallback::CancelFunctionRemoval ProgressCallback::SetCancellationFunction(std::function<void()>&& f) @@ -63,9 +64,9 @@ namespace AppInstaller } } - void ProgressCallback::Cancel() + void ProgressCallback::Cancel(CancelReason reason) { - m_cancelled = true; + m_cancelReason = reason; if (m_cancellationFunction) { m_cancellationFunction(); diff --git a/src/AppInstallerCommonCore/Public/AppInstallerProgress.h b/src/AppInstallerCommonCore/Public/AppInstallerProgress.h @@ -27,6 +27,19 @@ namespace AppInstaller Percent, }; + // The reason why progress is cancelled. + enum class CancelReason : uint32_t + { + None = 0x0, + Abort = 0x1, + CtrlCSignal = 0x2, + User = Abort | CtrlCSignal, + AppShutdown = 0x4, + Any = 0xFFFFFFFF + }; + + DEFINE_ENUM_FLAG_OPERATORS(CancelReason); + // Interface that only receives progress, and does not participate in cancellation. // This allows a sink be simple, and let ProgressCallback handle the complications // of cancel state. @@ -53,7 +66,7 @@ namespace AppInstaller using CancelFunctionRemoval = wil::unique_any<IProgressCallback*, decltype(&details::RemoveCancellationFunction), details::RemoveCancellationFunction>; // Returns a value indicating if the future has been cancelled. - virtual bool IsCancelled() = 0; + virtual bool IsCancelledBy(CancelReason cancelReasons) = 0; // Sets a cancellation function that will be called when the operation is to be cancelled. [[nodiscard]] virtual CancelFunctionRemoval SetCancellationFunction(std::function<void()>&& f) = 0; @@ -73,18 +86,18 @@ namespace AppInstaller void EndProgress(bool hideProgressWhenDone) override; - bool IsCancelled() override; + bool IsCancelledBy(CancelReason cancelReasons) override; [[nodiscard]] IProgressCallback::CancelFunctionRemoval SetCancellationFunction(std::function<void()>&& f) override; - void Cancel(); + void Cancel(CancelReason reason = CancelReason::Abort); IProgressSink* GetSink(); private: std::atomic<IProgressSink*> m_sink = nullptr; - std::atomic_bool m_cancelled = false; std::function<void()> m_cancellationFunction; + CancelReason m_cancelReason = CancelReason::None; }; // A progress callback that reports its progress as a partial range of percentage to its base progress callback diff --git a/src/AppInstallerCommonCore/Public/winget/MSStore.h b/src/AppInstallerCommonCore/Public/winget/MSStore.h @@ -7,8 +7,13 @@ #include <winrt/Windows.Foundation.Collections.h> #include <winrt/Windows.ApplicationModel.Store.Preview.InstallControl.h> +#include <string> + namespace AppInstaller::MSStore { + using namespace std::string_view_literals; + static constexpr std::wstring_view s_AppInstallerProductId = L"9NBLGGH4NNS1"sv; + enum class MSStoreOperationType { Install, diff --git a/src/AppInstallerCommonCore/Synchronization.cpp b/src/AppInstallerCommonCore/Synchronization.cpp @@ -112,7 +112,7 @@ namespace AppInstaller::Synchronization auto lock = controlMutex.acquire(&status, static_cast<DWORD>(timeout.count())); THROW_LAST_ERROR_IF(status == WAIT_FAILED); - if (status == WAIT_TIMEOUT || (progress && progress->IsCancelled())) + if (status == WAIT_TIMEOUT || (progress && progress->IsCancelledBy(CancelReason::Any))) { return result; } @@ -165,7 +165,7 @@ namespace AppInstaller::Synchronization // Wait for one/all of the mutexes (or cancellation) bool waitAgain = true; - while (waitAgain && (!progress || !progress->IsCancelled())) + while (waitAgain && (!progress || !progress->IsCancelledBy(CancelReason::Any))) { DWORD millisecondsToWait = 0; if (progress) @@ -231,7 +231,7 @@ namespace AppInstaller::Synchronization } } - if (status == WAIT_TIMEOUT || (progress && progress->IsCancelled())) + if (status == WAIT_TIMEOUT || (progress && progress->IsCancelledBy(CancelReason::Any))) { return result; } @@ -270,7 +270,7 @@ namespace AppInstaller::Synchronization bool CrossProcessInstallLock::Acquire(IProgressCallback& progress) { - while (!progress.IsCancelled()) + while (!progress.IsCancelledBy(CancelReason::Any)) { auto lock = m_mutex.acquire(nullptr, static_cast<DWORD>(std::chrono::duration_cast<std::chrono::milliseconds>(s_CrossProcessInstallLock_WaitLoopTime).count())); diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp @@ -220,7 +220,7 @@ namespace AppInstaller::Repository::Microsoft THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_DATA_INTEGRITY_FAILURE, GetPackageFamilyNameFromDetails(details) != Msix::GetPackageFamilyNameFromFullName(packageInfo.MsixInfo().GetPackageFullName())); - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { AICLI_LOG(Repo, Info, << "Cancelling update upon request"); return false; @@ -315,7 +315,7 @@ namespace AppInstaller::Repository::Microsoft } } - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { AICLI_LOG(Repo, Info, << "Cancelling update upon request"); return false; @@ -450,7 +450,7 @@ namespace AppInstaller::Repository::Microsoft Msix::MsixInfo packageInfo(packageLocation); packageInfo.WriteToFileHandle(s_PreIndexedPackageSourceFactory_IndexFilePath, tempIndexFile.GetFileHandle(), progress); - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { AICLI_LOG(Repo, Info, << "Cancelling open upon request"); return {}; @@ -508,7 +508,7 @@ namespace AppInstaller::Repository::Microsoft } bool updateSuccess = false; - if (progress.IsCancelled()) + if (progress.IsCancelledBy(CancelReason::Any)) { AICLI_LOG(Repo, Info, << "Cancelling update upon request"); } diff --git a/src/AppInstallerSharedLib/Errors.cpp b/src/AppInstallerSharedLib/Errors.cpp @@ -224,6 +224,8 @@ namespace AppInstaller return "The package has a pin that prevents upgrade."; case APPINSTALLER_CLI_ERROR_PACKAGE_IS_STUB: return "The package currently installed is the stub package"; + case APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED: + return "Application shutdown signal received"; // Install errors case APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE: diff --git a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h @@ -118,6 +118,7 @@ #define APPINSTALLER_CLI_ERROR_NOT_ALL_QUERIES_FOUND_SINGLE ((HRESULT)0x8A150067) #define APPINSTALLER_CLI_ERROR_PACKAGE_IS_PINNED ((HRESULT)0x8A150068) #define APPINSTALLER_CLI_ERROR_PACKAGE_IS_STUB ((HRESULT)0x8A150069) +#define APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED ((HRESULT)0x8A15006A) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101)