Skip to content

Commit

Permalink
jchoover: Burn tainting the cache when cancel or error happens during…
Browse files Browse the repository at this point in the history 
… apply execute.
  • Loading branch information
@jchoover
jchoover committed Jan 19, 2018
1 parent e6dd234 commit 74a76b0
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 2 deletions.
67 changes: 67 additions & 0 deletions src/burn/engine/apply.cpp
View file
Expand Up @@ -420,6 +420,8 @@ extern "C" HRESULT ApplyCache(
__in BURN_VARIABLES* pVariables,
__in BURN_PLAN* pPlan,
__in HANDLE hPipe,
__in APPLY_SYNCHRONIZE_CACHE_THREAD* pSynchronizeCache,
__in BOOL fParallelCacheAndExecute,
__inout DWORD* pcOverallProgressTicks,
__out BOOL* pfRollback
)
Expand Down Expand Up @@ -686,6 +688,25 @@ extern "C" HRESULT ApplyCache(
}
} while (fRetry);

// Should we wait for ApplyExecute to finish, and conditionally roll back the cache plan if ApplyExecute failed?
if (SUCCEEDED(hr) && (fParallelCacheAndExecute))
{
if (!::SetEvent(pSynchronizeCache->hEventCacheComplete))
{
ExitWithLastError(hr, "Failed to set cache complete event.");
}

if (WAIT_OBJECT_0 != ::WaitForSingleObject(pSynchronizeCache->hEventApplyExecuteComplete, INFINITE))
{
ExitWithLastError(hr, "Failed to wait for apply exeucte complete event.");
}

if (FAILED(pSynchronizeCache->hrCacheExecuteRollback))
{
hr = pSynchronizeCache->hrCacheExecuteRollback;
}
}

LExit:
Assert(NULL == pStartedPackage);
Assert(BURN_PLAN_INVALID_ACTION_INDEX == iPackageStartAction);
Expand All @@ -712,6 +733,7 @@ extern "C" HRESULT ApplyCache(
extern "C" HRESULT ApplyExecute(
__in BURN_ENGINE_STATE* pEngineState,
__in_opt HANDLE hCacheThread,
__in APPLY_SYNCHRONIZE_CACHE_THREAD* pSynchronizeCache,
__inout DWORD* pcOverallProgressTicks,
__out BOOL* pfKeepRegistration,
__out BOOL* pfRollback,
Expand Down Expand Up @@ -792,6 +814,51 @@ extern "C" HRESULT ApplyExecute(
}

LExit:
if (FAILED(hr) && !*pfRollback)
{
// Origionally I had additional conditions below, but my current thought is the plan should win. If we are doing a
// Repair/Uninstall, then the CacheRollback shouldn't have any of the existing cached packages in the plan, as they
// should already be on the machine.
// && !pEngineState->registration.fInstalled && (BOOTSTRAPPER_ACTION_INSTALL == pEngineState->plan.action)
if (pEngineState->fParallelCacheAndExecute)
{
pSynchronizeCache->hrCacheExecuteRollback = hr;
if (!::SetEvent(pSynchronizeCache->hEventApplyExecuteComplete))
{
// ExitWithLastError(hr, "Failed to set apply execute complete event.");
{ DWORD Dutil_er = ::GetLastError(); HRESULT x = HRESULT_FROM_WIN32(Dutil_er); if (!FAILED(x)) { x = E_FAIL; } Dutil_RootFailure(__FILE__, __LINE__, x); ExitTrace(x, "Failed to set apply execute complete event."); }
}
}
else
{
// Scan to find the last checkpoint.
dwCheckpoint = 0;
for (DWORD i = pEngineState->plan.cRollbackCacheActions - 1; i >= 0; --i)
{
BURN_CACHE_ACTION* pRollbackCacheAction = &pEngineState->plan.rgRollbackCacheActions[i];

if (BURN_CACHE_ACTION_TYPE_CHECKPOINT == pRollbackCacheAction->type)
{
dwCheckpoint = i;
break;
}
}

DoRollbackCache(&pEngineState->userExperience, &pEngineState->plan, pEngineState->companionConnection.hPipe, dwCheckpoint);
}
}
else
{
if (pEngineState->fParallelCacheAndExecute)
{
if (!::SetEvent(pSynchronizeCache->hEventApplyExecuteComplete))
{
// ExitWithLastError(hr, "Failed to set apply execute complete event.");
{ DWORD Dutil_er = ::GetLastError(); HRESULT x = HRESULT_FROM_WIN32(Dutil_er); if (!FAILED(x)) { x = E_FAIL; } Dutil_RootFailure(__FILE__, __LINE__, x); ExitTrace(x, "Failed to set apply execute complete event."); }
}
}
}

// Send execute complete to BA.
pEngineState->userExperience.pUserExperience->OnExecuteComplete(hr);

Expand Down
9 changes: 9 additions & 0 deletions src/burn/engine/apply.h
View file
Expand Up @@ -46,6 +46,12 @@ typedef struct _GENERIC_EXECUTE_MESSAGE
};
} GENERIC_EXECUTE_MESSAGE;

