[vcpkg] Wrap all external process spawning in a Ctrl-C catcher to avoid corrupted consoles

6 files changed, 94 insertions, 18 deletions

diff --git a/toolsrc/include/vcpkg/base/system.h b/toolsrc/include/vcpkg/base/system.h
index 3c4326ade..005e09a3f 100644
--- a/toolsrc/include/vcpkg/base/system.h
+++ b/toolsrc/include/vcpkg/base/system.h
@@ -32,15 +32,15 @@ namespace vcpkg::System
     };
 
     int cmd_execute_clean(const CStringView cmd_line,
-                          const std::unordered_map<std::string, std::string>& extra_env = {});
+                          const std::unordered_map<std::string, std::string>& extra_env = {}) noexcept;
 
-    int cmd_execute(const CStringView cmd_line);
+    int cmd_execute(const CStringView cmd_line) noexcept;
 
 #if defined(_WIN32)
-    void cmd_execute_no_wait(const CStringView cmd_line);
+    void cmd_execute_no_wait(const CStringView cmd_line) noexcept;
 #endif
 
-    ExitCodeAndOutput cmd_execute_and_capture_output(const CStringView cmd_line);
+    ExitCodeAndOutput cmd_execute_and_capture_output(const CStringView cmd_line) noexcept;
 
     enum class Color
     {
diff --git a/toolsrc/include/vcpkg/globalstate.h b/toolsrc/include/vcpkg/globalstate.h
index bc28e3ff8..2a76c8741 100644
--- a/toolsrc/include/vcpkg/globalstate.h
+++ b/toolsrc/include/vcpkg/globalstate.h
@@ -18,5 +18,25 @@ namespace vcpkg
 
         static std::atomic<int> g_init_console_cp;
         static std::atomic<int> g_init_console_output_cp;
+        static std::atomic<bool> g_init_console_initialized;
+
+        struct CtrlCStateMachine
+        {
+            void transition_to_spawn_process() noexcept;
+            void transition_from_spawn_process() noexcept;
+            void transition_handle_ctrl_c() noexcept;
+
+        private:
+            enum class CtrlCState
+            {
+                normal,
+                blocked_on_child,
+                exit_requested,
+            };
+
+            std::atomic<CtrlCState> m_state = CtrlCState::normal;
+        };
+
+        static CtrlCStateMachine g_ctrl_c_state;
     };
 }
diff --git a/toolsrc/src/vcpkg.cpp b/toolsrc/src/vcpkg.cpp
index 6f10c503d..ba9710562 100644
--- a/toolsrc/src/vcpkg.cpp
+++ b/toolsrc/src/vcpkg.cpp
@@ -261,6 +261,7 @@ int main(const int argc, const char* const* const argv)
 #if defined(_WIN32)
     GlobalState::g_init_console_cp = GetConsoleCP();
     GlobalState::g_init_console_output_cp = GetConsoleOutputCP();
+    GlobalState::g_init_console_initialized = true;
 
     SetConsoleCP(CP_UTF8);
     SetConsoleOutputCP(CP_UTF8);
@@ -275,6 +276,9 @@ int main(const int argc, const char* const* const argv)
         locked_metrics->track_property("cmdline", trimmed_command_line);
 #endif
     }
+
+    Checks::register_console_ctrl_handler();
+
     load_config();
 
     const auto vcpkg_feature_flags_env = System::get_environment_variable("VCPKG_FEATURE_FLAGS");
@@ -293,8 +297,6 @@ int main(const int argc, const char* const* const argv)
     if (const auto p = args.sendmetrics.get()) Metrics::g_metrics.lock()->set_send_metrics(*p);
     if (const auto p = args.debug.get()) GlobalState::debugging = *p;
 
