From 8b8029f15e6e3de2fcf855037fe75dbb765475dc Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Sun, 2 May 2021 17:49:18 -0500 Subject: Synchronize access to cOverallProgressTicks between Cache and Execute. #4414 --- src/burn/engine/apply.cpp | 62 +++++++++++++++++++++-------------------------- src/burn/engine/apply.h | 7 ++---- src/burn/engine/core.cpp | 43 +++++++++++++++++--------------- src/burn/engine/core.h | 8 ++++++ 4 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/burn/engine/apply.cpp b/src/burn/engine/apply.cpp index f0b78e81..b4fa9dac 100644 --- a/src/burn/engine/apply.cpp +++ b/src/burn/engine/apply.cpp @@ -56,11 +56,11 @@ typedef struct _BURN_CACHE_PROGRESS_CONTEXT typedef struct _BURN_EXECUTE_CONTEXT { BURN_USER_EXPERIENCE* pUX; + BURN_APPLY_CONTEXT* pApplyContext; BOOL fRollback; BURN_PACKAGE* pExecutingPackage; DWORD cExecutedPackages; DWORD cExecutePackagesTotal; - DWORD* pcOverallProgressTicks; } BURN_EXECUTE_CONTEXT; @@ -187,7 +187,6 @@ static void DoRollbackCache( static HRESULT DoExecuteAction( __in BURN_ENGINE_STATE* pEngineState, __in BURN_EXECUTE_ACTION* pExecuteAction, - __in_opt HANDLE hCacheThread, __in BURN_EXECUTE_CONTEXT* pContext, __inout BURN_ROLLBACK_BOUNDARY** ppRollbackBoundary, __inout BURN_EXECUTE_ACTION_CHECKPOINT** ppCheckpoint, @@ -284,7 +283,7 @@ static HRESULT ReportOverallProgressTicks( __in BURN_USER_EXPERIENCE* pUX, __in BOOL fRollback, __in DWORD cOverallProgressTicksTotal, - __in DWORD cOverallProgressTicks + __in BURN_APPLY_CONTEXT* pApplyContext ); static HRESULT ExecutePackageComplete( __in BURN_USER_EXPERIENCE* pUX, @@ -499,8 +498,7 @@ extern "C" HRESULT ApplyCache( __in BURN_VARIABLES* pVariables, __in BURN_PLAN* pPlan, __in HANDLE hPipe, - __inout DWORD* pcOverallProgressTicks, - __inout BOOL* pfRollback + __in BURN_APPLY_CONTEXT* pContext ) { HRESULT hr = S_OK; @@ -508,8 +506,6 @@ extern "C" HRESULT ApplyCache( BURN_CACHE_CONTEXT cacheContext = { }; BURN_PACKAGE* pPackage = NULL; - *pfRollback = FALSE; - hr = UserExperienceOnCacheBegin(pUX); ExitOnRootFailure(hr, "BA aborted cache."); @@ -539,9 +535,7 @@ extern "C" HRESULT ApplyCache( hr = ApplyLayoutBundle(&cacheContext, pCacheAction->bundleLayout.pPayloadGroup, pCacheAction->bundleLayout.sczExecutableName, pCacheAction->bundleLayout.sczUnverifiedPath, pCacheAction->bundleLayout.qwBundleSize); ExitOnFailure(hr, "Failed cache action: %ls", L"layout bundle"); - ++(*pcOverallProgressTicks); - - hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, *pcOverallProgressTicks); + hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, pContext); LogExitOnFailure(hr, MSG_USER_CANCELED, "Cancel during cache: %ls", L"layout bundle"); break; @@ -567,9 +561,7 @@ extern "C" HRESULT ApplyCache( hr = ApplyCachePackage(&cacheContext, pPackage); ExitOnFailure(hr, "Failed cache action: %ls", L"cache package"); - ++(*pcOverallProgressTicks); - - hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, *pcOverallProgressTicks); + hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, pContext); LogExitOnFailure(hr, MSG_USER_CANCELED, "Cancel during cache: %ls", L"cache package"); break; @@ -598,7 +590,7 @@ LExit: if (FAILED(hr)) { DoRollbackCache(pUX, pPlan, hPipe, dwCheckpoint); - *pfRollback = TRUE; + pContext->fRollback = TRUE; } // Clean up any remanents in the cache. @@ -622,9 +614,7 @@ LExit: extern "C" HRESULT ApplyExecute( __in BURN_ENGINE_STATE* pEngineState, - __in_opt HANDLE hCacheThread, - __inout DWORD* pcOverallProgressTicks, - __out BOOL* pfRollback, + __in BURN_APPLY_CONTEXT* pApplyContext, __out BOOL* pfSuspend, __out BOOTSTRAPPER_APPLY_RESTART* pRestart ) @@ -637,10 +627,9 @@ extern "C" HRESULT ApplyExecute( BOOL fSeekNextRollbackBoundary = FALSE; context.pUX = &pEngineState->userExperience; + context.pApplyContext = pApplyContext; context.cExecutePackagesTotal = pEngineState->plan.cExecutePackagesTotal; - context.pcOverallProgressTicks = pcOverallProgressTicks; - *pfRollback = FALSE; *pfSuspend = FALSE; // Send execute begin to BA. @@ -670,7 +659,7 @@ extern "C" HRESULT ApplyExecute( } // Execute the action. - hr = DoExecuteAction(pEngineState, pExecuteAction, hCacheThread, &context, &pRollbackBoundary, &pCheckpoint, pfSuspend, pRestart); + hr = DoExecuteAction(pEngineState, pExecuteAction, &context, &pRollbackBoundary, &pCheckpoint, pfSuspend, pRestart); if (*pfSuspend || BOOTSTRAPPER_APPLY_RESTART_INITIATED == *pRestart) { @@ -698,7 +687,7 @@ extern "C" HRESULT ApplyExecute( IgnoreRollbackError(hrRollback, "Failed commit transaction from disable rollback"); } - *pfRollback = TRUE; + pApplyContext->fRollback = TRUE; break; } @@ -719,7 +708,7 @@ extern "C" HRESULT ApplyExecute( // If the rollback boundary is vital, end execution here. if (pRollbackBoundary && pRollbackBoundary->fVital) { - *pfRollback = TRUE; + pApplyContext->fRollback = TRUE; break; } @@ -2163,7 +2152,6 @@ static void DoRollbackCache( static HRESULT DoExecuteAction( __in BURN_ENGINE_STATE* pEngineState, __in BURN_EXECUTE_ACTION* pExecuteAction, - __in_opt HANDLE hCacheThread, __in BURN_EXECUTE_CONTEXT* pContext, __inout BURN_ROLLBACK_BOUNDARY** ppRollbackBoundary, __inout BURN_EXECUTE_ACTION_CHECKPOINT** ppCheckpoint, @@ -2195,14 +2183,14 @@ static HRESULT DoExecuteAction( case BURN_EXECUTE_ACTION_TYPE_WAIT_SYNCPOINT: // wait for cache sync-point rghWait[0] = pExecuteAction->syncpoint.hEvent; - rghWait[1] = hCacheThread; + rghWait[1] = pContext->pApplyContext->hCacheThread; switch (::WaitForMultipleObjects(rghWait[1] ? 2 : 1, rghWait, FALSE, INFINITE)) { case WAIT_OBJECT_0: break; case WAIT_OBJECT_0 + 1: - if (!::GetExitCodeThread(hCacheThread, (DWORD*)&hr)) + if (!::GetExitCodeThread(pContext->pApplyContext->hCacheThread, (DWORD*)&hr)) { ExitWithLastError(hr, "Failed to get cache thread exit code."); } @@ -2449,9 +2437,8 @@ static HRESULT ExecuteExePackage( ExitOnRootFailure(hr, "BA aborted EXE progress."); pContext->cExecutedPackages += fRollback ? -1 : 1; - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1; - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks); + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext); ExitOnRootFailure(hr, "BA aborted EXE package execute progress."); LExit: @@ -2514,9 +2501,8 @@ static HRESULT ExecuteMsiPackage( } pContext->cExecutedPackages += fRollback ? -1 : 1; - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1; - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks); + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext); ExitOnRootFailure(hr, "BA aborted MSI package execute progress."); LExit: @@ -2588,9 +2574,8 @@ static HRESULT ExecuteMspPackage( } pContext->cExecutedPackages += fRollback ? -1 : 1; - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1; - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks); + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext); ExitOnRootFailure(hr, "BA aborted MSP package execute progress."); LExit: @@ -2669,9 +2654,8 @@ static HRESULT ExecuteMsuPackage( ExitOnRootFailure(hr, "BA aborted MSU progress."); pContext->cExecutedPackages += fRollback ? -1 : 1; - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1; - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks); + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext); ExitOnRootFailure(hr, "BA aborted MSU package execute progress."); LExit: @@ -3040,15 +3024,23 @@ static HRESULT ReportOverallProgressTicks( __in BURN_USER_EXPERIENCE* pUX, __in BOOL fRollback, __in DWORD cOverallProgressTicksTotal, - __in DWORD cOverallProgressTicks + __in BURN_APPLY_CONTEXT* pApplyContext ) { HRESULT hr = S_OK; - DWORD dwProgress = cOverallProgressTicksTotal ? (cOverallProgressTicks * 100 / cOverallProgressTicksTotal) : 0; + DWORD dwProgress = 0; + + ::EnterCriticalSection(&pApplyContext->csApply); + + pApplyContext->cOverallProgressTicks += fRollback ? -1 : 1; + + dwProgress = cOverallProgressTicksTotal ? (pApplyContext->cOverallProgressTicks * 100 / cOverallProgressTicksTotal) : 0; // TODO: consider sending different progress numbers in the future. hr = UserExperienceOnProgress(pUX, fRollback, dwProgress, dwProgress); + ::LeaveCriticalSection(&pApplyContext->csApply); + return hr; } diff --git a/src/burn/engine/apply.h b/src/burn/engine/apply.h index 548e147d..39580297 100644 --- a/src/burn/engine/apply.h +++ b/src/burn/engine/apply.h @@ -81,14 +81,11 @@ HRESULT ApplyCache( __in BURN_VARIABLES* pVariables, __in BURN_PLAN* pPlan, __in HANDLE hPipe, - __inout DWORD* pcOverallProgressTicks, - __inout BOOL* pfRollback + __in BURN_APPLY_CONTEXT* pContext ); HRESULT ApplyExecute( __in BURN_ENGINE_STATE* pEngineState, - __in_opt HANDLE hCacheThread, - __inout DWORD* pcOverallProgressTicks, - __out BOOL* pfRollback, + __in BURN_APPLY_CONTEXT* pApplyContext, __out BOOL* pfSuspend, __out BOOTSTRAPPER_APPLY_RESTART* pRestart ); diff --git a/src/burn/engine/core.cpp b/src/burn/engine/core.cpp index 535043af..02cab7e6 100644 --- a/src/burn/engine/core.cpp +++ b/src/burn/engine/core.cpp @@ -8,8 +8,7 @@ struct BURN_CACHE_THREAD_CONTEXT { BURN_ENGINE_STATE* pEngineState; - DWORD* pcOverallProgressTicks; - BOOL* pfRollback; + BURN_APPLY_CONTEXT* pApplyContext; }; @@ -606,14 +605,13 @@ extern "C" HRESULT CoreApply( { HRESULT hr = S_OK; HANDLE hLock = NULL; - DWORD cOverallProgressTicks = 0; - HANDLE hCacheThread = NULL; BOOL fApplyInitialize = FALSE; BOOL fElevated = FALSE; BOOL fRegistered = FALSE; - BOOL fRollback = FALSE; BOOL fSuspend = FALSE; BOOTSTRAPPER_APPLY_RESTART restart = BOOTSTRAPPER_APPLY_RESTART_NONE; + BURN_APPLY_CONTEXT applyContext = { }; + BOOL fDeleteApplyCs = FALSE; BURN_CACHE_THREAD_CONTEXT cacheThreadContext = { }; DWORD dwPhaseCount = 0; BOOTSTRAPPER_APPLYCOMPLETE_ACTION applyCompleteAction = BOOTSTRAPPER_APPLYCOMPLETE_ACTION_NONE; @@ -675,6 +673,9 @@ extern "C" HRESULT CoreApply( ExitFunction(); } + fDeleteApplyCs = TRUE; + ::InitializeCriticalSection(&applyContext.csApply); + // Ensure the engine is cached to the working path. if (!pEngineState->sczBundleEngineWorkingPath) { @@ -707,33 +708,32 @@ extern "C" HRESULT CoreApply( { // Launch the cache thread. cacheThreadContext.pEngineState = pEngineState; - cacheThreadContext.pcOverallProgressTicks = &cOverallProgressTicks; - cacheThreadContext.pfRollback = &fRollback; + cacheThreadContext.pApplyContext = &applyContext; - hCacheThread = ::CreateThread(NULL, 0, CacheThreadProc, &cacheThreadContext, 0, NULL); - ExitOnNullWithLastError(hCacheThread, hr, "Failed to create cache thread."); + applyContext.hCacheThread = ::CreateThread(NULL, 0, CacheThreadProc, &cacheThreadContext, 0, NULL); + ExitOnNullWithLastError(applyContext.hCacheThread, hr, "Failed to create cache thread."); // If we're not caching in parallel, wait for the cache thread to terminate. if (!pEngineState->fParallelCacheAndExecute) { - hr = WaitForCacheThread(hCacheThread); + hr = WaitForCacheThread(applyContext.hCacheThread); ExitOnFailure(hr, "Failed while caching, aborting execution."); - ReleaseHandle(hCacheThread); + ReleaseHandle(applyContext.hCacheThread); } } // Execute. if (pEngineState->plan.cExecuteActions) { - hr = ApplyExecute(pEngineState, hCacheThread, &cOverallProgressTicks, &fRollback, &fSuspend, &restart); + hr = ApplyExecute(pEngineState, &applyContext, &fSuspend, &restart); UserExperienceExecutePhaseComplete(&pEngineState->userExperience, hr); // signal that execute completed. } // Wait for cache thread to terminate, this should return immediately unless we're waiting for layout to complete. - if (hCacheThread) + if (applyContext.hCacheThread) { - HRESULT hrCached = WaitForCacheThread(hCacheThread); + HRESULT hrCached = WaitForCacheThread(applyContext.hCacheThread); if (SUCCEEDED(hr)) { hr = hrCached; @@ -741,7 +741,7 @@ extern "C" HRESULT CoreApply( } // If something went wrong or force restarted, skip cleaning. - if (FAILED(hr) || fRollback || fSuspend || BOOTSTRAPPER_APPLY_RESTART_INITIATED == restart) + if (FAILED(hr) || applyContext.fRollback || fSuspend || BOOTSTRAPPER_APPLY_RESTART_INITIATED == restart) { ExitFunction(); } @@ -756,7 +756,7 @@ LExit: // Unregister. if (fRegistered) { - ApplyUnregister(pEngineState, FAILED(hr) || fRollback, fSuspend, restart); + ApplyUnregister(pEngineState, FAILED(hr) || applyContext.fRollback, fSuspend, restart); } if (fElevated) @@ -777,7 +777,12 @@ LExit: ::CloseHandle(hLock); } - ReleaseHandle(hCacheThread); + ReleaseHandle(applyContext.hCacheThread); + + if (fDeleteApplyCs) + { + DeleteCriticalSection(&applyContext.csApply); + } UserExperienceOnApplyComplete(&pEngineState->userExperience, hr, restart, &applyCompleteAction); if (BOOTSTRAPPER_APPLYCOMPLETE_ACTION_RESTART == applyCompleteAction) @@ -1712,8 +1717,6 @@ static DWORD WINAPI CacheThreadProc( HRESULT hr = S_OK; BURN_CACHE_THREAD_CONTEXT* pContext = reinterpret_cast(lpThreadParameter); BURN_ENGINE_STATE* pEngineState = pContext->pEngineState; - DWORD* pcOverallProgressTicks = pContext->pcOverallProgressTicks; - BOOL* pfRollback = pContext->pfRollback; BOOL fComInitialized = FALSE; // initialize COM @@ -1722,7 +1725,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->pApplyContext); LExit: UserExperienceExecutePhaseComplete(&pEngineState->userExperience, hr); // signal that cache completed. diff --git a/src/burn/engine/core.h b/src/burn/engine/core.h index e96440bb..9f779366 100644 --- a/src/burn/engine/core.h +++ b/src/burn/engine/core.h @@ -134,6 +134,14 @@ typedef struct _BURN_ENGINE_STATE LPWSTR* argv; } BURN_ENGINE_STATE; +typedef struct _BURN_APPLY_CONTEXT +{ + CRITICAL_SECTION csApply; + DWORD cOverallProgressTicks; + BOOL fRollback; + HANDLE hCacheThread; +} BURN_APPLY_CONTEXT; + // function declarations -- cgit v1.2.3-55-g6feb