commit 224fcad5e36788619c987e274ea29d90e6fa94c4
parent 0653f2c506b47d61930b781c550d6ae6adb4237b
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Fri, 8 May 2020 10:06:56 -0700
Fix CTRL-C causing process exit due to code also using E_ABORT (#108)
Due to the way that CTRL signals were being handled, the code that would exit the process in response to multiple CTRL signals was being invoked if the cooperative cancellation was resulting in another E_ABORT context termination. This change uses a unique error code for CTRL signals, thus removing the possibility of accidental early termination.
Diffstat:
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp
@@ -12,7 +12,7 @@ namespace AppInstaller::CLI::Execution
Context* s_contextForCtrlHandler = nullptr;
BOOL WINAPI CtrlHandlerForContext(DWORD ctrlType)
- {
+ {
AICLI_LOG(CLI, Info, << "Got CTRL type: " << ctrlType);
// Won't save us from every crash, but a few more than direct access.
@@ -26,7 +26,7 @@ namespace AppInstaller::CLI::Execution
{
case CTRL_C_EVENT:
case CTRL_BREAK_EVENT:
- context->Terminate(E_ABORT);
+ context->Terminate(APPINSTALLER_CLI_ERROR_CTRL_SIGNAL_RECEIVED);
context->Reporter.CancelInProgressTask(false);
return TRUE;
// According to MSDN, we should never receive these due to having gdi32/user32 loaded in our process.
@@ -34,10 +34,10 @@ namespace AppInstaller::CLI::Execution
case CTRL_CLOSE_EVENT:
case CTRL_LOGOFF_EVENT:
case CTRL_SHUTDOWN_EVENT:
- context->Terminate(E_ABORT);
+ context->Terminate(APPINSTALLER_CLI_ERROR_CTRL_SIGNAL_RECEIVED);
context->Reporter.CancelInProgressTask(true);
return TRUE;
- default:
+ default:
return FALSE;
}
}
@@ -92,12 +92,19 @@ namespace AppInstaller::CLI::Execution
void Context::Terminate(HRESULT hr)
{
- if (m_isTerminated && m_terminationHR == hr && hr == E_ABORT)
+ if (hr == APPINSTALLER_CLI_ERROR_CTRL_SIGNAL_RECEIVED)
{
+ ++m_CtrlSignalCount;
+ // Use a more recognizable error
+ hr = E_ABORT;
+
// If things aren't terminating fast enough for the user, they will probably press CTRL+C again.
// In that case, we should forcibly terminate.
// Unless we want to spin a separate thread for all work, we have to just exit here.
- std::exit(hr);
+ if (m_CtrlSignalCount >= 2)
+ {
+ std::exit(hr);
+ }
}
m_isTerminated = true;
diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h
@@ -188,5 +188,6 @@ namespace AppInstaller::CLI::Execution
bool m_isTerminated = false;
HRESULT m_terminationHR = S_OK;
std::map<Data, details::DataVariant> m_data;
+ size_t m_CtrlSignalCount = 0;
};
}
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h
@@ -14,7 +14,7 @@
#define APPINSTALLER_CLI_ERROR_INVALID_CL_ARGUMENTS ((HRESULT)0x8A150002)
#define APPINSTALLER_CLI_ERROR_COMMAND_FAILED ((HRESULT)0x8A150003)
#define APPINSTALLER_CLI_ERROR_MANIFEST_FAILED ((HRESULT)0x8A150004)
-//#define APPINSTALLER_CLI_ERROR_WORKFLOW_FAILED ((HRESULT)0x8A150005) // Unused, can be repurposed
+#define APPINSTALLER_CLI_ERROR_CTRL_SIGNAL_RECEIVED ((HRESULT)0x8A150005)
#define APPINSTALLER_CLI_ERROR_SHELLEXEC_INSTALL_FAILED ((HRESULT)0x8A150006)
//#define APPINSTALLER_CLI_ERROR_RUNTIME_ERROR ((HRESULT)0x8A150007) // Unused, can be repurposed
#define APPINSTALLER_CLI_ERROR_DOWNLOAD_FAILED ((HRESULT)0x8A150008)