-    Checks::register_console_ctrl_handler();
-
     if (GlobalState::debugging)
     {
         inner(args);
diff --git a/toolsrc/src/vcpkg/base/checks.cpp b/toolsrc/src/vcpkg/base/checks.cpp
index 72176e58c..cc439adfe 100644
--- a/toolsrc/src/vcpkg/base/checks.cpp
+++ b/toolsrc/src/vcpkg/base/checks.cpp
@@ -24,8 +24,11 @@ namespace vcpkg::Checks
         metrics->flush();
 
 #if defined(_WIN32)
-        SetConsoleCP(GlobalState::g_init_console_cp);
-        SetConsoleOutputCP(GlobalState::g_init_console_output_cp);
+        if (GlobalState::g_init_console_initialized)
+        {
+            SetConsoleCP(GlobalState::g_init_console_cp);
+            SetConsoleOutputCP(GlobalState::g_init_console_output_cp);
+        }
 #endif
 
         auto elapsed_us = GlobalState::timer.lock()->microseconds();
@@ -46,12 +49,11 @@ namespace vcpkg::Checks
 #if defined(_WIN32)
     static BOOL ctrl_handler(DWORD fdw_ctrl_type)
     {
+        switch (fdw_ctrl_type)
         {
-            auto locked_metrics = Metrics::g_metrics.lock();
-            locked_metrics->track_property("CtrlHandler", std::to_string(fdw_ctrl_type));
-            locked_metrics->track_property("error", "CtrlHandler was fired.");
+            case CTRL_C_EVENT: GlobalState::g_ctrl_c_state.transition_handle_ctrl_c(); return TRUE;
+            default: return FALSE;
         }
-        cleanup_and_exit(EXIT_FAILURE);
     }
 
     void register_console_ctrl_handler()
diff --git a/toolsrc/src/vcpkg/base/system.cpp b/toolsrc/src/vcpkg/base/system.cpp
index d968c6dac..085471b19 100644
--- a/toolsrc/src/vcpkg/base/system.cpp
+++ b/toolsrc/src/vcpkg/base/system.cpp
@@ -133,7 +133,7 @@ namespace vcpkg::System
     static void windows_create_clean_process(const CStringView cmd_line,
                                              const std::unordered_map<std::string, std::string>& extra_env,
                                              PROCESS_INFORMATION& process_info,
-                                             DWORD dwCreationFlags)
+                                             DWORD dwCreationFlags) noexcept
     {
         static const std::string SYSTEM_ROOT = get_environment_variable("SystemRoot").value_or_exit(VCPKG_LINE_INFO);
         static const std::string SYSTEM_32 = SYSTEM_ROOT + R"(\system32)";
@@ -242,7 +242,7 @@ namespace vcpkg::System
 #endif
 
 #if defined(_WIN32)
-    void cmd_execute_no_wait(const CStringView cmd_line)
+    void cmd_execute_no_wait(const CStringView cmd_line) noexcept
     {
         auto timer = Chrono::ElapsedTimer::create_started();
 
@@ -258,7 +258,8 @@ namespace vcpkg::System
     }
 #endif
 
-    int cmd_execute_clean(const CStringView cmd_line, const std::unordered_map<std::string, std::string>& extra_env)
+    int cmd_execute_clean(const CStringView cmd_line,
+                          const std::unordered_map<std::string, std::string>& extra_env) noexcept
     {
         auto timer = Chrono::ElapsedTimer::create_started();
 #if defined(_WIN32)
@@ -266,11 +267,13 @@ namespace vcpkg::System
         PROCESS_INFORMATION process_info;
         memset(&process_info, 0, sizeof(PROCESS_INFORMATION));
 
+        GlobalState::g_ctrl_c_state.transition_to_spawn_process();
         windows_create_clean_process(cmd_line, extra_env, process_info, NULL);
 
         CloseHandle(process_info.hThread);
 
         const DWORD result = WaitForSingleObject(process_info.hProcess, INFINITE);
+        GlobalState::g_ctrl_c_state.transition_from_spawn_process();
         Checks::check_exit(VCPKG_LINE_INFO, result != WAIT_FAILED, "WaitForSingleObject failed");
 
         DWORD exit_code = 0;
@@ -279,6 +282,7 @@ namespace vcpkg::System
         CloseHandle(process_info.hProcess);
 
         Debug::println("CreateProcessW() returned %lu after %d us", exit_code, static_cast<int>(timer.microseconds()));
+
         return static_cast<int>(exit_code);
 #else
         Debug::println("system(%s)", cmd_line.c_str());
@@ -289,16 +293,18 @@ namespace vcpkg::System
 #endif
     }
 
-    int cmd_execute(const CStringView cmd_line)
+    int cmd_execute(const CStringView cmd_line) noexcept
     {
         // Flush stdout before launching external process
         fflush(nullptr);
 
-        // Basically we are wrapping it in quotes
 #if defined(_WIN32)
+        // We are wrap the command line in quotes to cause cmd.exe to correctly process it
         const std::string& actual_cmd_line = Strings::format(R"###("%s")###", cmd_line);
         Debug::println("_wsystem(%s)", actual_cmd_line);
+        GlobalState::g_ctrl_c_state.transition_to_spawn_process();
         const int exit_code = _wsystem(Strings::to_utf16(actual_cmd_line).c_str());
+        GlobalState::g_ctrl_c_state.transition_from_spawn_process();
         Debug::println("_wsystem() returned %d", exit_code);
 #else
         Debug::println("_system(%s)", cmd_line);
@@ -308,7 +314,7 @@ namespace vcpkg::System
         return exit_code;
     }
 
-    ExitCodeAndOutput cmd_execute_and_capture_output(const CStringView cmd_line)
+    ExitCodeAndOutput cmd_execute_and_capture_output(const CStringView cmd_line) noexcept
     {
         // Flush stdout before launching external process
         fflush(stdout);
@@ -321,9 +327,11 @@ namespace vcpkg::System
         Debug::println("_wpopen(%s)", actual_cmd_line);
         std::wstring output;
         wchar_t buf[1024];
+        GlobalState::g_ctrl_c_state.transition_to_spawn_process();
         const auto pipe = _wpopen(Strings::to_utf16(actual_cmd_line).c_str(), L"r");
         if (pipe == nullptr)
         {
+            GlobalState::g_ctrl_c_state.transition_from_spawn_process();
             return {1, Strings::to_utf8(output.c_str())};
         }
         while (fgetws(buf, 1024, pipe))
@@ -332,10 +340,12 @@ namespace vcpkg::System
         }
         if (!feof(pipe))
         {
+            GlobalState::g_ctrl_c_state.transition_from_spawn_process();
             return {1, Strings::to_utf8(output.c_str())};
         }
 
         const auto ec = _pclose(pipe);
+        GlobalState::g_ctrl_c_state.transition_from_spawn_process();
 
         // On Win7, output from powershell calls contain a utf-8 byte order mark in the utf-16 stream, so we strip it
         // out if it is present. 0xEF,0xBB,0xBF is the UTF-8 byte-order mark
diff --git a/toolsrc/src/vcpkg/globalstate.cpp b/toolsrc/src/vcpkg/globalstate.cpp
index a4100acf7..ee576f9ce 100644
--- a/toolsrc/src/vcpkg/globalstate.cpp
+++ b/toolsrc/src/vcpkg/globalstate.cpp
@@ -13,4 +13,46 @@ namespace vcpkg
 
     std::atomic<int> GlobalState::g_init_console_cp(0);
     std::atomic<int> GlobalState::g_init_console_output_cp(0);
+    std::atomic<bool> GlobalState::g_init_console_initialized(false);
+
+    GlobalState::CtrlCStateMachine GlobalState::g_ctrl_c_state;
+
+    void GlobalState::CtrlCStateMachine::transition_to_spawn_process() noexcept
+    {
+        auto expected = CtrlCState::normal;
+        auto transitioned = m_state.compare_exchange_strong(expected, CtrlCState::blocked_on_child);
+        if (!transitioned)
+        {
+            // Ctrl-C was hit and is asynchronously executing on another thread
+            Checks::exit_fail(VCPKG_LINE_INFO);
+        }
+    }
+    void GlobalState::CtrlCStateMachine::transition_from_spawn_process() noexcept
+    {
+        auto expected = CtrlCState::blocked_on_child;
+        auto transitioned = m_state.compare_exchange_strong(expected, CtrlCState::normal);
+        if (!transitioned)
+        {
+            // Ctrl-C was hit while blocked on the child process
+            Checks::exit_fail(VCPKG_LINE_INFO);
+        }
+    }
+    void GlobalState::CtrlCStateMachine::transition_handle_ctrl_c() noexcept
+    {
+        auto prev_state = m_state.exchange(CtrlCState::exit_requested);
+
+        if (prev_state == CtrlCState::normal)
+        {
+            // Not currently blocked on a child process and Ctrl-C has not been hit.
+            Checks::exit_fail(VCPKG_LINE_INFO);
+        }
+        else if (prev_state == CtrlCState::exit_requested)
+        {
+            // Ctrl-C was hit previously
+        }
+        else
+        {
+            // This is the case where we are currently blocked on a child process
+        }
+    }
 }