From 50313c2e6a8448660049ed21c478e16294e3d6ce Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 1 Apr 2024 17:06:59 +0000 Subject: [PATCH 1/8] Add the ability to send a signal to a subprocess Signed-off-by: Michael Carroll --- include/gz/utils/Subprocess.hh | 34 ++++++++++++++++++++++++++++ include/gz/utils/detail/subprocess.h | 11 +++++++++ test/integration/subprocess_TEST.cc | 34 ++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/include/gz/utils/Subprocess.hh b/include/gz/utils/Subprocess.hh index f7f9fab..c434d93 100644 --- a/include/gz/utils/Subprocess.hh +++ b/include/gz/utils/Subprocess.hh @@ -36,6 +36,20 @@ namespace gz namespace utils { +enum class Signals +{ + #ifdef WIN32 + CTRL_C_EVENT = 0, + CTRL_BREAK_EVENT = 1, + CTRL_CLOSE_EVENT = 2, + CTRL_LOGOFF_EVENT = 5, + CTRL_SHUTDOWN_EVENT = 6, + #else + SIGNAL_SIGINT = 2, + SIGNAL_SIGQUIT = 3, + #endif +}; + /// \brief Create a RAII-type object that encapsulates a subprocess. class Subprocess { @@ -183,6 +197,26 @@ class Subprocess return false; } + public: bool Signal(int signum) + { + if (this->process != nullptr) + return subprocess_signal(this->process, signum) != 0; + else + return false; + + } + + public: bool SendExitSignal() + { + int signal = 0; + #ifdef WIN32 + signal = static_cast(Signals::CTRL_C_EVENT); + #else + signal = static_cast(Signals::SIGNAL_SIGINT); + #endif + return this->Signal(signal); + } + public: bool Terminate() { if (this->process != nullptr) diff --git a/include/gz/utils/detail/subprocess.h b/include/gz/utils/detail/subprocess.h index 137aa14..08e64ec 100644 --- a/include/gz/utils/detail/subprocess.h +++ b/include/gz/utils/detail/subprocess.h @@ -923,6 +923,17 @@ FILE *subprocess_stderr(const struct subprocess_s *const process) { } } +int subprocess_signal(const subprocess_s *const process, + int signum) +{ +#if defined(_WIN32) + return GenerateConsoleCtrlEvent(signum, process->hProcess); +#else + return kill(process->child, signum); +#endif + +} + int subprocess_join(struct subprocess_s *const process, int *const out_return_code) { #if defined(_WIN32) diff --git a/test/integration/subprocess_TEST.cc b/test/integration/subprocess_TEST.cc index 47c8976..8d61266 100644 --- a/test/integration/subprocess_TEST.cc +++ b/test/integration/subprocess_TEST.cc @@ -200,5 +200,39 @@ TEST(Subprocess, Environment) EXPECT_EQ(std::string::npos, cout.find("GZ_BAZ_KEY=GZ_BAZ_VAL")); EXPECT_NE(std::string::npos, cout.find("QUX_KEY=QUX_VAL")); } +} + +///////////////////////////////////////////////// +TEST(Subprocess, Signal) +{ + auto start = std::chrono::steady_clock::now(); + auto proc = Subprocess({kExecutablePath, + "--output=cerr", + "--iterations=100", + "--iteration-ms=10"}); + EXPECT_TRUE(proc.Alive()); + + // Sleep for >1 iteration before sending signal + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + + // Signal + proc.SendExitSignal(); + // Block until the executable is done + auto ret = proc.Join(); + EXPECT_EQ(1u, ret); + + auto end = std::chrono::steady_clock::now(); + auto elapsed = + std::chrono::duration_cast(end - start); + + // Check that process finished in less than half a second + // If the signal failed to send, then the process would run for over 1 second + EXPECT_LT(elapsed.count(), 500); + + EXPECT_FALSE(proc.Alive()); + auto cout = proc.Stdout(); + auto cerr = proc.Stderr(); + EXPECT_TRUE(cout.empty()); + EXPECT_FALSE(cerr.empty()); } From 9c28ca4b2041a5538eb4d2e0bac247e94aaf4e96 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 1 Apr 2024 20:18:14 +0000 Subject: [PATCH 2/8] Fix Windows Signed-off-by: Michael Carroll --- include/gz/utils/Subprocess.hh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/include/gz/utils/Subprocess.hh b/include/gz/utils/Subprocess.hh index c434d93..9128b24 100644 --- a/include/gz/utils/Subprocess.hh +++ b/include/gz/utils/Subprocess.hh @@ -18,6 +18,10 @@ #ifndef GZ_UTILS__SUBPROCESS_HH_ #define GZ_UTILS__SUBPROCESS_HH_ +#if defined(_WIN32) +#include +#endif // defined(_WIN32) + #include "detail/subprocess.h" #include "gz/utils/Environment.hh" @@ -38,16 +42,16 @@ namespace utils enum class Signals { - #ifdef WIN32 +#if defined(_WIN32) CTRL_C_EVENT = 0, CTRL_BREAK_EVENT = 1, CTRL_CLOSE_EVENT = 2, CTRL_LOGOFF_EVENT = 5, CTRL_SHUTDOWN_EVENT = 6, - #else +#else SIGNAL_SIGINT = 2, SIGNAL_SIGQUIT = 3, - #endif +#endif }; /// \brief Create a RAII-type object that encapsulates a subprocess. @@ -209,11 +213,11 @@ class Subprocess public: bool SendExitSignal() { int signal = 0; - #ifdef WIN32 +#if defined(_WIN32) signal = static_cast(Signals::CTRL_C_EVENT); - #else +#else signal = static_cast(Signals::SIGNAL_SIGINT); - #endif +#endif return this->Signal(signal); } From e817d54cdd1853f55820d7e291e68b1070a94e0d Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 2 Apr 2024 16:24:51 +0000 Subject: [PATCH 3/8] More Windows fixes --- include/gz/utils/Subprocess.hh | 18 ++------------ include/gz/utils/detail/subprocess.h | 35 +++++++++++++++++++--------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/include/gz/utils/Subprocess.hh b/include/gz/utils/Subprocess.hh index 9128b24..87ae874 100644 --- a/include/gz/utils/Subprocess.hh +++ b/include/gz/utils/Subprocess.hh @@ -40,20 +40,6 @@ namespace gz namespace utils { -enum class Signals -{ -#if defined(_WIN32) - CTRL_C_EVENT = 0, - CTRL_BREAK_EVENT = 1, - CTRL_CLOSE_EVENT = 2, - CTRL_LOGOFF_EVENT = 5, - CTRL_SHUTDOWN_EVENT = 6, -#else - SIGNAL_SIGINT = 2, - SIGNAL_SIGQUIT = 3, -#endif -}; - /// \brief Create a RAII-type object that encapsulates a subprocess. class Subprocess { @@ -214,9 +200,9 @@ class Subprocess { int signal = 0; #if defined(_WIN32) - signal = static_cast(Signals::CTRL_C_EVENT); + signal = static_cast(CTRL_C_EVENT); #else - signal = static_cast(Signals::SIGNAL_SIGINT); + signal = static_cast(SIGINT); #endif return this->Signal(signal); } diff --git a/include/gz/utils/detail/subprocess.h b/include/gz/utils/detail/subprocess.h index 08e64ec..4d2aef4 100644 --- a/include/gz/utils/detail/subprocess.h +++ b/include/gz/utils/detail/subprocess.h @@ -170,6 +170,17 @@ subprocess_weak int subprocess_join(struct subprocess_s *const process, /// the parent process. subprocess_weak int subprocess_destroy(struct subprocess_s *const process); +/// @brief Signal a previously created process. +/// @param process The process to signal. +/// @param signal The signal number to send to the process. +/// @return On success zero is returned. +/// +/// Allows to send signals to the subprocess. For unix-based systems, +/// this includes any signal that appears in `kill -l`. For Windows +/// systems, this includes events available to GenerateConsoleCtrlEvent. +subprocess_weak int subprocess_signal(struct subprocess_s *const process, + int signum); + /// @brief Terminate a previously created process. /// @param process The process to terminate. /// @return On success zero is returned. @@ -923,17 +934,6 @@ FILE *subprocess_stderr(const struct subprocess_s *const process) { } } -int subprocess_signal(const subprocess_s *const process, - int signum) -{ -#if defined(_WIN32) - return GenerateConsoleCtrlEvent(signum, process->hProcess); -#else - return kill(process->child, signum); -#endif - -} - int subprocess_join(struct subprocess_s *const process, int *const out_return_code) { #if defined(_WIN32) @@ -1033,6 +1033,19 @@ int subprocess_destroy(struct subprocess_s *const process) { return 0; } +int subprocess_signal(const subprocess_s *const process, + int signum) +{ + int result; +#if defined(_WIN32) + auto processId = GetProcessId(process->hProcess); + result = GenerateConsoleCtrlEvent(signum, processId); +#else + result = kill(process->child, signum); +#endif + return result; +} + int subprocess_terminate(struct subprocess_s *const process) { #if defined(_WIN32) unsigned int killed_process_exit_code; From ee96f510c1f712c99dd4134a9aba9f948bc84fd8 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 2 Apr 2024 16:27:03 +0000 Subject: [PATCH 4/8] Create new process group Signed-off-by: Michael Carroll --- include/gz/utils/Subprocess.hh | 2 +- include/gz/utils/detail/subprocess.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/gz/utils/Subprocess.hh b/include/gz/utils/Subprocess.hh index 87ae874..aaaffe3 100644 --- a/include/gz/utils/Subprocess.hh +++ b/include/gz/utils/Subprocess.hh @@ -91,7 +91,7 @@ class Subprocess { } - + /// \brief Create a subprocess private: void Create() { if (this->process != nullptr) diff --git a/include/gz/utils/detail/subprocess.h b/include/gz/utils/detail/subprocess.h index 4d2aef4..eeb4194 100644 --- a/include/gz/utils/detail/subprocess.h +++ b/include/gz/utils/detail/subprocess.h @@ -497,6 +497,7 @@ int subprocess_create_ex(const char *const commandLine[], int options, const unsigned long startFUseStdHandles = 0x00000100; const unsigned long handleFlagInherit = 0x00000001; const unsigned long createNoWindow = 0x08000000; + const unsigned long createNewProcessGroup = 0x00000200; struct subprocess_subprocess_information_s processInfo; struct subprocess_security_attributes_s saAttr = {sizeof(saAttr), SUBPROCESS_NULL, 1}; @@ -523,6 +524,7 @@ int subprocess_create_ex(const char *const commandLine[], int options, startInfo.cb = sizeof(startInfo); startInfo.dwFlags = startFUseStdHandles; + flags |= createNewProcessGroup; if (subprocess_option_no_window == (options & subprocess_option_no_window)) { flags |= createNoWindow; } From 750f3af54caf64973dc8e6531b958ed06dde57bb Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 2 Apr 2024 16:29:52 +0000 Subject: [PATCH 5/8] Fix function Signed-off-by: Michael Carroll --- include/gz/utils/detail/subprocess.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/gz/utils/detail/subprocess.h b/include/gz/utils/detail/subprocess.h index eeb4194..d73a232 100644 --- a/include/gz/utils/detail/subprocess.h +++ b/include/gz/utils/detail/subprocess.h @@ -178,8 +178,7 @@ subprocess_weak int subprocess_destroy(struct subprocess_s *const process); /// Allows to send signals to the subprocess. For unix-based systems, /// this includes any signal that appears in `kill -l`. For Windows /// systems, this includes events available to GenerateConsoleCtrlEvent. -subprocess_weak int subprocess_signal(struct subprocess_s *const process, - int signum); +subprocess_weak int subprocess_signal(struct subprocess_s *const process, int signum); /// @brief Terminate a previously created process. /// @param process The process to terminate. @@ -1035,8 +1034,7 @@ int subprocess_destroy(struct subprocess_s *const process) { return 0; } -int subprocess_signal(const subprocess_s *const process, - int signum) +int subprocess_signal(subprocess_s *const process, int signum) { int result; #if defined(_WIN32) From f8129882d56a85a7d86768ae645bf439502137ff Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 2 Apr 2024 16:42:04 +0000 Subject: [PATCH 6/8] Send Ctrl-Break not Ctrl-C on Windows Signed-off-by: Michael Carroll --- include/gz/utils/Subprocess.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/gz/utils/Subprocess.hh b/include/gz/utils/Subprocess.hh index aaaffe3..ce23773 100644 --- a/include/gz/utils/Subprocess.hh +++ b/include/gz/utils/Subprocess.hh @@ -200,7 +200,7 @@ class Subprocess { int signal = 0; #if defined(_WIN32) - signal = static_cast(CTRL_C_EVENT); + signal = static_cast(CTRL_BREAK_EVENT); #else signal = static_cast(SIGINT); #endif From a1eaeadc30b0d022ae63480b0360ff6d58066310 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 2 Apr 2024 16:52:17 +0000 Subject: [PATCH 7/8] Use correct Windows exit code Signed-off-by: Michael Carroll --- test/integration/subprocess_TEST.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/integration/subprocess_TEST.cc b/test/integration/subprocess_TEST.cc index 8d61266..1e52ce3 100644 --- a/test/integration/subprocess_TEST.cc +++ b/test/integration/subprocess_TEST.cc @@ -220,7 +220,13 @@ TEST(Subprocess, Signal) // Block until the executable is done auto ret = proc.Join(); + +#if defined(_WIN32) + // Windows has a special exit code for Ctrl-C received + EXPECT_EQ(0xC000013A, ret) +#else EXPECT_EQ(1u, ret); +#endif auto end = std::chrono::steady_clock::now(); auto elapsed = From 376a27840ab4b93b2a23dc8f1b5e8d99f05d6870 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 2 Apr 2024 17:41:54 +0000 Subject: [PATCH 8/8] Missing semicolon Signed-off-by: Michael Carroll --- test/integration/subprocess_TEST.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/subprocess_TEST.cc b/test/integration/subprocess_TEST.cc index 1e52ce3..773c837 100644 --- a/test/integration/subprocess_TEST.cc +++ b/test/integration/subprocess_TEST.cc @@ -223,7 +223,7 @@ TEST(Subprocess, Signal) #if defined(_WIN32) // Windows has a special exit code for Ctrl-C received - EXPECT_EQ(0xC000013A, ret) + EXPECT_EQ(0xC000013A, ret); #else EXPECT_EQ(1u, ret); #endif