Synchronize access to cOverallProgressTicks between Cache and Execute.

#4414

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<BURN_CACHE_THREAD_CONTEXT*>(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