Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register restart for resume #3858

Merged
merged 25 commits into from Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/AppInstallerCLICore/Command.cpp
Expand Up @@ -871,6 +871,7 @@ namespace AppInstaller::CLI
ExecuteInternal(context);
}

// NOTE: Reboot logic will still run even if the context is terminated (not including unhandled exceptions).
if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::Reboot) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the context is terminated (not with an exception, but "normally"), this code will still run. That may or may not be correct, but I want to make sure it is considered. It feels ok, but then I worry that it won't happen for an exception. I would say that we haven't been strict on the precise method for failures to propagate, which means that this code is likely to not run in some situations when it probably should.

Since this is experimental, it is fine to live here for now. But it will be good to keep this in mind.

context.Args.Contains(Execution::Args::Type::AllowReboot) &&
WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::RebootRequired))
Expand All @@ -886,7 +887,7 @@ namespace AppInstaller::CLI
{
// RegisterResume context flag assumes we already wrote to the RunOnce registry.
// Since we are about to initiate a restart, this is no longer needed as a safety net.
Reboot::UnregisterApplicationForReboot();
Reboot::UnregisterRestartForWER();

context.ClearFlags(Execution::ContextFlag::RegisterResume);
}
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/ExecutionContext.cpp
Expand Up @@ -295,7 +295,7 @@ namespace AppInstaller::CLI::Execution
if (m_checkpointManager && (!IsTerminated() || ShouldRemoveCheckpointDatabase(GetTerminationHR())))
{
m_checkpointManager->CleanUpDatabase();
AppInstaller::Reboot::UnregisterApplicationForReboot();
AppInstaller::Reboot::UnregisterRestartForWER();
}
}

Expand Down Expand Up @@ -490,7 +490,7 @@ namespace AppInstaller::CLI::Execution
m_checkpointManager->CreateAutomaticCheckpoint(*this);

// Register for restart only when we first call checkpoint to support restarting from an unexpected shutdown.
AppInstaller::Reboot::RegisterApplicationForReboot("resume -g " + GetResumeId());
AppInstaller::Reboot::RegisterRestartForWER("resume -g " + GetResumeId());
}

// TODO: Capture context data for checkpoint.
Expand Down
13 changes: 5 additions & 8 deletions src/AppInstallerCLICore/Workflows/ResumeFlow.cpp
@@ -1,9 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#include <AppInstallerRuntime.h>
#include "ResumeFlow.h"
#include "winget/Reboot.h"
#include <AppInstallerRuntime.h>

namespace AppInstaller::CLI::Workflow
{
Expand All @@ -27,15 +27,12 @@ namespace AppInstaller::CLI::Workflow

if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::RegisterResume))
{
int argc = 0;
wil::unique_hlocal_ptr<LPWSTR> argv{ CommandLineToArgvW(GetCommandLineW(), &argc) };
THROW_LAST_ERROR_IF_NULL(argv);
auto executablePath = AppInstaller::Runtime::GetPathTo(AppInstaller::Runtime::PathName::CLIExecutable);

// RunOnce registry value must start with the full path of the executable.
std::string commandLine = Utility::ConvertToUTF8(argv.get()[0]) + " resume -g " + context.GetResumeId();

bool isAdmin = AppInstaller::Runtime::IsRunningAsAdmin();
Reboot::WriteToRunOnceRegistry(commandLine, isAdmin);
const auto& resumeId = context.GetResumeId();
std::string commandLine = executablePath.u8string() + " resume -g " + resumeId;
Reboot::WriteToRunOnceRegistry(resumeId, commandLine);
}
}
}
6 changes: 3 additions & 3 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Expand Up @@ -2632,10 +2632,10 @@ Please specify one of them using the --source option to proceed.</value>
<value>Failed to initiate a reboot.</value>
</data>
<data name="ResumeLimitExceeded" xml:space="preserve">
<value>Resume operation exceeds the allowable limit of {0} resume(s). To manually resume, run the command: </value>
<comment>{Locked="{0}"} {0} is a placeholder that is replaced by an integer number of the number of allowed resumes.</comment>
<value>Resume operation exceeds the allowable limit of {0} resume(s). To manually resume, run the command `{1}`.</value>
<comment>{Locked="{0}", "{1}"} {0} is a placeholder that is replaced by an integer number of the number of allowed resumes. {1} is a placeholder for the command to run to perform a manual resume.</comment>
</data>
<data name="IgnoreResumeLimitArgumentDescription" xml:space="preserve">
<value>Ignore the max limit of resuming a saved state</value>
<value>Ignore the limit on resuming a saved state</value>
</data>
</root>
2 changes: 2 additions & 0 deletions src/AppInstallerCommonCore/Public/AppInstallerRuntime.h
Expand Up @@ -51,6 +51,8 @@ namespace AppInstaller::Runtime
UserProfileDownloads,
// The location where checkpoints are stored.
CheckpointsLocation,
// The location of the CLI executable file.
CLIExecutable,
// Always one more than the last path; for being able to iterate paths in tests.
Max
};
Expand Down
10 changes: 5 additions & 5 deletions src/AppInstallerCommonCore/Public/winget/Reboot.h
Expand Up @@ -6,12 +6,12 @@ namespace AppInstaller::Reboot
{
bool InitiateReboot();

// Registers the application to be restarted by Windows Error Reporting in case of an unexpected shutdown.
bool RegisterApplicationForReboot(const std::string& commandLineArgs);
// Registers the application to be restarted by Windows Error Reporting (WER) in case of an unexpected shutdown.
bool RegisterRestartForWER(const std::string& commandLineArgs);

// Unregisters the application from being restarted by Windows Error Reporting.
bool UnregisterApplicationForReboot();
// Unregisters the application from being restarted by Windows Error Reporting (WER).
bool UnregisterRestartForWER();

// Runs a program when a user logs on.
void WriteToRunOnceRegistry(const std::string& commandLine, bool isAdmin);
void WriteToRunOnceRegistry(const std::string& resumeId, const std::string& commandLine);
}
15 changes: 8 additions & 7 deletions src/AppInstallerCommonCore/Reboot.cpp
Expand Up @@ -5,6 +5,7 @@
#include "AppInstallerStrings.h"
#include "Public/winget/Reboot.h"
#include "Public/winget/Registry.h"
#include <AppInstallerRuntime.h>
#include <Windows.h>