typedef struct _APPLY_SYNCHRONIZE_CACHE_THREAD
{
HANDLE hEventCacheComplete;
HANDLE hEventApplyExecuteComplete;
HRESULT hrCacheExecuteRollback;
} APPLY_SYNCHRONIZE_CACHE_THREAD;

typedef int (*PFN_GENERICMESSAGEHANDLER)(
__in GENERIC_EXECUTE_MESSAGE* pMessage,
Expand Down Expand Up @@ -82,12 +88,15 @@ HRESULT ApplyCache(
__in BURN_VARIABLES* pVariables,
__in BURN_PLAN* pPlan,
__in HANDLE hPipe,
__in APPLY_SYNCHRONIZE_CACHE_THREAD* pSynchronizeCache,
__in BOOL fParallelCacheAndExecute,
__inout DWORD* pcOverallProgressTicks,
__out BOOL* pfRollback
);
HRESULT ApplyExecute(
__in BURN_ENGINE_STATE* pEngineState,
__in_opt HANDLE hCacheThread,
__in APPLY_SYNCHRONIZE_CACHE_THREAD* pSynchronizeCache,
__inout DWORD* pcOverallProgressTicks,
__out BOOL* pfKeepRegistration,
__out BOOL* pfRollback,
Expand Down
54 changes: 52 additions & 2 deletions src/burn/engine/core.cpp
View file
Expand Up @@ -10,6 +10,7 @@ struct BURN_CACHE_THREAD_CONTEXT
BURN_ENGINE_STATE* pEngineState;
DWORD* pcOverallProgressTicks;
BOOL* pfRollback;
APPLY_SYNCHRONIZE_CACHE_THREAD* pSychronizeCache;
};


Expand Down Expand Up @@ -47,7 +48,13 @@ static DWORD WINAPI CacheThreadProc(
);
static HRESULT WaitForCacheThread(
__in HANDLE hCacheThread
);
#ifdef CACHE_THREAD_ALWAYS_CLEANUP
static HRESULT WaitForCacheThread(
__in HANDLE hCacheThread,
__in HANDLE hEventCacheComplete
);
#endif
static void LogPackages(
__in_opt const BURN_PACKAGE* pUpgradeBundlePackage,
__in_opt const BURN_PACKAGE* pForwardCompatibleBundlePackage,
Expand Down Expand Up @@ -552,6 +559,7 @@ extern "C" HRESULT CoreApply(
BOOL fSuspend = FALSE;
BOOTSTRAPPER_APPLY_RESTART restart = BOOTSTRAPPER_APPLY_RESTART_NONE;
BURN_CACHE_THREAD_CONTEXT cacheThreadContext = { };
APPLY_SYNCHRONIZE_CACHE_THREAD synchronizeCache = {};
DWORD dwPhaseCount = 0;

LogId(REPORT_STANDARD, MSG_APPLY_BEGIN);
Expand Down Expand Up @@ -634,6 +642,15 @@ extern "C" HRESULT CoreApply(
// Cache.
if (pEngineState->plan.cCacheActions)
{
if (pEngineState->fParallelCacheAndExecute)
{
synchronizeCache.hEventCacheComplete = ::CreateEventW(NULL, TRUE, FALSE, NULL);
synchronizeCache.hEventApplyExecuteComplete = ::CreateEventW(NULL, TRUE, FALSE, NULL);
synchronizeCache.hrCacheExecuteRollback = S_OK;

cacheThreadContext.pSychronizeCache = &synchronizeCache;
}

// Launch the cache thread.
cacheThreadContext.pEngineState = pEngineState;
cacheThreadContext.pcOverallProgressTicks = &cOverallProgressTicks;
Expand All @@ -655,7 +672,7 @@ extern "C" HRESULT CoreApply(
// Execute.
if (pEngineState->plan.cExecuteActions)
{
hr = ApplyExecute(pEngineState, hCacheThread, &cOverallProgressTicks, &fKeepRegistration, &fRollback, &fSuspend, &restart);
hr = ApplyExecute(pEngineState, hCacheThread, &synchronizeCache, &cOverallProgressTicks, &fKeepRegistration, &fRollback, &fSuspend, &restart);
UserExperienceExecutePhaseComplete(&pEngineState->userExperience, hr); // signal that execute completed.
}

Expand Down Expand Up @@ -709,6 +726,8 @@ extern "C" HRESULT CoreApply(
}

ReleaseHandle(hCacheThread);
ReleaseHandle(synchronizeCache.hEventCacheComplete);
ReleaseHandle(synchronizeCache.hEventApplyExecuteComplete);

nResult = pEngineState->userExperience.pUserExperience->OnApplyComplete(hr, restart);
if (IDRESTART == nResult)
Expand Down Expand Up @@ -1558,7 +1577,7 @@ static DWORD WINAPI CacheThreadProc(
fComInitialized = TRUE;

// cache packages
hr = ApplyCache(pEngineState->section.hSourceEngineFile, &pEngineState->userExperience, &pEngineState->variables, &pEngineState->plan, pEngineState->companionConnection.hCachePipe, pcOverallProgressTicks, pfRollback);
hr = ApplyCache(pEngineState->section.hSourceEngineFile, &pEngineState->userExperience, &pEngineState->variables, &pEngineState->plan, pEngineState->companionConnection.hCachePipe,pContext->pSychronizeCache, pEngineState->fParallelCacheAndExecute, pcOverallProgressTicks, pfRollback);

LExit:
UserExperienceExecutePhaseComplete(&pEngineState->userExperience, hr); // signal that cache completed.
Expand Down Expand Up @@ -1591,6 +1610,37 @@ static HRESULT WaitForCacheThread(
return hr;
}

#ifdef CACHE_THREAD_ALWAYS_CLEANUP
static HRESULT WaitForCacheThread(
__in HANDLE hCacheThread,
__in HANDLE hEventCacheComplete
)
{
HRESULT hr = S_OK;
HANDLE rghSyncOpt[2];

rghSyncOpt[0] = hCacheThread;
rghSyncOpt[1] = hEventCacheComplete;

switch (::WaitForMultipleObjects(2, rghSyncOpt, FALSE, INFINITE))
{
case WAIT_OBJECT_0:
if (!::GetExitCodeThread(hCacheThread, (DWORD*)&hr))
{
ExitWithLastError(hr, "Failed to get cache thread exit code.");
}
break;
case WAIT_OBJECT_0+1:
break;
default:
ExitWithLastError(hr, "Failed to wait for cache thread to terminate.");
}

LExit:
return hr;
}
#endif

static void LogPackages(
__in_opt const BURN_PACKAGE* pUpgradeBundlePackage,
__in_opt const BURN_PACKAGE* pForwardCompatibleBundlePackage,
Expand Down
12 changes: 12 additions & 0 deletions src/burn/engine/plan.cpp
View file
Expand Up @@ -516,6 +516,18 @@ extern "C" HRESULT PlanPackages(
}
}

// Insert a trailing checkpoint to the end of the rollback cache plan.
if (!fBundleInstalled && (BOOTSTRAPPER_ACTION_INSTALL == pPlan->action))
{
BURN_CACHE_ACTION* pCacheAction = NULL;
DWORD dwCheckpoint = GetNextCheckpointId();
hr = AppendRollbackCacheAction(pPlan, &pCacheAction);
ExitOnFailure(hr, "Failed to append rollback cache action.");

pCacheAction->type = BURN_CACHE_ACTION_TYPE_CHECKPOINT;
pCacheAction->checkpoint.dwId = dwCheckpoint;
}

// Insert the "keep registration" and "remove registration" actions in the plan when installing the first time and anytime we are uninstalling respectively.
if (!fBundleInstalled && (BOOTSTRAPPER_ACTION_INSTALL == pPlan->action || BOOTSTRAPPER_ACTION_MODIFY == pPlan->action || BOOTSTRAPPER_ACTION_REPAIR == pPlan->action))
{
Expand Down

0 comments on commit 74a76b0

Please sign in to comment.