From fbc1a73743368211d5d8c7fc0625adf6eb9ca50c Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Fri, 18 Mar 2022 20:16:07 -0500 Subject: Add checkpoint so Exe and Msu packages rollback after being cancelled. Fixes 5950 --- src/burn/engine/apply.cpp | 93 +++++++++++++++++----- src/burn/engine/apply.h | 2 + src/burn/engine/elevation.cpp | 18 +++++ src/burn/engine/engine.mc | 7 ++ src/burn/engine/exeengine.cpp | 37 ++++++--- src/burn/engine/msuengine.cpp | 27 ++++--- src/burn/engine/package.h | 1 + src/burn/test/BurnUnitTest/PlanTest.cpp | 4 + src/test/burn/TestBA/TestBA.cs | 25 +++++- .../burn/WixToolsetTest.BurnE2E/FailureTests.cs | 14 ++-- 10 files changed, 171 insertions(+), 57 deletions(-) (limited to 'src') diff --git a/src/burn/engine/apply.cpp b/src/burn/engine/apply.cpp index 73b5b396..6af5c040 100644 --- a/src/burn/engine/apply.cpp +++ b/src/burn/engine/apply.cpp @@ -63,6 +63,7 @@ typedef struct _BURN_EXECUTE_CONTEXT LPCWSTR wzExecutingPackageId; DWORD cExecutedPackages; DWORD cExecutePackagesTotal; + BOOL fAbandonedProcess; } BURN_EXECUTE_CONTEXT; @@ -322,6 +323,7 @@ static HRESULT ExecutePackageComplete( __in BURN_VARIABLES* pVariables, __in LPCWSTR wzPackageId, __in BOOL fPackageVital, + __in BOOL fAbandonedProcess, __in HRESULT hrOverall, __in HRESULT hrExecute, __in BOOL fRollback, @@ -369,6 +371,7 @@ extern "C" void ApplyReset( BURN_PACKAGE* pPackage = pPackages->rgPackages + i; pPackage->hrCacheResult = S_OK; pPackage->fReachedExecution = FALSE; + pPackage->fAbandonedProcess = FALSE; pPackage->transactionRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN; } } @@ -715,6 +718,9 @@ extern "C" HRESULT ApplyExecute( continue; } + context.wzExecutingPackageId = NULL; + context.fAbandonedProcess = FALSE; + // If we are seeking the end of the rollback boundary, skip if this action wasn't it. if (fSeekRollbackBoundaryEnd) { @@ -2437,6 +2443,9 @@ static HRESULT DoRollbackActions( continue; } + pContext->wzExecutingPackageId = NULL; + pContext->fAbandonedProcess = FALSE; + if (BURN_EXECUTE_ACTION_TYPE_CHECKPOINT == pRollbackAction->type) { if (pRollbackAction->checkpoint.dwId == dwCheckpoint) @@ -2533,6 +2542,28 @@ LExit: return hr; } +static BOOL ShouldSkipPackage( + __in BURN_PACKAGE* pPackage, + __in BOOL fRollback + ) +{ + BOOL fSkip = FALSE; + + if (FAILED(pPackage->hrCacheResult)) + { + LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_FAILED_CACHED_PACKAGE, pPackage->sczId, pPackage->hrCacheResult); + ExitFunction1(fSkip = TRUE); + } + else if (fRollback && pPackage->fAbandonedProcess) + { + LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_PACKAGE_WITH_ABANDONED_PROCESS, pPackage->sczId); + ExitFunction1(fSkip = TRUE); + } + +LExit: + return fSkip; +} + static HRESULT ExecuteRelatedBundle( __in BURN_ENGINE_STATE* pEngineState, __in BURN_EXECUTE_ACTION* pExecuteAction, @@ -2551,13 +2582,13 @@ static HRESULT ExecuteRelatedBundle( BURN_RELATED_BUNDLE* pRelatedBundle = pExecuteAction->relatedBundle.pRelatedBundle; BURN_PACKAGE* pPackage = &pRelatedBundle->package; - if (FAILED(pPackage->hrCacheResult)) + Assert(pContext->fRollback == fRollback); + + if (ShouldSkipPackage(pPackage, fRollback)) { - LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_FAILED_CACHED_PACKAGE, pPackage->sczId, pPackage->hrCacheResult); ExitFunction1(hr = S_OK); } - Assert(pContext->fRollback == fRollback); pContext->wzExecutingPackageId = pPackage->sczId; fBeginCalled = TRUE; @@ -2599,7 +2630,8 @@ static HRESULT ExecuteRelatedBundle( LExit: if (fBeginCalled) { - hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); + pPackage->fAbandonedProcess = pContext->fAbandonedProcess; + hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, pPackage->fAbandonedProcess, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); } return hr; @@ -2624,6 +2656,9 @@ static HRESULT DoRestoreRelatedBundleActions( continue; } + pContext->wzExecutingPackageId = NULL; + pContext->fAbandonedProcess = FALSE; + BOOTSTRAPPER_APPLY_RESTART restart = BOOTSTRAPPER_APPLY_RESTART_NONE; switch (pRestoreRelatedBundleAction->type) { @@ -2665,13 +2700,13 @@ static HRESULT ExecuteExePackage( BOOL fExecuted = FALSE; BURN_PACKAGE* pPackage = pExecuteAction->exePackage.pPackage; - if (FAILED(pPackage->hrCacheResult)) + Assert(pContext->fRollback == fRollback); + + if (ShouldSkipPackage(pPackage, fRollback)) { - LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_FAILED_CACHED_PACKAGE, pPackage->sczId, pPackage->hrCacheResult); ExitFunction1(hr = S_OK); } - Assert(pContext->fRollback == fRollback); pContext->wzExecutingPackageId = pPackage->sczId; fBeginCalled = TRUE; @@ -2720,7 +2755,8 @@ LExit: if (fBeginCalled) { - hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); + pPackage->fAbandonedProcess = pContext->fAbandonedProcess; + hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, pPackage->fAbandonedProcess, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); } return hr; @@ -2743,13 +2779,13 @@ static HRESULT ExecuteMsiPackage( BOOL fExecuted = FALSE; BURN_PACKAGE* pPackage = pExecuteAction->msiPackage.pPackage; - if (FAILED(pPackage->hrCacheResult)) + Assert(pContext->fRollback == fRollback); + + if (ShouldSkipPackage(pPackage, fRollback)) { - LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_FAILED_CACHED_PACKAGE, pPackage->sczId, pPackage->hrCacheResult); ExitFunction1(hr = S_OK); } - Assert(pContext->fRollback == fRollback); pContext->wzExecutingPackageId = pPackage->sczId; fBeginCalled = TRUE; @@ -2784,7 +2820,8 @@ LExit: if (fBeginCalled) { - hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); + Assert(!pContext->fAbandonedProcess); + hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, FALSE, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); } return hr; @@ -2807,13 +2844,13 @@ static HRESULT ExecuteMspPackage( BOOL fExecuted = FALSE; BURN_PACKAGE* pPackage = pExecuteAction->mspTarget.pPackage; - if (FAILED(pPackage->hrCacheResult)) + Assert(pContext->fRollback == fRollback); + + if (ShouldSkipPackage(pPackage, fRollback)) { - LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_FAILED_CACHED_PACKAGE, pPackage->sczId, pPackage->hrCacheResult); ExitFunction1(hr = S_OK); } - Assert(pContext->fRollback == fRollback); pContext->wzExecutingPackageId = pPackage->sczId; fBeginCalled = TRUE; @@ -2857,7 +2894,8 @@ LExit: if (fBeginCalled) { - hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); + Assert(!pContext->fAbandonedProcess); + hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, FALSE, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); } return hr; @@ -2882,13 +2920,13 @@ static HRESULT ExecuteMsuPackage( BOOL fExecuted = FALSE; BURN_PACKAGE* pPackage = pExecuteAction->msuPackage.pPackage; - if (FAILED(pPackage->hrCacheResult)) + Assert(pContext->fRollback == fRollback); + + if (ShouldSkipPackage(pPackage, fRollback)) { - LogId(REPORT_STANDARD, MSG_APPLY_SKIPPED_FAILED_CACHED_PACKAGE, pPackage->sczId, pPackage->hrCacheResult); ExitFunction1(hr = S_OK); } - Assert(pContext->fRollback == fRollback); pContext->wzExecutingPackageId = pPackage->sczId; fBeginCalled = TRUE; @@ -2937,7 +2975,8 @@ LExit: if (fBeginCalled) { - hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); + pPackage->fAbandonedProcess = pContext->fAbandonedProcess; + hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pPackage->sczId, pPackage->fVital, pPackage->fAbandonedProcess, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); } return hr; @@ -3256,7 +3295,8 @@ LExit: if (fBeginCalled) { - hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pContext->wzExecutingPackageId, FALSE, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); + Assert(!pContext->fAbandonedProcess); + hr = ExecutePackageComplete(&pEngineState->userExperience, &pEngineState->variables, pContext->wzExecutingPackageId, FALSE, FALSE, hr, hrExecute, fRollback, pRestart, pfRetry, pfSuspend); } return hr; @@ -3334,6 +3374,14 @@ static int GenericExecuteMessageHandler( } break; + case GENERIC_EXECUTE_MESSAGE_PROCESS_STARTED: + pContext->fAbandonedProcess = TRUE; + break; + + case GENERIC_EXECUTE_MESSAGE_PROCESS_COMPLETED: + pContext->fAbandonedProcess = FALSE; + break; + case GENERIC_EXECUTE_MESSAGE_ERROR: UserExperienceOnError(pContext->pUX, BOOTSTRAPPER_ERROR_TYPE_EXE_PACKAGE, pContext->wzExecutingPackageId, pMessage->error.dwErrorCode, pMessage->error.wzMessage, pMessage->dwUIHint, 0, NULL, &nResult); // ignore return value. break; @@ -3428,6 +3476,7 @@ static HRESULT ExecutePackageComplete( __in BURN_VARIABLES* pVariables, __in LPCWSTR wzPackageId, __in BOOL fPackageVital, + __in BOOL fAbandonedProcess, __in HRESULT hrOverall, __in HRESULT hrExecute, __in BOOL fRollback, @@ -3445,7 +3494,7 @@ static HRESULT ExecutePackageComplete( { *pRestart = BOOTSTRAPPER_APPLY_RESTART_INITIATED; } - *pfRetry = (FAILED(hrExecute) && BOOTSTRAPPER_EXECUTEPACKAGECOMPLETE_ACTION_RETRY == executePackageCompleteAction); // allow retry only on failures. + *pfRetry = (FAILED(hrExecute) && BOOTSTRAPPER_EXECUTEPACKAGECOMPLETE_ACTION_RETRY == executePackageCompleteAction && !fAbandonedProcess); // allow retry only on failures. *pfSuspend = (BOOTSTRAPPER_EXECUTEPACKAGECOMPLETE_ACTION_SUSPEND == executePackageCompleteAction); // Remember this package as the package that initiated the forced restart. diff --git a/src/burn/engine/apply.h b/src/burn/engine/apply.h index 47f0ece6..7c786cc6 100644 --- a/src/burn/engine/apply.h +++ b/src/burn/engine/apply.h @@ -14,6 +14,8 @@ enum GENERIC_EXECUTE_MESSAGE_TYPE GENERIC_EXECUTE_MESSAGE_PROGRESS, GENERIC_EXECUTE_MESSAGE_NETFX_FILES_IN_USE, GENERIC_EXECUTE_MESSAGE_PROCESS_CANCEL, + GENERIC_EXECUTE_MESSAGE_PROCESS_STARTED, + GENERIC_EXECUTE_MESSAGE_PROCESS_COMPLETED, }; typedef struct _APPLY_AUTHENTICATION_REQUIRED_DATA diff --git a/src/burn/engine/elevation.cpp b/src/burn/engine/elevation.cpp index 3c2872f1..ea56e242 100644 --- a/src/burn/engine/elevation.cpp +++ b/src/burn/engine/elevation.cpp @@ -44,6 +44,8 @@ typedef enum _BURN_ELEVATION_MESSAGE_TYPE BURN_ELEVATION_MESSAGE_TYPE_BURN_CACHE_SUCCESS, BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROGRESS, BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_CANCEL, + BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_STARTED, + BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_COMPLETED, BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_ERROR, BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_MSI_MESSAGE, BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_MSI_FILES_IN_USE, @@ -1823,6 +1825,14 @@ static HRESULT ProcessGenericExecuteMessages( ExitOnFailure(hr, "Failed to read processId."); break; + case BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_STARTED: + message.type = GENERIC_EXECUTE_MESSAGE_PROCESS_STARTED; + break; + + case BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_COMPLETED: + message.type = GENERIC_EXECUTE_MESSAGE_PROCESS_COMPLETED; + break; + case BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_ERROR: message.type = GENERIC_EXECUTE_MESSAGE_ERROR; @@ -3465,6 +3475,14 @@ static int GenericExecuteMessageHandler( dwMessage = BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_CANCEL; break; + case GENERIC_EXECUTE_MESSAGE_PROCESS_STARTED: + dwMessage = BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_STARTED; + break; + + case GENERIC_EXECUTE_MESSAGE_PROCESS_COMPLETED: + dwMessage = BURN_ELEVATION_MESSAGE_TYPE_EXECUTE_PROCESS_COMPLETED; + break; + case GENERIC_EXECUTE_MESSAGE_ERROR: // serialize message data hr = BuffWriteNumber(&pbData, &cbData, pMessage->error.dwErrorCode); diff --git a/src/burn/engine/engine.mc b/src/burn/engine/engine.mc index 9e139661..32473252 100644 --- a/src/burn/engine/engine.mc +++ b/src/burn/engine/engine.mc @@ -940,6 +940,13 @@ Language=English Bootstrapper application requested delayed cancel during package process progress, id: %1!ls!. Waiting... . +MessageId=365 +Severity=Success +SymbolicName=MSG_APPLY_SKIPPED_PACKAGE_WITH_ABANDONED_PROCESS +Language=English +Skipping rollback of package: %1!ls! due to abandoning its process. Continuing... +. + MessageId=370 Severity=Success SymbolicName=MSG_SESSION_BEGIN diff --git a/src/burn/engine/exeengine.cpp b/src/burn/engine/exeengine.cpp index 4c3c6fb0..a1049006 100644 --- a/src/burn/engine/exeengine.cpp +++ b/src/burn/engine/exeengine.cpp @@ -276,30 +276,33 @@ extern "C" HRESULT ExeEnginePlanAddPackage( hr = DependencyPlanPackage(NULL, pPackage, pPlan); ExitOnFailure(hr, "Failed to plan package dependency actions."); - // add execute action - if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->execute) + // add rollback action + if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->rollback) { - hr = PlanAppendExecuteAction(pPlan, &pAction); - ExitOnFailure(hr, "Failed to append execute action."); + hr = PlanAppendRollbackAction(pPlan, &pAction); + ExitOnFailure(hr, "Failed to append rollback action."); pAction->type = BURN_EXECUTE_ACTION_TYPE_EXE_PACKAGE; pAction->exePackage.pPackage = pPackage; - pAction->exePackage.action = pPackage->execute; + pAction->exePackage.action = pPackage->rollback; - LoggingSetPackageVariable(pPackage, NULL, FALSE, pLog, pVariables, NULL); // ignore errors. + LoggingSetPackageVariable(pPackage, NULL, TRUE, pLog, pVariables, NULL); // ignore errors. + + hr = PlanExecuteCheckpoint(pPlan); + ExitOnFailure(hr, "Failed to append execute checkpoint."); } - // add rollback action - if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->rollback) + // add execute action + if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->execute) { - hr = PlanAppendRollbackAction(pPlan, &pAction); - ExitOnFailure(hr, "Failed to append rollback action."); + hr = PlanAppendExecuteAction(pPlan, &pAction); + ExitOnFailure(hr, "Failed to append execute action."); pAction->type = BURN_EXECUTE_ACTION_TYPE_EXE_PACKAGE; pAction->exePackage.pPackage = pPackage; - pAction->exePackage.action = pPackage->rollback; + pAction->exePackage.action = pPackage->execute; - LoggingSetPackageVariable(pPackage, NULL, TRUE, pLog, pVariables, NULL); // ignore errors. + LoggingSetPackageVariable(pPackage, NULL, FALSE, pLog, pVariables, NULL); // ignore errors. } LExit: @@ -493,6 +496,10 @@ extern "C" HRESULT ExeEngineRunProcess( ExitWithLastError(hr, "Failed to CreateProcess on path: %ls", wzExecutablePath); } + message.type = GENERIC_EXECUTE_MESSAGE_PROCESS_STARTED; + message.dwUIHint = MB_OK; + pfnGenericMessageHandler(&message, pvContext); + if (fFireAndForget) { ::WaitForInputIdle(pi.hProcess, 5000); @@ -504,6 +511,7 @@ extern "C" HRESULT ExeEngineRunProcess( // Wait for the executable process while sending fake progress to allow cancel. do { + memset(&message, 0, sizeof(message)); message.type = GENERIC_EXECUTE_MESSAGE_PROGRESS; message.dwUIHint = MB_OKCANCEL; message.progress.dwPercentage = 50; @@ -546,6 +554,11 @@ extern "C" HRESULT ExeEngineRunProcess( } } while (HRESULT_FROM_WIN32(WAIT_TIMEOUT) == hr); + memset(&message, 0, sizeof(message)); + message.type = GENERIC_EXECUTE_MESSAGE_PROCESS_COMPLETED; + message.dwUIHint = MB_OK; + pfnGenericMessageHandler(&message, pvContext); + if (fDelayedCancel) { ExitWithRootFailure(hr, HRESULT_FROM_WIN32(ERROR_INSTALL_USEREXIT), "Bootstrapper application cancelled during package process progress, exit code: 0x%x", *pdwExitCode); diff --git a/src/burn/engine/msuengine.cpp b/src/burn/engine/msuengine.cpp index 2f1fb61c..2591973f 100644 --- a/src/burn/engine/msuengine.cpp +++ b/src/burn/engine/msuengine.cpp @@ -222,30 +222,33 @@ extern "C" HRESULT MsuEnginePlanAddPackage( hr = DependencyPlanPackage(NULL, pPackage, pPlan); ExitOnFailure(hr, "Failed to plan package dependency actions."); - // add execute action - if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->execute) + // add rollback action + if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->rollback) { - hr = PlanAppendExecuteAction(pPlan, &pAction); - ExitOnFailure(hr, "Failed to append execute action."); + hr = PlanAppendRollbackAction(pPlan, &pAction); + ExitOnFailure(hr, "Failed to append rollback action."); pAction->type = BURN_EXECUTE_ACTION_TYPE_MSU_PACKAGE; pAction->msuPackage.pPackage = pPackage; - pAction->msuPackage.action = pPackage->execute; + pAction->msuPackage.action = pPackage->rollback; - LoggingSetPackageVariable(pPackage, NULL, FALSE, pLog, pVariables, &pAction->msuPackage.sczLogPath); // ignore errors. + LoggingSetPackageVariable(pPackage, NULL, TRUE, pLog, pVariables, &pAction->msuPackage.sczLogPath); // ignore errors. + + hr = PlanExecuteCheckpoint(pPlan); + ExitOnFailure(hr, "Failed to append execute checkpoint."); } - // add rollback action - if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->rollback) + // add execute action + if (BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->execute) { - hr = PlanAppendRollbackAction(pPlan, &pAction); - ExitOnFailure(hr, "Failed to append rollback action."); + hr = PlanAppendExecuteAction(pPlan, &pAction); + ExitOnFailure(hr, "Failed to append execute action."); pAction->type = BURN_EXECUTE_ACTION_TYPE_MSU_PACKAGE; pAction->msuPackage.pPackage = pPackage; - pAction->msuPackage.action = pPackage->rollback; + pAction->msuPackage.action = pPackage->execute; - LoggingSetPackageVariable(pPackage, NULL, TRUE, pLog, pVariables, &pAction->msuPackage.sczLogPath); // ignore errors. + LoggingSetPackageVariable(pPackage, NULL, FALSE, pLog, pVariables, &pAction->msuPackage.sczLogPath); // ignore errors. } LExit: diff --git a/src/burn/engine/package.h b/src/burn/engine/package.h index eb40443d..e3e39c51 100644 --- a/src/burn/engine/package.h +++ b/src/burn/engine/package.h @@ -282,6 +282,7 @@ typedef struct _BURN_PACKAGE LPWSTR sczCacheFolder; // only valid during Apply. HRESULT hrCacheResult; // only valid during Apply. BOOL fReachedExecution; // only valid during Apply. + BOOL fAbandonedProcess; // only valid during Apply. BURN_PACKAGE_REGISTRATION_STATE cacheRegistrationState; // initialized during Detect, updated during Apply. BURN_PACKAGE_REGISTRATION_STATE installRegistrationState; // initialized during Detect, updated during Apply. diff --git a/src/burn/test/BurnUnitTest/PlanTest.cpp b/src/burn/test/BurnUnitTest/PlanTest.cpp index 770922b4..2febe277 100644 --- a/src/burn/test/BurnUnitTest/PlanTest.cpp +++ b/src/burn/test/BurnUnitTest/PlanTest.cpp @@ -715,6 +715,7 @@ namespace Bootstrapper ValidateExecuteRollbackBoundaryStart(pPlan, fRollback, dwIndex++, L"WixDefaultBoundary", TRUE, FALSE); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteWaitCachePackage(pPlan, fRollback, dwIndex++, L"ExeA"); + ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteExePackage(pPlan, fRollback, dwIndex++, L"ExeA", BOOTSTRAPPER_ACTION_STATE_INSTALL); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); @@ -729,6 +730,7 @@ namespace Bootstrapper ValidateExecuteExePackage(pPlan, fRollback, dwIndex++, L"ExeA", BOOTSTRAPPER_ACTION_STATE_UNINSTALL); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); + ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteRollbackBoundaryEnd(pPlan, fRollback, dwIndex++); Assert::Equal(dwIndex, pPlan->cRollbackActions); @@ -1682,6 +1684,7 @@ namespace Bootstrapper ValidateExecuteRollbackBoundaryStart(pPlan, fRollback, dwIndex++, L"WixDefaultBoundary", TRUE, FALSE); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteWaitCachePackage(pPlan, fRollback, dwIndex++, L"test.msu"); + ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteMsuPackage(pPlan, fRollback, dwIndex++, L"test.msu", BOOTSTRAPPER_ACTION_STATE_INSTALL); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); @@ -1697,6 +1700,7 @@ namespace Bootstrapper ValidateExecuteMsuPackage(pPlan, fRollback, dwIndex++, L"test.msu", BOOTSTRAPPER_ACTION_STATE_UNINSTALL); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); + ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteRollbackBoundaryEnd(pPlan, fRollback, dwIndex++); Assert::Equal(dwIndex, pPlan->cRollbackActions); diff --git a/src/test/burn/TestBA/TestBA.cs b/src/test/burn/TestBA/TestBA.cs index c219ce9c..fdbcc6d4 100644 --- a/src/test/burn/TestBA/TestBA.cs +++ b/src/test/burn/TestBA/TestBA.cs @@ -4,6 +4,7 @@ namespace WixToolset.Test.BA { using System; using System.Collections.Generic; + using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -37,6 +38,7 @@ namespace WixToolset.Test.BA private string cancelExecuteActionName; private int cancelOnProgressAtProgress; private int retryExecuteFilesInUse; + private bool rollingBack; private IBootstrapperCommand Command { get; } @@ -350,6 +352,8 @@ namespace WixToolset.Test.BA { this.Log("OnExecutePackageBegin() - package: {0}, rollback: {1}", args.PackageId, !args.ShouldExecute); + this.rollingBack = !args.ShouldExecute; + string slowProgress = this.ReadPackageAction(args.PackageId, "SlowExecute"); if (String.IsNullOrEmpty(slowProgress) || !Int32.TryParse(slowProgress, out this.sleepDuringExecute)) { @@ -404,7 +408,7 @@ namespace WixToolset.Test.BA if (!String.IsNullOrEmpty(recordTestRegistryValue) && Boolean.TryParse(recordTestRegistryValue, out logTestRegistryValue) && logTestRegistryValue) { var value = this.ReadTestRegistryValue(args.PackageId); - this.Log("TestRegistryValue: {0}, Version, '{1}'", args.PackageId, value); + this.Log("TestRegistryValue: {0}, {1}, Version, '{2}'", this.rollingBack ? "Rollback" : "Execute", args.PackageId, value); } } @@ -419,8 +423,22 @@ namespace WixToolset.Test.BA if (args.Action == BOOTSTRAPPER_EXECUTEPROCESSCANCEL_ACTION.Abandon) { - // Give time to the process to start before its files are deleted. - Thread.Sleep(2000); + // Kill process to make sure it doesn't affect other tests. + try + { + using (Process process = Process.GetProcessById(args.ProcessId)) + { + if (process != null) + { + process.Kill(); + } + } + } + catch (Exception e) + { + this.Log("Failed to kill process {0}: {1}", args.ProcessId, e); + Thread.Sleep(5000); + } } this.Log("OnExecuteProcessCancel({0})", args.Action); @@ -494,6 +512,7 @@ namespace WixToolset.Test.BA this.cancelOnProgressAtProgress = -1; this.cancelExecuteAtProgress = -1; this.cancelCacheAtProgress = -1; + this.rollingBack = false; } protected override void OnApplyComplete(ApplyCompleteEventArgs args) diff --git a/src/test/burn/WixToolsetTest.BurnE2E/FailureTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/FailureTests.cs index bbc0b387..b50be49a 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/FailureTests.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/FailureTests.cs @@ -25,12 +25,8 @@ namespace WixToolsetTest.BurnE2E var logPath = bundleD.Install((int)MSIExec.MSIExecReturnCode.ERROR_INSTALL_USEREXIT); bundleD.VerifyUnregisteredAndRemovedFromPackageCache(); - Assert.True(LogVerifier.MessageInLogFile(logPath, "TestRegistryValue: ExeA, Version, ''")); - - // Make sure ExeA finishes running. - Thread.Sleep(3000); - - bundleD.VerifyExeTestRegistryValue("ExeA", "1.0.0.0"); + Assert.True(LogVerifier.MessageInLogFile(logPath, "TestRegistryValue: Execute, ExeA, Version, ''")); + Assert.False(LogVerifier.MessageInLogFile(logPath, "TestRegistryValue: Rollback, ExeA, Version")); } [Fact] @@ -47,9 +43,11 @@ namespace WixToolsetTest.BurnE2E var logPath = bundleD.Install((int)MSIExec.MSIExecReturnCode.ERROR_INSTALL_USEREXIT); bundleD.VerifyUnregisteredAndRemovedFromPackageCache(); - Assert.True(LogVerifier.MessageInLogFile(logPath, "TestRegistryValue: ExeA, Version, '1.0.0.0'")); + Assert.True(LogVerifier.MessageInLogFile(logPath, "TestRegistryValue: Execute, ExeA, Version, '1.0.0.0'")); + Assert.True(LogVerifier.MessageInLogFile(logPath, "TestRegistryValue: Rollback, ExeA, Version, ''")); - bundleD.VerifyExeTestRegistryValue("ExeA", "1.0.0.0"); + // The package should have rolled back. + bundleD.VerifyExeTestRegistryRootDeleted("ExeA"); } [Fact] -- cgit v1.2.3-55-g6feb