using namespace AppInstaller::Registry;
Expand Down Expand Up @@ -71,7 +72,7 @@ namespace AppInstaller::Reboot
return ExitWindowsEx(EWX_RESTARTAPPS, SHTDN_REASON_MINOR_INSTALLATION);
}

bool RegisterApplicationForReboot(const std::string& commandLineArgs)
bool RegisterRestartForWER(const std::string& commandLineArgs)
{
#ifndef AICLI_DISABLE_TEST_HOOKS
if (s_RegisterForRestartResult_TestHook_Override)
Expand All @@ -81,7 +82,6 @@ namespace AppInstaller::Reboot
#endif

HRESULT result = RegisterApplicationRestart(AppInstaller::Utility::ConvertToUTF16(commandLineArgs).c_str(), 0 /* Always restart process */);
AICLI_LOG(CLI, Info, << "Register for restart with command line args: " << commandLineArgs);

if (FAILED(result))
{
Expand All @@ -90,11 +90,12 @@ namespace AppInstaller::Reboot
}
else
{
AICLI_LOG(CLI, Info, << "Register for restart with command line args: " << commandLineArgs);
return true;
}
}

bool UnregisterApplicationForReboot()
bool UnregisterRestartForWER()
{
HRESULT result = UnregisterApplicationRestart();
AICLI_LOG(CLI, Info, << "Application unregistered for restart.");
Expand All @@ -110,20 +111,20 @@ namespace AppInstaller::Reboot
}
}

void WriteToRunOnceRegistry(const std::string& commandLine, bool isAdmin)
void WriteToRunOnceRegistry(const std::string& resumeId, const std::string& commandLine)
{
THROW_HR_IF(E_UNEXPECTED, commandLine.size() > MAX_PATH);

HKEY root = isAdmin ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
HKEY root = AppInstaller::Runtime::IsRunningAsAdmin() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
std::wstring subKey = std::wstring{ s_RunOnceRegistry };
Key key = Key::OpenIfExists(root, subKey, 0, KEY_ALL_ACCESS);

if (key == NULL)
if (!key)
{
key = Key::Create(root, subKey);
}

key.SetValue(L"WingetResume", Utility::ConvertToUTF16(commandLine), REG_SZ);
key.SetValue(Utility::ConvertToUTF16("WingetResume-" + resumeId), Utility::ConvertToUTF16(commandLine), REG_SZ);
AICLI_LOG(CLI, Info, << "Set RunOnce registry with value: " << commandLine);
}
}
22 changes: 22 additions & 0 deletions src/AppInstallerCommonCore/Runtime.cpp
Expand Up @@ -41,6 +41,9 @@ namespace AppInstaller::Runtime

constexpr std::string_view s_UserProfileEnvironmentVariable = "%USERPROFILE%";
constexpr std::string_view s_LocalAppDataEnvironmentVariable = "%LOCALAPPDATA%";
constexpr std::string_view s_WindowsApps_Base = "Microsoft\\WindowsApps"sv;
constexpr std::string_view s_WinGetDev_Exe = "wingetdev.exe";
constexpr std::string_view s_WinGet_Exe = "winget.exe";

static std::optional<std::string> s_runtimePathStateName;
static wil::srwlock s_runtimePathStateNameLock;
Expand Down Expand Up @@ -472,6 +475,20 @@ namespace AppInstaller::Runtime
result = GetPathDetailsForPackagedContext(PathName::LocalState, forDisplay);
result.Path /= s_CheckpointsDirectory;
break;
case PathName::CLIExecutable:
result.Path = GetKnownFolderPath(FOLDERID_LocalAppData);
result.Path /= s_WindowsApps_Base;
if (IsReleaseBuild())
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
{
result.Path /= s_WinGet_Exe;
}
else
{
result.Path /= s_WinGetDev_Exe;
}
result.Create = false;
mayBeInProfilePath = true;
break;
default:
THROW_HR(E_UNEXPECTED);
}
Expand Down Expand Up @@ -555,8 +572,13 @@ namespace AppInstaller::Runtime
result = GetPathDetailsCommon(path, forDisplay);
break;
case PathName::SelfPackageRoot:
case PathName::CLIExecutable:
result.Path = GetBinaryDirectoryPath();
result.Create = false;
if (path == PathName::CLIExecutable)
{
result.Path /= s_WinGet_Exe;
}
break;
case PathName::CheckpointsLocation:
result = GetPathDetailsForUnpackagedContext(PathName::LocalState, forDisplay);
Expand Down