From d86b223274e9f2e42b5b3c23ac1be9dae7645202 Mon Sep 17 00:00:00 2001 From: sachintaMSFT <80828309+sachintaMSFT@users.noreply.github.com> Date: Wed, 8 Jun 2022 00:47:03 -0700 Subject: [PATCH] Cherry-pick fix for break away process creation attribute from release/1.1-stable to main (#2588) * Return Extended Hresult instead of Hresult to help customers by providing them with unbucketed error codes instead of bucketed error codes, in a best effort manner. * Deployment agent break away process creation fixes (#2554) * Create Breakaway process for DeplomentAgent.exe in Running state (not in suspended state) * And remove Resume * Don't inherent parent handles in breakaway process * Create process outside desktop app runtime environment instead * Don't return a copy but a reference to tracelogger activity class object Co-authored-by: Santosh Chintalapati --- dev/Deployment/DeploymentActivityContext.h | 2 +- dev/Deployment/DeploymentManager.cpp | 15 +++++---------- dev/Deployment/DeploymentManager.h | 1 - .../MddBootstrapActivity.h | 4 ++-- installer/dev/InstallActivityContext.cpp | 2 ++ installer/dev/InstallActivityContext.h | 8 ++++++++ installer/dev/install.cpp | 7 ++++--- installer/dev/main.cpp | 2 +- installer/dev/tracelogging.cpp | 2 +- installer/dev/tracelogging.h | 4 ++-- 10 files changed, 26 insertions(+), 21 deletions(-) diff --git a/dev/Deployment/DeploymentActivityContext.h b/dev/Deployment/DeploymentActivityContext.h index 16d4d59b3..ad8430084 100644 --- a/dev/Deployment/DeploymentActivityContext.h +++ b/dev/Deployment/DeploymentActivityContext.h @@ -68,7 +68,7 @@ namespace WindowsAppRuntime::Deployment::Activity return m_deploymentErrorActivityId; } - WindowsAppRuntimeDeployment_TraceLogger::Initialize GetActivity() const + WindowsAppRuntimeDeployment_TraceLogger::Initialize& GetActivity() { return m_activity; } diff --git a/dev/Deployment/DeploymentManager.cpp b/dev/Deployment/DeploymentManager.cpp index 9757462ba..bd46f276e 100644 --- a/dev/Deployment/DeploymentManager.cpp +++ b/dev/Deployment/DeploymentManager.cpp @@ -332,10 +332,10 @@ namespace winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::implem THROW_IF_WIN32_BOOL_FALSE(InitializeProcThreadAttributeList(attributeList, attributeCount, 0, &attributeListSize)); auto freeAttributeList{ wil::scope_exit([&] { DeleteProcThreadAttributeList(attributeList); }) }; - // Desktop Bridge applications by default have their child processes break away from the parent process. - // In order to recreate the calling process' environment correctly, we must prevent child breakaway semantics - // when calling the agent. Additionally the agent must do the same when restarting the caller. - DWORD policy{ PROCESS_CREATION_DESKTOP_APP_BREAKAWAY_OVERRIDE }; + // https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute + // The process being created will create any child processes outside of the desktop app runtime environment. + // This behavior is the default for processes for which no policy has been set + DWORD policy{ PROCESS_CREATION_DESKTOP_APP_BREAKAWAY_ENABLE_PROCESS_TREE }; THROW_IF_WIN32_BOOL_FALSE(UpdateProcThreadAttribute(attributeList, 0, PROC_THREAD_ATTRIBUTE_DESKTOP_APP_POLICY, &policy, sizeof(policy), nullptr, nullptr)); STARTUPINFOEX info{}; @@ -343,12 +343,7 @@ namespace winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::implem info.lpAttributeList = attributeList; wil::unique_process_information processInfo; - THROW_IF_WIN32_BOOL_FALSE(CreateProcess(nullptr, cmdLine.get(), nullptr, nullptr, TRUE, CREATE_SUSPENDED | EXTENDED_STARTUPINFO_PRESENT, nullptr, nullptr, - &info.StartupInfo, &processInfo)); - - // Transfer foreground rights to the new process before resuming it. - AllowSetForegroundWindow(processInfo.dwProcessId); - ResumeThread(processInfo.hThread); + THROW_IF_WIN32_BOOL_FALSE(CreateProcess(nullptr, cmdLine.get(), nullptr, nullptr, FALSE, EXTENDED_STARTUPINFO_PRESENT, nullptr, nullptr, &info.StartupInfo, &processInfo)); // This API is designed to only return to the caller on failure, otherwise block until process termination. // Wait for the agent to exit. If the agent succeeds, it will terminate this process. If the agent fails, diff --git a/dev/Deployment/DeploymentManager.h b/dev/Deployment/DeploymentManager.h index 406f803b8..766a74ba6 100644 --- a/dev/Deployment/DeploymentManager.h +++ b/dev/Deployment/DeploymentManager.h @@ -40,4 +40,3 @@ namespace winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::factor { }; } - diff --git a/dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h b/dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h index 6baa35aeb..6f1ad8d26 100644 --- a/dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h +++ b/dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h @@ -67,12 +67,12 @@ namespace WindowsAppRuntime::MddBootstrap::Activity return m_initializationPackageFullName; } - WindowsAppRuntimeBootstrap_TraceLogger::Initialize GetInitializeActivity() + WindowsAppRuntimeBootstrap_TraceLogger::Initialize& GetInitializeActivity() { return m_bootstrapInitializeActivity; } - WindowsAppRuntimeBootstrap_TraceLogger::Shutdown GetShutdownActivity() + WindowsAppRuntimeBootstrap_TraceLogger::Shutdown& GetShutdownActivity() { return m_bootstrapShutdownActivity; } diff --git a/installer/dev/InstallActivityContext.cpp b/installer/dev/InstallActivityContext.cpp index c170a403e..1e7b42de2 100644 --- a/installer/dev/InstallActivityContext.cpp +++ b/installer/dev/InstallActivityContext.cpp @@ -19,10 +19,12 @@ void WindowsAppRuntimeInstaller::InstallActivity::Context::Reset() } void WindowsAppRuntimeInstaller::InstallActivity::Context::SetDeploymentErrorInfo( + const HRESULT& deploymentErrorHresult, const HRESULT& deploymentErrorExtendedHresult, const std::wstring& deploymentErrorText, const GUID& deploymentErrorActivityId) { + m_deploymentErrorHresult = deploymentErrorHresult; m_deploymentErrorExtendedHresult = deploymentErrorExtendedHresult; m_deploymentErrorText = deploymentErrorText; SetDeploymentErrorActivityId(deploymentErrorActivityId); diff --git a/installer/dev/InstallActivityContext.h b/installer/dev/InstallActivityContext.h index d50883449..bdabc8a32 100644 --- a/installer/dev/InstallActivityContext.h +++ b/installer/dev/InstallActivityContext.h @@ -32,6 +32,7 @@ namespace WindowsAppRuntimeInstaller::InstallActivity { InstallStage m_installStage{}; std::wstring m_currentResourceId; + HRESULT m_deploymentErrorHresult{}; HRESULT m_deploymentErrorExtendedHresult{}; std::wstring m_deploymentErrorText; GUID m_deploymentErrorActivityId{}; @@ -54,6 +55,12 @@ namespace WindowsAppRuntimeInstaller::InstallActivity return m_currentResourceId; } + const HRESULT& GetdeploymentErrorHresult() const + { + return m_deploymentErrorHresult; + } + + const HRESULT& GetDeploymentErrorExtendedHResult() const { return m_deploymentErrorExtendedHresult; @@ -90,6 +97,7 @@ namespace WindowsAppRuntimeInstaller::InstallActivity } void SetDeploymentErrorInfo( + const HRESULT& deploymentErrorHresult, const HRESULT& deploymentErrorExtendedHresult, const std::wstring& deploymentErrorText, const GUID& deploymentErrorActivityId); diff --git a/installer/dev/install.cpp b/installer/dev/install.cpp index 4e545d0bf..cf0d9fd73 100644 --- a/installer/dev/install.cpp +++ b/installer/dev/install.cpp @@ -27,11 +27,12 @@ namespace WindowsAppRuntimeInstaller { const auto deploymentResult{ deploymentOperation.GetResults() }; WindowsAppRuntimeInstaller::InstallActivity::Context::Get().SetDeploymentErrorInfo( + deploymentOperation.ErrorCode(), deploymentResult.ExtendedErrorCode(), deploymentResult.ErrorText().c_str(), deploymentResult.ActivityId()); - return static_cast(deploymentOperation.ErrorCode()); + return static_cast(deploymentResult.ExtendedErrorCode()); } return S_OK; @@ -61,9 +62,9 @@ namespace WindowsAppRuntimeInstaller else { const auto deploymentResult{ deploymentOperation.GetResults() }; - WindowsAppRuntimeInstaller::InstallActivity::Context::Get().SetDeploymentErrorInfo(deploymentResult.ExtendedErrorCode(), deploymentResult.ErrorText().c_str(), deploymentResult.ActivityId()); + WindowsAppRuntimeInstaller::InstallActivity::Context::Get().SetDeploymentErrorInfo(hrAddPackage, deploymentResult.ExtendedErrorCode(), deploymentResult.ErrorText().c_str(), deploymentResult.ActivityId()); - RETURN_HR(hrAddPackage); + RETURN_HR(static_cast(deploymentResult.ExtendedErrorCode())); } } return S_OK; diff --git a/installer/dev/main.cpp b/installer/dev/main.cpp index 7d961f245..f28b2ecce 100644 --- a/installer/dev/main.cpp +++ b/installer/dev/main.cpp @@ -136,7 +136,7 @@ int wmain(int argc, wchar_t *argv[]) installActivityContext.GetLastFailure().message.c_str(), static_cast(installActivityContext.GetInstallStage()), installActivityContext.GetCurrentResourceId().c_str(), - installActivityContext.GetDeploymentErrorExtendedHResult(), + installActivityContext.GetdeploymentErrorHresult(), installActivityContext.GetDeploymentErrorText().c_str(), installActivityContext.GetDeploymentErrorActivityId()); } diff --git a/installer/dev/tracelogging.cpp b/installer/dev/tracelogging.cpp index ed8a031c0..b88a7816a 100644 --- a/installer/dev/tracelogging.cpp +++ b/installer/dev/tracelogging.cpp @@ -77,7 +77,7 @@ void __stdcall wilResultLoggingCallback(const wil::FailureInfo& failure) noexcep failure.pszMessage, static_cast(installActivityContext.GetInstallStage()), installActivityContext.GetCurrentResourceId().c_str(), - installActivityContext.GetDeploymentErrorExtendedHResult(), + installActivityContext.GetdeploymentErrorHresult(), installActivityContext.GetDeploymentErrorText().c_str(), installActivityContext.GetDeploymentErrorActivityId()); diff --git a/installer/dev/tracelogging.h b/installer/dev/tracelogging.h index 34f2cf9db..291d01699 100644 --- a/installer/dev/tracelogging.h +++ b/installer/dev/tracelogging.h @@ -54,7 +54,7 @@ public: PCWSTR failureMessage, UINT32 failedInstallStage, PCWSTR currentResourceId, - HRESULT deploymentErrorExtendedHResult, + HRESULT deploymentErrorHresult, PCWSTR deploymentErrorText, GUID deploymentErrorActivityId) { @@ -70,7 +70,7 @@ public: TraceLoggingValue(failureMessage, "FailureMessage"), TraceLoggingValue(failedInstallStage, "FailedInstallStage"), TraceLoggingValue(currentResourceId, "CurrentResourceId"), - TraceLoggingValue(deploymentErrorExtendedHResult, "DeploymentErrorExtendedHResult"), + TraceLoggingValue(deploymentErrorHresult, "DeploymentErrorHResult"), TraceLoggingValue(deploymentErrorText, "DeploymentErrorText"), TraceLoggingValue(deploymentErrorActivityId, "DeploymentErrorActivityId")); }