From 98080672cdbbde00ea40a96c1ce38e8a52f24fee Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Wed, 19 Oct 2022 15:44:40 -0500 Subject: Add queutil so Burn can manage its own queue of BA requested actions. Fixes 6349 --- src/burn/engine/EngineForApplication.cpp | 12 +- src/burn/engine/EngineForApplication.h | 33 +++- src/burn/engine/approvedexe.cpp | 1 - src/burn/engine/core.cpp | 31 ++-- src/burn/engine/core.h | 9 +- src/burn/engine/elevation.cpp | 5 +- src/burn/engine/engine.cpp | 95 ++++++------ src/burn/engine/externalengine.cpp | 166 ++++++++++++++------- src/burn/engine/externalengine.h | 14 +- src/burn/engine/platform.h | 1 + src/burn/engine/precomp.h | 1 + src/libs/dutil/WixToolset.DUtil/dutil.vcxproj | 2 + .../dutil/WixToolset.DUtil/dutil.vcxproj.filters | 6 + src/libs/dutil/WixToolset.DUtil/inc/dutilsources.h | 1 + src/libs/dutil/WixToolset.DUtil/inc/queutil.h | 52 +++++++ src/libs/dutil/WixToolset.DUtil/precomp.h | 1 + src/libs/dutil/WixToolset.DUtil/queutil.cpp | 144 ++++++++++++++++++ .../burn/WixToolsetTest.BurnE2E/ElevationTests.cs | 2 +- 18 files changed, 451 insertions(+), 125 deletions(-) create mode 100644 src/libs/dutil/WixToolset.DUtil/inc/queutil.h create mode 100644 src/libs/dutil/WixToolset.DUtil/queutil.cpp diff --git a/src/burn/engine/EngineForApplication.cpp b/src/burn/engine/EngineForApplication.cpp index 27f815c6..45bfaf83 100644 --- a/src/burn/engine/EngineForApplication.cpp +++ b/src/burn/engine/EngineForApplication.cpp @@ -325,7 +325,7 @@ static HRESULT BAEngineDetect( ValidateMessageArgs(hr, pvArgs, BAENGINE_DETECT_ARGS, pArgs); ValidateMessageResults(hr, pvResults, BAENGINE_DETECT_RESULTS, pResults); - hr = ExternalEngineDetect(pContext->dwThreadId, pArgs->hwndParent); + hr = ExternalEngineDetect(pContext, pArgs->hwndParent); LExit: return hr; @@ -341,7 +341,7 @@ static HRESULT BAEnginePlan( ValidateMessageArgs(hr, pvArgs, BAENGINE_PLAN_ARGS, pArgs); ValidateMessageResults(hr, pvResults, BAENGINE_PLAN_RESULTS, pResults); - hr = ExternalEnginePlan(pContext->dwThreadId, pArgs->action); + hr = ExternalEnginePlan(pContext, pArgs->action); LExit: return hr; @@ -357,7 +357,7 @@ static HRESULT BAEngineElevate( ValidateMessageArgs(hr, pvArgs, BAENGINE_ELEVATE_ARGS, pArgs); ValidateMessageResults(hr, pvResults, BAENGINE_ELEVATE_RESULTS, pResults); - hr = ExternalEngineElevate(pContext->pEngineState, pContext->dwThreadId, pArgs->hwndParent); + hr = ExternalEngineElevate(pContext, pArgs->hwndParent); LExit: return hr; @@ -373,7 +373,7 @@ static HRESULT BAEngineApply( ValidateMessageArgs(hr, pvArgs, BAENGINE_APPLY_ARGS, pArgs); ValidateMessageResults(hr, pvResults, BAENGINE_APPLY_RESULTS, pResults); - hr = ExternalEngineApply(pContext->dwThreadId, pArgs->hwndParent); + hr = ExternalEngineApply(pContext, pArgs->hwndParent); LExit: return hr; @@ -389,7 +389,7 @@ static HRESULT BAEngineQuit( ValidateMessageArgs(hr, pvArgs, BAENGINE_QUIT_ARGS, pArgs); ValidateMessageResults(hr, pvResults, BAENGINE_QUIT_RESULTS, pResults); - hr = ExternalEngineQuit(pContext->dwThreadId, pArgs->dwExitCode); + hr = ExternalEngineQuit(pContext, pArgs->dwExitCode); LExit: return hr; @@ -405,7 +405,7 @@ static HRESULT BAEngineLaunchApprovedExe( ValidateMessageArgs(hr, pvArgs, BAENGINE_LAUNCHAPPROVEDEXE_ARGS, pArgs); ValidateMessageResults(hr, pvResults, BAENGINE_LAUNCHAPPROVEDEXE_RESULTS, pResults); - hr = ExternalEngineLaunchApprovedExe(pContext->pEngineState, pContext->dwThreadId, pArgs->hwndParent, pArgs->wzApprovedExeForElevationId, pArgs->wzArguments, pArgs->dwWaitForInputIdleTimeout); + hr = ExternalEngineLaunchApprovedExe(pContext, pArgs->hwndParent, pArgs->wzApprovedExeForElevationId, pArgs->wzArguments, pArgs->dwWaitForInputIdleTimeout); LExit: return hr; diff --git a/src/burn/engine/EngineForApplication.h b/src/burn/engine/EngineForApplication.h index d25a7e51..bf86b7ee 100644 --- a/src/burn/engine/EngineForApplication.h +++ b/src/burn/engine/EngineForApplication.h @@ -11,9 +11,40 @@ extern "C" { typedef struct _BOOTSTRAPPER_ENGINE_CONTEXT { BURN_ENGINE_STATE* pEngineState; - DWORD dwThreadId; + QUEUTIL_QUEUE_HANDLE hQueue; + HANDLE hQueueSemaphore; + CRITICAL_SECTION csQueue; } BOOTSTRAPPER_ENGINE_CONTEXT; +typedef struct _BOOTSTRAPPER_ENGINE_ACTION +{ + WM_BURN dwMessage; + union + { + struct + { + HWND hwndParent; + } detect; + struct + { + BOOTSTRAPPER_ACTION action; + } plan; + struct + { + HWND hwndParent; + } elevate; + struct + { + HWND hwndParent; + } apply; + BURN_LAUNCH_APPROVED_EXE launchApprovedExe; + struct + { + DWORD dwExitCode; + } quit; + }; +} BOOTSTRAPPER_ENGINE_ACTION; + // function declarations HRESULT WINAPI EngineForApplicationProc( diff --git a/src/burn/engine/approvedexe.cpp b/src/burn/engine/approvedexe.cpp index d8bd956b..28b26d6d 100644 --- a/src/burn/engine/approvedexe.cpp +++ b/src/burn/engine/approvedexe.cpp @@ -106,7 +106,6 @@ extern "C" void ApprovedExesUninitializeLaunch( ReleaseStr(pLaunchApprovedExe->sczArguments); ReleaseStr(pLaunchApprovedExe->sczExecutablePath); ReleaseStr(pLaunchApprovedExe->sczId); - MemFree(pLaunchApprovedExe); } } diff --git a/src/burn/engine/core.cpp b/src/burn/engine/core.cpp index c443e0c8..8903b5b2 100644 --- a/src/burn/engine/core.cpp +++ b/src/burn/engine/core.cpp @@ -884,17 +884,16 @@ LExit: LogId(REPORT_STANDARD, MSG_LAUNCH_APPROVED_EXE_COMPLETE, hr, dwProcessId); - ApprovedExesUninitializeLaunch(pLaunchApprovedExe); - return hr; } -extern "C" HRESULT CoreQuit( - __in BURN_ENGINE_STATE* pEngineState, - __in int nExitCode +extern "C" void CoreQuit( + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, + __in DWORD dwExitCode ) { HRESULT hr = S_OK; + BURN_ENGINE_STATE* pEngineState = pEngineContext->pEngineState; // Save engine state if resume mode is unequal to "none". if (BURN_RESUME_MODE_NONE != pEngineState->resumeMode) @@ -907,13 +906,15 @@ extern "C" HRESULT CoreQuit( } } - LogId(REPORT_STANDARD, MSG_QUIT, nExitCode); + LogId(REPORT_STANDARD, MSG_QUIT, dwExitCode); - pEngineState->fQuit = TRUE; + pEngineState->userExperience.dwExitCode = dwExitCode; - ::PostQuitMessage(nExitCode); // go bye-bye. + ::EnterCriticalSection(&pEngineContext->csQueue); - return hr; + pEngineState->fQuit = TRUE; + + ::LeaveCriticalSection(&pEngineContext->csQueue); } extern "C" HRESULT CoreSaveEngineState( @@ -2071,6 +2072,18 @@ LExit: return hr; } +extern "C" void DAPI CoreBootstrapperEngineActionUninitialize( + __in BOOTSTRAPPER_ENGINE_ACTION* pAction + ) +{ + switch (pAction->dwMessage) + { + case WM_BURN_LAUNCH_APPROVED_EXE: + ApprovedExesUninitializeLaunch(&pAction->launchApprovedExe); + break; + } +} + // internal helper functions static HRESULT AppendEscapedArgumentToCommandLine( diff --git a/src/burn/engine/core.h b/src/burn/engine/core.h index 3e636640..186b49ca 100644 --- a/src/burn/engine/core.h +++ b/src/burn/engine/core.h @@ -262,9 +262,9 @@ HRESULT CoreLaunchApprovedExe( __in BURN_ENGINE_STATE* pEngineState, __in BURN_LAUNCH_APPROVED_EXE* pLaunchApprovedExe ); -HRESULT CoreQuit( - __in BURN_ENGINE_STATE* pEngineState, - __in int nExitCode +void CoreQuit( + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, + __in DWORD dwExitCode ); HRESULT CoreSaveEngineState( __in BURN_ENGINE_STATE* pEngineState @@ -354,6 +354,9 @@ HRESULT DAPI CoreCloseElevatedLoggingThread( HRESULT DAPI CoreWaitForUnelevatedLoggingThread( __in HANDLE hUnelevatedLoggingThread ); +void DAPI CoreBootstrapperEngineActionUninitialize( + __in BOOTSTRAPPER_ENGINE_ACTION* pAction + ); #if defined(__cplusplus) } diff --git a/src/burn/engine/elevation.cpp b/src/burn/engine/elevation.cpp index 7fd372b0..154c407d 100644 --- a/src/burn/engine/elevation.cpp +++ b/src/burn/engine/elevation.cpp @@ -3824,7 +3824,8 @@ static HRESULT OnLaunchApprovedExe( { HRESULT hr = S_OK; SIZE_T iData = 0; - BURN_LAUNCH_APPROVED_EXE* pLaunchApprovedExe = NULL; + BURN_LAUNCH_APPROVED_EXE launchApprovedExe = { }; + BURN_LAUNCH_APPROVED_EXE* pLaunchApprovedExe = &launchApprovedExe; BURN_APPROVED_EXE* pApprovedExe = NULL; HKEY hKey = NULL; DWORD dwProcessId = 0; @@ -3832,8 +3833,6 @@ static HRESULT OnLaunchApprovedExe( SIZE_T cbSendData = 0; DWORD dwResult = 0; - pLaunchApprovedExe = (BURN_LAUNCH_APPROVED_EXE*)MemAlloc(sizeof(BURN_LAUNCH_APPROVED_EXE), TRUE); - // Deserialize message data. hr = BuffReadString(pbData, cbData, &iData, &pLaunchApprovedExe->sczId); ExitOnFailure(hr, "Failed to read approved exe id."); diff --git a/src/burn/engine/engine.cpp b/src/burn/engine/engine.cpp index 48196655..c9d2bdcd 100644 --- a/src/burn/engine/engine.cpp +++ b/src/burn/engine/engine.cpp @@ -42,8 +42,8 @@ static HRESULT RunApplication( __out BOOL* pfSkipCleanup ); static HRESULT ProcessMessage( - __in BURN_ENGINE_STATE* pEngineState, - __in const MSG* pmsg + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, + __in BOOTSTRAPPER_ENGINE_ACTION* pAction ); static HRESULT DAPI RedirectLoggingOverPipe( __in_z LPCSTR szString, @@ -790,6 +790,19 @@ LExit: return hr; } +static void CALLBACK FreeQueueItem( + __in void* pvValue, + __in void* /*pvContext*/ + ) +{ + BOOTSTRAPPER_ENGINE_ACTION* pAction = reinterpret_cast(pvValue); + + LogId(REPORT_WARNING, MSG_IGNORE_OPERATION_AFTER_QUIT, LoggingBurnMessageToString(pAction->dwMessage)); + + CoreBootstrapperEngineActionUninitialize(pAction); + MemFree(pAction); +} + static HRESULT RunApplication( __in BURN_ENGINE_STATE* pEngineState, __out BOOL* pfReloadApp, @@ -799,16 +812,20 @@ static HRESULT RunApplication( HRESULT hr = S_OK; BOOTSTRAPPER_ENGINE_CONTEXT engineContext = { }; BOOL fStartupCalled = FALSE; - BOOL fRet = FALSE; - MSG msg = { }; BOOTSTRAPPER_SHUTDOWN_ACTION shutdownAction = BOOTSTRAPPER_SHUTDOWN_ACTION_NONE; - - ::PeekMessageW(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE); + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; // Setup the bootstrapper engine. - engineContext.dwThreadId = ::GetCurrentThreadId(); engineContext.pEngineState = pEngineState; + ::InitializeCriticalSection(&engineContext.csQueue); + + engineContext.hQueueSemaphore = ::CreateSemaphoreW(NULL, 0, LONG_MAX, NULL); + ExitOnNullWithLastError(engineContext.hQueueSemaphore, hr, "Failed to create semaphore for queue."); + + hr = QueCreate(&engineContext.hQueue); + ExitOnFailure(hr, "Failed to create queue for bootstrapper engine."); + // Load the bootstrapper application. hr = UserExperienceLoad(&pEngineState->userExperience, &engineContext, &pEngineState->command); ExitOnFailure(hr, "Failed to load BA."); @@ -817,28 +834,24 @@ static HRESULT RunApplication( hr = UserExperienceOnStartup(&pEngineState->userExperience); ExitOnFailure(hr, "Failed to start bootstrapper application."); - // Enter the message pump. - while (0 != (fRet = ::GetMessageW(&msg, NULL, 0, 0))) + while (!pEngineState->fQuit) { - if (-1 == fRet) - { - hr = E_UNEXPECTED; - ExitOnRootFailure(hr, "Unexpected return value from message pump."); - } - else - { - // When the BA makes a request from its own thread, it's common for the PostThreadMessage in externalengine.cpp - // to block until this thread waits on something. It's also common for Detect and Plan to never wait on something. - // In the extreme case, the engine could be elevating in Apply before the Detect call returned to the BA. - // This helps to avoid that situation, which could be blocking a UI thread. - ::Sleep(0); + hr = AppWaitForSingleObject(engineContext.hQueueSemaphore, INFINITE); + ExitOnFailure(hr, "Failed to wait on queue event."); - ProcessMessage(pEngineState, &msg); - } - } + ::EnterCriticalSection(&engineContext.csQueue); + + hr = QueDequeue(engineContext.hQueue, reinterpret_cast(&pAction)); + + ::LeaveCriticalSection(&engineContext.csQueue); - // Get exit code. - pEngineState->userExperience.dwExitCode = (DWORD)msg.wParam; + ExitOnFailure(hr, "Failed to dequeue action."); + + ProcessMessage(&engineContext, pAction); + + CoreBootstrapperEngineActionUninitialize(pAction); + MemFree(pAction); + } LExit: if (fStartupCalled) @@ -864,52 +877,50 @@ LExit: // Unload BA. UserExperienceUnload(&pEngineState->userExperience, *pfReloadApp); + ::DeleteCriticalSection(&engineContext.csQueue); + ReleaseHandle(engineContext.hQueueSemaphore); + ReleaseQueue(engineContext.hQueue, FreeQueueItem, &engineContext); + return hr; } static HRESULT ProcessMessage( - __in BURN_ENGINE_STATE* pEngineState, - __in const MSG* pmsg + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, + __in BOOTSTRAPPER_ENGINE_ACTION* pAction ) { HRESULT hr = S_OK; + BURN_ENGINE_STATE* pEngineState = pEngineContext->pEngineState; UserExperienceActivateEngine(&pEngineState->userExperience); - if (pEngineState->fQuit) - { - LogId(REPORT_WARNING, MSG_IGNORE_OPERATION_AFTER_QUIT, LoggingBurnMessageToString(pmsg->message)); - ExitFunction1(hr = E_INVALIDSTATE); - } - - switch (pmsg->message) + switch (pAction->dwMessage) { case WM_BURN_DETECT: - hr = CoreDetect(pEngineState, reinterpret_cast(pmsg->lParam)); + hr = CoreDetect(pEngineState, pAction->detect.hwndParent); break; case WM_BURN_PLAN: - hr = CorePlan(pEngineState, static_cast(pmsg->lParam)); + hr = CorePlan(pEngineState, pAction->plan.action); break; case WM_BURN_ELEVATE: - hr = CoreElevate(pEngineState, WM_BURN_ELEVATE, reinterpret_cast(pmsg->lParam)); + hr = CoreElevate(pEngineState, WM_BURN_ELEVATE, pAction->elevate.hwndParent); break; case WM_BURN_APPLY: - hr = CoreApply(pEngineState, reinterpret_cast(pmsg->lParam)); + hr = CoreApply(pEngineState, pAction->apply.hwndParent); break; case WM_BURN_LAUNCH_APPROVED_EXE: - hr = CoreLaunchApprovedExe(pEngineState, reinterpret_cast(pmsg->lParam)); + hr = CoreLaunchApprovedExe(pEngineState, &pAction->launchApprovedExe); break; case WM_BURN_QUIT: - hr = CoreQuit(pEngineState, static_cast(pmsg->wParam)); + CoreQuit(pEngineContext, pAction->quit.dwExitCode); break; } -LExit: UserExperienceDeactivateEngine(&pEngineState->userExperience); return hr; diff --git a/src/burn/engine/externalengine.cpp b/src/burn/engine/externalengine.cpp index 5e540c2a..262bb1a9 100644 --- a/src/burn/engine/externalengine.cpp +++ b/src/burn/engine/externalengine.cpp @@ -13,6 +13,10 @@ static HRESULT ProcessUnknownEmbeddedMessages( __in_opt LPVOID /*pvContext*/, __out DWORD* pdwResult ); +static HRESULT EnqueueAction( + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, + __inout BOOTSTRAPPER_ENGINE_ACTION** ppAction + ); // function definitions @@ -582,69 +586,91 @@ HRESULT ExternalEngineCompareVersions( } HRESULT ExternalEngineDetect( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent ) { HRESULT hr = S_OK; + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; - if (!::PostThreadMessageW(dwThreadId, WM_BURN_DETECT, 0, reinterpret_cast(hwndParent))) - { - ExitWithLastError(hr, "Failed to post detect message."); - } + pAction = (BOOTSTRAPPER_ENGINE_ACTION*)MemAlloc(sizeof(BOOTSTRAPPER_ENGINE_ACTION), TRUE); + ExitOnNull(pAction, hr, E_OUTOFMEMORY, "Failed to alloc BOOTSTRAPPER_ENGINE_ACTION"); + + pAction->dwMessage = WM_BURN_DETECT; + pAction->detect.hwndParent = hwndParent; + + hr = EnqueueAction(pEngineContext, &pAction); + ExitOnFailure(hr, "Failed to enqueue detect action."); LExit: + ReleaseMem(pAction); + return hr; } HRESULT ExternalEnginePlan( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in const BOOTSTRAPPER_ACTION action ) { HRESULT hr = S_OK; + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; if (BOOTSTRAPPER_ACTION_LAYOUT > action || BOOTSTRAPPER_ACTION_UPDATE_REPLACE_EMBEDDED < action) { ExitOnRootFailure(hr = E_INVALIDARG, "BA passed invalid action to Plan: %u.", action); } - if (!::PostThreadMessageW(dwThreadId, WM_BURN_PLAN, 0, action)) - { - ExitWithLastError(hr, "Failed to post plan message."); - } + pAction = (BOOTSTRAPPER_ENGINE_ACTION*)MemAlloc(sizeof(BOOTSTRAPPER_ENGINE_ACTION), TRUE); + ExitOnNull(pAction, hr, E_OUTOFMEMORY, "Failed to alloc BOOTSTRAPPER_ENGINE_ACTION"); + + pAction->dwMessage = WM_BURN_PLAN; + pAction->plan.action = action; + + hr = EnqueueAction(pEngineContext, &pAction); + ExitOnFailure(hr, "Failed to enqueue plan action."); LExit: + ReleaseMem(pAction); + return hr; } HRESULT ExternalEngineElevate( - __in BURN_ENGINE_STATE* pEngineState, - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent ) { HRESULT hr = S_OK; + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; - if (INVALID_HANDLE_VALUE != pEngineState->companionConnection.hPipe) - { - hr = HRESULT_FROM_WIN32(ERROR_ALREADY_INITIALIZED); - } - else if (!::PostThreadMessageW(dwThreadId, WM_BURN_ELEVATE, 0, reinterpret_cast(hwndParent))) + if (INVALID_HANDLE_VALUE != pEngineContext->pEngineState->companionConnection.hPipe) { - ExitWithLastError(hr, "Failed to post elevate message."); + ExitFunction1(hr = HRESULT_FROM_WIN32(ERROR_ALREADY_INITIALIZED)); } + pAction = (BOOTSTRAPPER_ENGINE_ACTION*)MemAlloc(sizeof(BOOTSTRAPPER_ENGINE_ACTION), TRUE); + ExitOnNull(pAction, hr, E_OUTOFMEMORY, "Failed to alloc BOOTSTRAPPER_ENGINE_ACTION"); + + pAction->dwMessage = WM_BURN_ELEVATE; + pAction->elevate.hwndParent = hwndParent; + + hr = EnqueueAction(pEngineContext, &pAction); + ExitOnFailure(hr, "Failed to enqueue elevate action."); + LExit: + ReleaseMem(pAction); + return hr; } HRESULT ExternalEngineApply( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent ) { HRESULT hr = S_OK; + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; ExitOnNull(hwndParent, hr, E_INVALIDARG, "BA passed NULL hwndParent to Apply."); if (!::IsWindow(hwndParent)) @@ -652,34 +678,46 @@ HRESULT ExternalEngineApply( ExitOnRootFailure(hr = E_INVALIDARG, "BA passed invalid hwndParent to Apply."); } - if (!::PostThreadMessageW(dwThreadId, WM_BURN_APPLY, 0, reinterpret_cast(hwndParent))) - { - ExitWithLastError(hr, "Failed to post apply message."); - } + pAction = (BOOTSTRAPPER_ENGINE_ACTION*)MemAlloc(sizeof(BOOTSTRAPPER_ENGINE_ACTION), TRUE); + ExitOnNull(pAction, hr, E_OUTOFMEMORY, "Failed to alloc BOOTSTRAPPER_ENGINE_ACTION"); + + pAction->dwMessage = WM_BURN_APPLY; + pAction->apply.hwndParent = hwndParent; + + hr = EnqueueAction(pEngineContext, &pAction); + ExitOnFailure(hr, "Failed to enqueue apply action."); LExit: + ReleaseMem(pAction); + return hr; } HRESULT ExternalEngineQuit( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in const DWORD dwExitCode ) { HRESULT hr = S_OK; + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; - if (!::PostThreadMessageW(dwThreadId, WM_BURN_QUIT, static_cast(dwExitCode), 0)) - { - ExitWithLastError(hr, "Failed to post shutdown message."); - } + pAction = (BOOTSTRAPPER_ENGINE_ACTION*)MemAlloc(sizeof(BOOTSTRAPPER_ENGINE_ACTION), TRUE); + ExitOnNull(pAction, hr, E_OUTOFMEMORY, "Failed to alloc BOOTSTRAPPER_ENGINE_ACTION"); + + pAction->dwMessage = WM_BURN_QUIT; + pAction->quit.dwExitCode = dwExitCode; + + hr = EnqueueAction(pEngineContext, &pAction); + ExitOnFailure(hr, "Failed to enqueue shutdown action."); LExit: + ReleaseMem(pAction); + return hr; } HRESULT ExternalEngineLaunchApprovedExe( - __in BURN_ENGINE_STATE* pEngineState, - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent, __in_z LPCWSTR wzApprovedExeForElevationId, __in_z_opt LPCWSTR wzArguments, @@ -688,25 +726,23 @@ HRESULT ExternalEngineLaunchApprovedExe( { HRESULT hr = S_OK; BURN_APPROVED_EXE* pApprovedExe = NULL; - BOOL fLeaveCriticalSection = FALSE; BURN_LAUNCH_APPROVED_EXE* pLaunchApprovedExe = NULL; - - pLaunchApprovedExe = (BURN_LAUNCH_APPROVED_EXE*)MemAlloc(sizeof(BURN_LAUNCH_APPROVED_EXE), TRUE); - ExitOnNull(pLaunchApprovedExe, hr, E_OUTOFMEMORY, "Failed to alloc BURN_LAUNCH_APPROVED_EXE"); - - ::EnterCriticalSection(&pEngineState->userExperience.csEngineActive); - fLeaveCriticalSection = TRUE; - hr = UserExperienceEnsureEngineInactive(&pEngineState->userExperience); - ExitOnFailure(hr, "Engine is active, cannot change engine state."); + BOOTSTRAPPER_ENGINE_ACTION* pAction = NULL; if (!wzApprovedExeForElevationId || !*wzApprovedExeForElevationId) { ExitFunction1(hr = E_INVALIDARG); } - hr = ApprovedExesFindById(&pEngineState->approvedExes, wzApprovedExeForElevationId, &pApprovedExe); + hr = ApprovedExesFindById(&pEngineContext->pEngineState->approvedExes, wzApprovedExeForElevationId, &pApprovedExe); ExitOnFailure(hr, "BA requested unknown approved exe with id: %ls", wzApprovedExeForElevationId); + pAction = (BOOTSTRAPPER_ENGINE_ACTION*)MemAlloc(sizeof(BOOTSTRAPPER_ENGINE_ACTION), TRUE); + ExitOnNull(pAction, hr, E_OUTOFMEMORY, "Failed to alloc BOOTSTRAPPER_ENGINE_ACTION"); + + pAction->dwMessage = WM_BURN_LAUNCH_APPROVED_EXE; + pLaunchApprovedExe = &pAction->launchApprovedExe; + hr = StrAllocString(&pLaunchApprovedExe->sczId, wzApprovedExeForElevationId, NULL); ExitOnFailure(hr, "Failed to copy the id."); @@ -720,20 +756,14 @@ HRESULT ExternalEngineLaunchApprovedExe( pLaunchApprovedExe->hwndParent = hwndParent; - if (!::PostThreadMessageW(dwThreadId, WM_BURN_LAUNCH_APPROVED_EXE, 0, reinterpret_cast(pLaunchApprovedExe))) - { - ExitWithLastError(hr, "Failed to post launch approved exe message."); - } + hr = EnqueueAction(pEngineContext, &pAction); + ExitOnFailure(hr, "Failed to enqueue launch approved exe action."); LExit: - if (fLeaveCriticalSection) + if (pAction) { - ::LeaveCriticalSection(&pEngineState->userExperience.csEngineActive); - } - - if (FAILED(hr)) - { - ApprovedExesUninitializeLaunch(pLaunchApprovedExe); + CoreBootstrapperEngineActionUninitialize(pAction); + MemFree(pAction); } return hr; @@ -836,3 +866,37 @@ static HRESULT ProcessUnknownEmbeddedMessages( return S_OK; } + +static HRESULT EnqueueAction( + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, + __inout BOOTSTRAPPER_ENGINE_ACTION** ppAction + ) +{ + HRESULT hr = S_OK; + + ::EnterCriticalSection(&pEngineContext->csQueue); + + if (pEngineContext->pEngineState->fQuit) + { + LogId(REPORT_WARNING, MSG_IGNORE_OPERATION_AFTER_QUIT, LoggingBurnMessageToString((*ppAction)->dwMessage)); + hr = E_INVALIDSTATE; + } + else + { + hr = QueEnqueue(pEngineContext->hQueue, *ppAction); + } + + ::LeaveCriticalSection(&pEngineContext->csQueue); + + ExitOnFailure(hr, "Failed to enqueue action."); + + *ppAction = NULL; + + if (!::ReleaseSemaphore(pEngineContext->hQueueSemaphore, 1, NULL)) + { + ExitWithLastError(hr, "Failed to signal queue semaphore."); + } + +LExit: + return hr; +} diff --git a/src/burn/engine/externalengine.h b/src/burn/engine/externalengine.h index f28971cd..9322234a 100644 --- a/src/burn/engine/externalengine.h +++ b/src/burn/engine/externalengine.h @@ -130,34 +130,32 @@ HRESULT ExternalEngineCompareVersions( ); HRESULT ExternalEngineDetect( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent ); HRESULT ExternalEnginePlan( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in const BOOTSTRAPPER_ACTION action ); HRESULT ExternalEngineElevate( - __in BURN_ENGINE_STATE* pEngineState, - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent ); HRESULT ExternalEngineApply( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent ); HRESULT ExternalEngineQuit( - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in const DWORD dwExitCode ); HRESULT ExternalEngineLaunchApprovedExe( - __in BURN_ENGINE_STATE* pEngineState, - __in const DWORD dwThreadId, + __in BOOTSTRAPPER_ENGINE_CONTEXT* pEngineContext, __in_opt const HWND hwndParent, __in_z LPCWSTR wzApprovedExeForElevationId, __in_z_opt LPCWSTR wzArguments, diff --git a/src/burn/engine/platform.h b/src/burn/engine/platform.h index 60184c23..5896a5ca 100644 --- a/src/burn/engine/platform.h +++ b/src/burn/engine/platform.h @@ -28,6 +28,7 @@ enum WM_BURN enum BURN_MODE; typedef struct _BOOTSTRAPPER_ENGINE_CONTEXT BOOTSTRAPPER_ENGINE_CONTEXT; +typedef struct _BOOTSTRAPPER_ENGINE_ACTION BOOTSTRAPPER_ENGINE_ACTION; typedef struct _BURN_CACHE BURN_CACHE; typedef struct _BURN_DEPENDENCIES BURN_DEPENDENCIES; typedef struct _BURN_ENGINE_COMMAND BURN_ENGINE_COMMAND; diff --git a/src/burn/engine/precomp.h b/src/burn/engine/precomp.h index e2d1b5cd..a64ce474 100644 --- a/src/burn/engine/precomp.h +++ b/src/burn/engine/precomp.h @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include diff --git a/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj b/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj index c84b98fe..dc6b913f 100644 --- a/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj +++ b/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj @@ -91,6 +91,7 @@ + @@ -153,6 +154,7 @@ + diff --git a/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj.filters b/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj.filters index efadef6f..c1095651 100644 --- a/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj.filters +++ b/src/libs/dutil/WixToolset.DUtil/dutil.vcxproj.filters @@ -135,6 +135,9 @@ Source Files + + Source Files + Source Files @@ -314,6 +317,9 @@ Header Files + + Header Files + Header Files diff --git a/src/libs/dutil/WixToolset.DUtil/inc/dutilsources.h b/src/libs/dutil/WixToolset.DUtil/inc/dutilsources.h index 664c21e5..cf10f910 100644 --- a/src/libs/dutil/WixToolset.DUtil/inc/dutilsources.h +++ b/src/libs/dutil/WixToolset.DUtil/inc/dutilsources.h @@ -64,6 +64,7 @@ typedef enum DUTIL_SOURCE DUTIL_SOURCE_WNDUTIL, DUTIL_SOURCE_ENVUTIL, DUTIL_SOURCE_THRDUTIL, + DUTIL_SOURCE_QUEUTIL, DUTIL_SOURCE_EXTERNAL = 256, } DUTIL_SOURCE; diff --git a/src/libs/dutil/WixToolset.DUtil/inc/queutil.h b/src/libs/dutil/WixToolset.DUtil/inc/queutil.h new file mode 100644 index 00000000..3b88825e --- /dev/null +++ b/src/libs/dutil/WixToolset.DUtil/inc/queutil.h @@ -0,0 +1,52 @@ +#pragma once +// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. + + +#ifdef __cplusplus +extern "C" { +#endif + +#define ReleaseQueue(qh, pfn, pv) if (qh) { QueDestroy(qh, pfn, pv); } +#define ReleaseNullQue(qh, pfv, pv) if (qh) { QueDestroy(qh, pfn, pv); qh = NULL; } + +typedef void* QUEUTIL_QUEUE_HANDLE; + +typedef void(CALLBACK* PFNQUEUTIL_QUEUE_RELEASE_VALUE)( + __in void* pvValue, + __in void* pvContext + ); + +extern const int QUEUTIL_QUEUE_HANDLE_BYTES; + +/******************************************************************** +QueCreate - Creates a simple queue. It is not thread safe. + +********************************************************************/ +HRESULT DAPI QueCreate( + __out_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE* phQueue + ); + +HRESULT DAPI QueEnqueue( + __in_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE hQueue, + __in void* pvValue + ); + +/******************************************************************** +QueDequeue - Returns the value from the beginning of the queue, + or E_NOMOREITEMS if the queue is empty. + +********************************************************************/ +HRESULT DAPI QueDequeue( + __in_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE hQueue, + __out void** ppvValue + ); + +void DAPI QueDestroy( + __in_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE hQueue, + __in_opt PFNQUEUTIL_QUEUE_RELEASE_VALUE pfnReleaseValue, + __in_opt void* pvContext + ); + +#ifdef __cplusplus +} +#endif diff --git a/src/libs/dutil/WixToolset.DUtil/precomp.h b/src/libs/dutil/WixToolset.DUtil/precomp.h index b628d271..607f7ce6 100644 --- a/src/libs/dutil/WixToolset.DUtil/precomp.h +++ b/src/libs/dutil/WixToolset.DUtil/precomp.h @@ -75,6 +75,7 @@ #include "perfutil.h" #include "polcutil.h" #include "procutil.h" +#include "queutil.h" #include "regutil.h" #include "butil.h" // NOTE: Butil must come after Regutil. #include "resrutil.h" diff --git a/src/libs/dutil/WixToolset.DUtil/queutil.cpp b/src/libs/dutil/WixToolset.DUtil/queutil.cpp new file mode 100644 index 00000000..c5d2d736 --- /dev/null +++ b/src/libs/dutil/WixToolset.DUtil/queutil.cpp @@ -0,0 +1,144 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. + +#include "precomp.h" + + +// Exit macros +#define QueExitOnLastError(x, s, ...) ExitOnLastErrorSource(DUTIL_SOURCE_QUEUTIL, x, s, __VA_ARGS__) +#define QueExitOnLastErrorDebugTrace(x, s, ...) ExitOnLastErrorDebugTraceSource(DUTIL_SOURCE_QUEUTIL, x, s, __VA_ARGS__) +#define QueExitWithLastError(x, s, ...) ExitWithLastErrorSource(DUTIL_SOURCE_QUEUTIL, x, s, __VA_ARGS__) +#define QueExitOnFailure(x, s, ...) ExitOnFailureSource(DUTIL_SOURCE_QUEUTIL, x, s, __VA_ARGS__) +#define QueExitOnRootFailure(x, s, ...) ExitOnRootFailureSource(DUTIL_SOURCE_QUEUTIL, x, s, __VA_ARGS__) +#define QueExitOnFailureDebugTrace(x, s, ...) ExitOnFailureDebugTraceSource(DUTIL_SOURCE_QUEUTIL, x, s, __VA_ARGS__) +#define QueExitOnNull(p, x, e, s, ...) ExitOnNullSource(DUTIL_SOURCE_QUEUTIL, p, x, e, s, __VA_ARGS__) +#define QueExitOnNullWithLastError(p, x, s, ...) ExitOnNullWithLastErrorSource(DUTIL_SOURCE_QUEUTIL, p, x, s, __VA_ARGS__) +#define QueExitOnNullDebugTrace(p, x, e, s, ...) ExitOnNullDebugTraceSource(DUTIL_SOURCE_QUEUTIL, p, x, e, s, __VA_ARGS__) +#define QueExitOnInvalidHandleWithLastError(p, x, s, ...) ExitOnInvalidHandleWithLastErrorSource(DUTIL_SOURCE_QUEUTIL, p, x, s, __VA_ARGS__) +#define QueExitOnWin32Error(e, x, s, ...) ExitOnWin32ErrorSource(DUTIL_SOURCE_QUEUTIL, e, x, s, __VA_ARGS__) +#define QueExitOnGdipFailure(g, x, s, ...) ExitOnGdipFailureSource(DUTIL_SOURCE_QUEUTIL, g, x, s, __VA_ARGS__) + + +struct QUEUTIL_QUEUE_ITEM +{ + QUEUTIL_QUEUE_ITEM* pNext; + void* pvValue; +}; + +struct QUEUTIL_QUEUE_STRUCT +{ + QUEUTIL_QUEUE_ITEM* pFirst; + QUEUTIL_QUEUE_ITEM* pLast; +}; + +const int QUEUTIL_QUEUE_HANDLE_BYTES = sizeof(QUEUTIL_QUEUE_STRUCT); + +extern "C" HRESULT DAPI QueCreate( + __out_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE* phQueue + ) +{ + HRESULT hr = S_OK; + + QueExitOnNull(phQueue, hr, E_INVALIDARG, "Handle not specified while creating queue."); + + *phQueue = reinterpret_cast(MemAlloc(QUEUTIL_QUEUE_HANDLE_BYTES, TRUE)); + QueExitOnNull(*phQueue, hr, E_OUTOFMEMORY, "Failed to allocate queue object."); + +LExit: + return hr; +} + +extern "C" HRESULT DAPI QueEnqueue( + __in_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE hQueue, + __in void* pvValue + ) +{ + HRESULT hr = S_OK; + QUEUTIL_QUEUE_ITEM* pItem = NULL; + QUEUTIL_QUEUE_STRUCT* pQueue = reinterpret_cast(hQueue); + + QueExitOnNull(pQueue, hr, E_INVALIDARG, "Handle not specified while enqueing value."); + + pItem = reinterpret_cast(MemAlloc(sizeof(QUEUTIL_QUEUE_ITEM), TRUE)); + QueExitOnNull(pItem, hr, E_OUTOFMEMORY, "Failed to allocate queue item."); + + pItem->pvValue = pvValue; + + if (!pQueue->pLast) + { + pQueue->pFirst = pItem; + } + else + { + pQueue->pLast->pNext = pItem; + } + + pQueue->pLast = pItem; + + pItem = NULL; + +LExit: + ReleaseMem(pItem); + + return hr; +} + +extern "C" HRESULT DAPI QueDequeue( + __in_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE hQueue, + __out void** ppvValue + ) +{ + HRESULT hr = S_OK; + QUEUTIL_QUEUE_ITEM* pItem = NULL; + QUEUTIL_QUEUE_STRUCT* pQueue = reinterpret_cast(hQueue); + + QueExitOnNull(pQueue, hr, E_INVALIDARG, "Handle not specified while dequeing value."); + + if (!pQueue->pFirst) + { + *ppvValue = NULL; + ExitFunction1(hr = E_NOMOREITEMS); + } + + pItem = pQueue->pFirst; + + if (!pItem->pNext) + { + pQueue->pFirst = NULL; + pQueue->pLast = NULL; + } + else + { + pQueue->pFirst = pItem->pNext; + } + + *ppvValue = pItem->pvValue; + +LExit: + ReleaseMem(pItem); + + return hr; +} + +extern "C" void DAPI QueDestroy( + __in_bcount(QUEUTIL_QUEUE_HANDLE_BYTES) QUEUTIL_QUEUE_HANDLE hQueue, + __in_opt PFNQUEUTIL_QUEUE_RELEASE_VALUE pfnReleaseValue, + __in_opt void* pvContext + ) +{ + HRESULT hr = S_OK; + void* pvValue = NULL; + + hr = hQueue ? QueDequeue(hQueue, &pvValue) : E_NOMOREITEMS; + + while (SUCCEEDED(hr)) + { + if (pfnReleaseValue) + { + pfnReleaseValue(pvValue, pvContext); + } + + hr = QueDequeue(hQueue, &pvValue); + } + + ReleaseMem(hQueue); +} diff --git a/src/test/burn/WixToolsetTest.BurnE2E/ElevationTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/ElevationTests.cs index 8f141e5d..f040905d 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/ElevationTests.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/ElevationTests.cs @@ -13,7 +13,7 @@ namespace WixToolsetTest.BurnE2E /// This test calls Elevate after Detect, and then calls Plan in OnElevateBegin. /// After calling Plan, it pumps some messages to simulate UI like the UAC callback. /// - [RuntimeFact(Skip = "https://github.com/wixtoolset/issues/issues/6349")] // CAUTION: this test currently hangs because the Plan request gets dropped. + [RuntimeFact] public void CanExplicitlyElevateAndPlanFromOnElevateBegin() { var packageA = this.CreatePackageInstaller("PackageA"); -- cgit v1.2.3-55-g6feb