@joebigelow / wix / commits / 8b8029f1

Synchronize access to cOverallProgressTicks between Cache and Execute.

#4414

Sean Hall committed May 2, 2021 at 17:49 UTC 8b8029f15e6e3de2fcf855037fe75dbb765475dc
4 files changed +60 -60
src/burn/engine/apply.cpp
+27 -35
@@ -56,11 +56,11 @@ typedef struct _BURN_CACHE_PROGRESS_CONTEXT
56 typedef struct _BURN_EXECUTE_CONTEXT
57 {
58 BURN_USER_EXPERIENCE* pUX;
59 + BURN_APPLY_CONTEXT* pApplyContext;
60 BOOL fRollback;
61 BURN_PACKAGE* pExecutingPackage;
62 DWORD cExecutedPackages;
63 DWORD cExecutePackagesTotal;
63 - DWORD* pcOverallProgressTicks;
64 } BURN_EXECUTE_CONTEXT;
65
66
@@ -187,7 +187,6 @@ static void DoRollbackCache(
187 static HRESULT DoExecuteAction(
188 __in BURN_ENGINE_STATE* pEngineState,
189 __in BURN_EXECUTE_ACTION* pExecuteAction,
190 - __in_opt HANDLE hCacheThread,
190 __in BURN_EXECUTE_CONTEXT* pContext,
191 __inout BURN_ROLLBACK_BOUNDARY** ppRollbackBoundary,
192 __inout BURN_EXECUTE_ACTION_CHECKPOINT** ppCheckpoint,
@@ -284,7 +283,7 @@ static HRESULT ReportOverallProgressTicks(
283 __in BURN_USER_EXPERIENCE* pUX,
284 __in BOOL fRollback,
285 __in DWORD cOverallProgressTicksTotal,
287 - __in DWORD cOverallProgressTicks
286 + __in BURN_APPLY_CONTEXT* pApplyContext
287 );
288 static HRESULT ExecutePackageComplete(
289 __in BURN_USER_EXPERIENCE* pUX,
@@ -499,8 +498,7 @@ extern "C" HRESULT ApplyCache(
498 __in BURN_VARIABLES* pVariables,
499 __in BURN_PLAN* pPlan,
500 __in HANDLE hPipe,
502 - __inout DWORD* pcOverallProgressTicks,
503 - __inout BOOL* pfRollback
501 + __in BURN_APPLY_CONTEXT* pContext
502 )
503 {
504 HRESULT hr = S_OK;
@@ -508,8 +506,6 @@ extern "C" HRESULT ApplyCache(
506 BURN_CACHE_CONTEXT cacheContext = { };
507 BURN_PACKAGE* pPackage = NULL;
508
511 - *pfRollback = FALSE;
512 -
509 hr = UserExperienceOnCacheBegin(pUX);
510 ExitOnRootFailure(hr, "BA aborted cache.");
511
@@ -539,9 +535,7 @@ extern "C" HRESULT ApplyCache(
535 hr = ApplyLayoutBundle(&cacheContext, pCacheAction->bundleLayout.pPayloadGroup, pCacheAction->bundleLayout.sczExecutableName, pCacheAction->bundleLayout.sczUnverifiedPath, pCacheAction->bundleLayout.qwBundleSize);
536 ExitOnFailure(hr, "Failed cache action: %ls", L"layout bundle");
537
542 - ++(*pcOverallProgressTicks);
543 -
544 - hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, *pcOverallProgressTicks);
538 + hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, pContext);
539 LogExitOnFailure(hr, MSG_USER_CANCELED, "Cancel during cache: %ls", L"layout bundle");
540
541 break;
@@ -567,9 +561,7 @@ extern "C" HRESULT ApplyCache(
561 hr = ApplyCachePackage(&cacheContext, pPackage);
562 ExitOnFailure(hr, "Failed cache action: %ls", L"cache package");
563
570 - ++(*pcOverallProgressTicks);
571 -
572 - hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, *pcOverallProgressTicks);
564 + hr = ReportOverallProgressTicks(pUX, FALSE, pPlan->cOverallProgressTicksTotal, pContext);
565 LogExitOnFailure(hr, MSG_USER_CANCELED, "Cancel during cache: %ls", L"cache package");
566
567 break;
@@ -598,7 +590,7 @@ LExit:
590 if (FAILED(hr))
591 {
592 DoRollbackCache(pUX, pPlan, hPipe, dwCheckpoint);
601 - *pfRollback = TRUE;
593 + pContext->fRollback = TRUE;
594 }
595
596 // Clean up any remanents in the cache.
@@ -622,9 +614,7 @@ LExit:
614
615 extern "C" HRESULT ApplyExecute(
616 __in BURN_ENGINE_STATE* pEngineState,
625 - __in_opt HANDLE hCacheThread,
626 - __inout DWORD* pcOverallProgressTicks,
627 - __out BOOL* pfRollback,
617 + __in BURN_APPLY_CONTEXT* pApplyContext,
618 __out BOOL* pfSuspend,
619 __out BOOTSTRAPPER_APPLY_RESTART* pRestart
620 )
@@ -637,10 +627,9 @@ extern "C" HRESULT ApplyExecute(
627 BOOL fSeekNextRollbackBoundary = FALSE;
628
629 context.pUX = &pEngineState->userExperience;
630 + context.pApplyContext = pApplyContext;
631 context.cExecutePackagesTotal = pEngineState->plan.cExecutePackagesTotal;
641 - context.pcOverallProgressTicks = pcOverallProgressTicks;
632
643 - *pfRollback = FALSE;
633 *pfSuspend = FALSE;
634
635 // Send execute begin to BA.
@@ -670,7 +659,7 @@ extern "C" HRESULT ApplyExecute(
659 }
660
661 // Execute the action.
673 - hr = DoExecuteAction(pEngineState, pExecuteAction, hCacheThread, &context, &pRollbackBoundary, &pCheckpoint, pfSuspend, pRestart);
662 + hr = DoExecuteAction(pEngineState, pExecuteAction, &context, &pRollbackBoundary, &pCheckpoint, pfSuspend, pRestart);
663
664 if (*pfSuspend || BOOTSTRAPPER_APPLY_RESTART_INITIATED == *pRestart)
665 {
@@ -698,7 +687,7 @@ extern "C" HRESULT ApplyExecute(
687 IgnoreRollbackError(hrRollback, "Failed commit transaction from disable rollback");
688 }
689
701 - *pfRollback = TRUE;
690 + pApplyContext->fRollback = TRUE;
691 break;
692 }
693
@@ -719,7 +708,7 @@ extern "C" HRESULT ApplyExecute(
708 // If the rollback boundary is vital, end execution here.
709 if (pRollbackBoundary && pRollbackBoundary->fVital)
710 {
722 - *pfRollback = TRUE;
711 + pApplyContext->fRollback = TRUE;
712 break;
713 }
714
@@ -2163,7 +2152,6 @@ static void DoRollbackCache(
2152 static HRESULT DoExecuteAction(
2153 __in BURN_ENGINE_STATE* pEngineState,
2154 __in BURN_EXECUTE_ACTION* pExecuteAction,
2166 - __in_opt HANDLE hCacheThread,
2155 __in BURN_EXECUTE_CONTEXT* pContext,
2156 __inout BURN_ROLLBACK_BOUNDARY** ppRollbackBoundary,
2157 __inout BURN_EXECUTE_ACTION_CHECKPOINT** ppCheckpoint,
@@ -2195,14 +2183,14 @@ static HRESULT DoExecuteAction(
2183 case BURN_EXECUTE_ACTION_TYPE_WAIT_SYNCPOINT:
2184 // wait for cache sync-point
2185 rghWait[0] = pExecuteAction->syncpoint.hEvent;
2198 - rghWait[1] = hCacheThread;
2186 + rghWait[1] = pContext->pApplyContext->hCacheThread;
2187 switch (::WaitForMultipleObjects(rghWait[1] ? 2 : 1, rghWait, FALSE, INFINITE))
2188 {
2189 case WAIT_OBJECT_0:
2190 break;
2191
2192 case WAIT_OBJECT_0 + 1:
2205 - if (!::GetExitCodeThread(hCacheThread, (DWORD*)&hr))
2193 + if (!::GetExitCodeThread(pContext->pApplyContext->hCacheThread, (DWORD*)&hr))
2194 {
2195 ExitWithLastError(hr, "Failed to get cache thread exit code.");
2196 }
@@ -2449,9 +2437,8 @@ static HRESULT ExecuteExePackage(
2437 ExitOnRootFailure(hr, "BA aborted EXE progress.");
2438
2439 pContext->cExecutedPackages += fRollback ? -1 : 1;
2452 - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1;
2440
2454 - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks);
2441 + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext);
2442 ExitOnRootFailure(hr, "BA aborted EXE package execute progress.");
2443
2444 LExit:
@@ -2514,9 +2501,8 @@ static HRESULT ExecuteMsiPackage(
2501 }
2502
2503 pContext->cExecutedPackages += fRollback ? -1 : 1;
2517 - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1;
2504
2519 - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks);
2505 + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext);
2506 ExitOnRootFailure(hr, "BA aborted MSI package execute progress.");
2507
2508 LExit:
@@ -2588,9 +2574,8 @@ static HRESULT ExecuteMspPackage(
2574 }
2575
2576 pContext->cExecutedPackages += fRollback ? -1 : 1;
2591 - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1;
2577
2593 - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks);
2578 + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext);
2579 ExitOnRootFailure(hr, "BA aborted MSP package execute progress.");
2580
2581 LExit:
@@ -2669,9 +2654,8 @@ static HRESULT ExecuteMsuPackage(
2654 ExitOnRootFailure(hr, "BA aborted MSU progress.");
2655
2656 pContext->cExecutedPackages += fRollback ? -1 : 1;
2672 - (*pContext->pcOverallProgressTicks) += fRollback ? -1 : 1;
2657
2674 - hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, *pContext->pcOverallProgressTicks);
2658 + hr = ReportOverallProgressTicks(&pEngineState->userExperience, fRollback, pEngineState->plan.cOverallProgressTicksTotal, pContext->pApplyContext);
2659 ExitOnRootFailure(hr, "BA aborted MSU package execute progress.");
2660
2661 LExit:
@@ -3040,15 +3024,23 @@ static HRESULT ReportOverallProgressTicks(
3024 __in BURN_USER_EXPERIENCE* pUX,
3025 __in BOOL fRollback,
3026 __in DWORD cOverallProgressTicksTotal,
3043 - __in DWORD cOverallProgressTicks
3027 + __in BURN_APPLY_CONTEXT* pApplyContext
3028 )
3029 {
3030 HRESULT hr = S_OK;
3047 - DWORD dwProgress = cOverallProgressTicksTotal ? (cOverallProgressTicks * 100 / cOverallProgressTicksTotal) : 0;
3031 + DWORD dwProgress = 0;
3032 +
3033 + ::EnterCriticalSection(&pApplyContext->csApply);
3034 +
3035 + pApplyContext->cOverallProgressTicks += fRollback ? -1 : 1;
3036 +
3037 + dwProgress = cOverallProgressTicksTotal ? (pApplyContext->cOverallProgressTicks * 100 / cOverallProgressTicksTotal) : 0;
3038
3039 // TODO: consider sending different progress numbers in the future.
3040 hr = UserExperienceOnProgress(pUX, fRollback, dwProgress, dwProgress);
3041
3042 + ::LeaveCriticalSection(&pApplyContext->csApply);
3043 +
3044 return hr;
3045 }
3046
src/burn/engine/apply.h
+2 -5
@@ -81,14 +81,11 @@ HRESULT ApplyCache(
81 __in BURN_VARIABLES* pVariables,
82 __in BURN_PLAN* pPlan,
83 __in HANDLE hPipe,
84 - __inout DWORD* pcOverallProgressTicks,
85 - __inout BOOL* pfRollback
84 + __in BURN_APPLY_CONTEXT* pContext
85 );
86 HRESULT ApplyExecute(
87 __in BURN_ENGINE_STATE* pEngineState,
89 - __in_opt HANDLE hCacheThread,
90 - __inout DWORD* pcOverallProgressTicks,
91 - __out BOOL* pfRollback,
88 + __in BURN_APPLY_CONTEXT* pApplyContext,
89 __out BOOL* pfSuspend,
90 __out BOOTSTRAPPER_APPLY_RESTART* pRestart
91 );
src/burn/engine/core.cpp
+23 -20
@@ -8,8 +8,7 @@
8 struct BURN_CACHE_THREAD_CONTEXT
9 {
10 BURN_ENGINE_STATE* pEngineState;
11 - DWORD* pcOverallProgressTicks;
12 - BOOL* pfRollback;
11 + BURN_APPLY_CONTEXT* pApplyContext;
12 };
13
14
@@ -606,14 +605,13 @@ extern "C" HRESULT CoreApply(
605 {
606 HRESULT hr = S_OK;
607 HANDLE hLock = NULL;
609 - DWORD cOverallProgressTicks = 0;
610 - HANDLE hCacheThread = NULL;
608 BOOL fApplyInitialize = FALSE;
609 BOOL fElevated = FALSE;
610 BOOL fRegistered = FALSE;
614 - BOOL fRollback = FALSE;
611 BOOL fSuspend = FALSE;
612 BOOTSTRAPPER_APPLY_RESTART restart = BOOTSTRAPPER_APPLY_RESTART_NONE;
613 + BURN_APPLY_CONTEXT applyContext = { };
614 + BOOL fDeleteApplyCs = FALSE;
615 BURN_CACHE_THREAD_CONTEXT cacheThreadContext = { };
616 DWORD dwPhaseCount = 0;
617 BOOTSTRAPPER_APPLYCOMPLETE_ACTION applyCompleteAction = BOOTSTRAPPER_APPLYCOMPLETE_ACTION_NONE;
@@ -675,6 +673,9 @@ extern "C" HRESULT CoreApply(
673 ExitFunction();
674 }
675
676 + fDeleteApplyCs = TRUE;
677 + ::InitializeCriticalSection(&applyContext.csApply);
678 +
679 // Ensure the engine is cached to the working path.
680 if (!pEngineState->sczBundleEngineWorkingPath)
681 {
@@ -707,33 +708,32 @@ extern "C" HRESULT CoreApply(
708 {
709 // Launch the cache thread.
710 cacheThreadContext.pEngineState = pEngineState;
710 - cacheThreadContext.pcOverallProgressTicks = &cOverallProgressTicks;
711 - cacheThreadContext.pfRollback = &fRollback;
711 + cacheThreadContext.pApplyContext = &applyContext;
712
713 - hCacheThread = ::CreateThread(NULL, 0, CacheThreadProc, &cacheThreadContext, 0, NULL);
714 - ExitOnNullWithLastError(hCacheThread, hr, "Failed to create cache thread.");
713 + applyContext.hCacheThread = ::CreateThread(NULL, 0, CacheThreadProc, &cacheThreadContext, 0, NULL);
714 + ExitOnNullWithLastError(applyContext.hCacheThread, hr, "Failed to create cache thread.");
715
716 // If we're not caching in parallel, wait for the cache thread to terminate.
717 if (!pEngineState->fParallelCacheAndExecute)
718 {
719 - hr = WaitForCacheThread(hCacheThread);
719 + hr = WaitForCacheThread(applyContext.hCacheThread);
720 ExitOnFailure(hr, "Failed while caching, aborting execution.");
721
722 - ReleaseHandle(hCacheThread);
722 + ReleaseHandle(applyContext.hCacheThread);
723 }
724 }
725
726 // Execute.
727 if (pEngineState->plan.cExecuteActions)
728 {
729 - hr = ApplyExecute(pEngineState, hCacheThread, &cOverallProgressTicks, &fRollback, &fSuspend, &restart);
729 + hr = ApplyExecute(pEngineState, &applyContext, &fSuspend, &restart);
730 UserExperienceExecutePhaseComplete(&pEngineState->userExperience, hr); // signal that execute completed.
731 }
732
733 // Wait for cache thread to terminate, this should return immediately unless we're waiting for layout to complete.
734 - if (hCacheThread)
734 + if (applyContext.hCacheThread)
735 {
736 - HRESULT hrCached = WaitForCacheThread(hCacheThread);
736 + HRESULT hrCached = WaitForCacheThread(applyContext.hCacheThread);
737 if (SUCCEEDED(hr))
738 {
739 hr = hrCached;
@@ -741,7 +741,7 @@ extern "C" HRESULT CoreApply(
741 }
742
743 // If something went wrong or force restarted, skip cleaning.
744 - if (FAILED(hr) || fRollback || fSuspend || BOOTSTRAPPER_APPLY_RESTART_INITIATED == restart)
744 + if (FAILED(hr) || applyContext.fRollback || fSuspend || BOOTSTRAPPER_APPLY_RESTART_INITIATED == restart)
745 {
746 ExitFunction();
747 }
@@ -756,7 +756,7 @@ LExit:
756 // Unregister.
757 if (fRegistered)
758 {
759 - ApplyUnregister(pEngineState, FAILED(hr) || fRollback, fSuspend, restart);
759 + ApplyUnregister(pEngineState, FAILED(hr) || applyContext.fRollback, fSuspend, restart);
760 }
761
762 if (fElevated)
@@ -777,7 +777,12 @@ LExit:
777 ::CloseHandle(hLock);
778 }
779
780 - ReleaseHandle(hCacheThread);
780 + ReleaseHandle(applyContext.hCacheThread);
781 +
782 + if (fDeleteApplyCs)
783 + {
784 + DeleteCriticalSection(&applyContext.csApply);
785 + }
786
787 UserExperienceOnApplyComplete(&pEngineState->userExperience, hr, restart, &applyCompleteAction);
788 if (BOOTSTRAPPER_APPLYCOMPLETE_ACTION_RESTART == applyCompleteAction)
@@ -1712,8 +1717,6 @@ static DWORD WINAPI CacheThreadProc(
1717 HRESULT hr = S_OK;
1718 BURN_CACHE_THREAD_CONTEXT* pContext = reinterpret_cast<BURN_CACHE_THREAD_CONTEXT*>(lpThreadParameter);
1719 BURN_ENGINE_STATE* pEngineState = pContext->pEngineState;
1715 - DWORD* pcOverallProgressTicks = pContext->pcOverallProgressTicks;
1716 - BOOL* pfRollback = pContext->pfRollback;
1720 BOOL fComInitialized = FALSE;
1721
1722 // initialize COM
@@ -1722,7 +1725,7 @@ static DWORD WINAPI CacheThreadProc(
1725 fComInitialized = TRUE;
1726
1727 // cache packages
1725 - hr = ApplyCache(pEngineState->section.hSourceEngineFile, &pEngineState->userExperience, &pEngineState->variables, &pEngineState->plan, pEngineState->companionConnection.hCachePipe, pcOverallProgressTicks, pfRollback);
1728 + hr = ApplyCache(pEngineState->section.hSourceEngineFile, &pEngineState->userExperience, &pEngineState->variables, &pEngineState->plan, pEngineState->companionConnection.hCachePipe, pContext->pApplyContext);
1729
1730 LExit:
1731 UserExperienceExecutePhaseComplete(&pEngineState->userExperience, hr); // signal that cache completed.
src/burn/engine/core.h
+8
@@ -134,6 +134,14 @@ typedef struct _BURN_ENGINE_STATE
134 LPWSTR* argv;
135 } BURN_ENGINE_STATE;
136
137 +typedef struct _BURN_APPLY_CONTEXT
138 +{
139 + CRITICAL_SECTION csApply;
140 + DWORD cOverallProgressTicks;
141 + BOOL fRollback;
142 + HANDLE hCacheThread;
143 +} BURN_APPLY_CONTEXT;
144 +
145
146 // function declarations
147