From 5e5d8faf04c0a711e862d50a1d8c76370eb6072d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20BOULMIER?= Date: Thu, 7 Mar 2019 00:22:00 -0500 Subject: [PATCH] fix(command): broken pipe when ignoring output. --- include/components/controller.hpp | 3 +- include/components/types.hpp | 5 + include/modules/script.hpp | 2 +- include/utils/command.hpp | 73 +++++++++---- src/adapters/net.cpp | 2 +- src/components/controller.cpp | 2 +- src/modules/ipc.cpp | 4 +- src/modules/script.cpp | 6 +- src/utils/command.cpp | 169 +++++++++++++++++------------- 9 files changed, 169 insertions(+), 97 deletions(-) diff --git a/include/components/controller.hpp b/include/components/controller.hpp index 0bf6f667..f37efd85 100644 --- a/include/components/controller.hpp +++ b/include/components/controller.hpp @@ -18,6 +18,7 @@ POLYBAR_NS enum class alignment; class bar; +template class command; class config; class connection; @@ -80,7 +81,7 @@ class controller unique_ptr m_bar; unique_ptr m_ipc; unique_ptr m_confwatch; - unique_ptr m_command; + unique_ptr> m_command; array, 2> m_queuefd{}; diff --git a/include/components/types.hpp b/include/components/types.hpp index 558a77cf..8125d4b6 100644 --- a/include/components/types.hpp +++ b/include/components/types.hpp @@ -236,4 +236,9 @@ struct event_timer { }; }; +enum class output_policy { + REDIRECTED, + IGNORED, +}; + POLYBAR_NS_END diff --git a/include/modules/script.hpp b/include/modules/script.hpp index d2db896e..7d46d4e3 100644 --- a/include/modules/script.hpp +++ b/include/modules/script.hpp @@ -27,7 +27,7 @@ namespace modules { mutex_wrapper()>> m_handler; - unique_ptr m_command; + unique_ptr> m_command; bool m_tail; diff --git a/include/utils/command.hpp b/include/utils/command.hpp index 91841bab..1619e3e3 100644 --- a/include/utils/command.hpp +++ b/include/utils/command.hpp @@ -4,6 +4,7 @@ #include "common.hpp" #include "components/logger.hpp" +#include "components/types.hpp" #include "errors.hpp" #include "utils/factory.hpp" #include "utils/functional.hpp" @@ -15,17 +16,22 @@ DEFINE_ERROR(command_error); /** * Wrapper used to execute command in a subprocess. * In-/output streams are opened to enable ipc. + * If the command is created using command_util::make_command, the child streams are + * redirected and you can read the program output or write into the program input. + * + * If the command is created using command_util::make_command, the output is not redirected and + * you can't communicate with the child program. * * Example usage: * * \code cpp - * auto cmd = command_util::make_command("cat /etc/rc.local"); + * auto cmd = command_util::make_command("cat /etc/rc.local"); * cmd->exec(); * cmd->tail([](string s) { std::cout << s << std::endl; }); * \endcode * * \code cpp - * auto cmd = command_util::make_command( + * auto cmd = command_util::make_command( * "while read -r line; do echo data from parent process: $line; done"); * cmd->exec(false); * cmd->writeline("Test"); @@ -34,29 +40,34 @@ DEFINE_ERROR(command_error); * \endcode * * \code cpp - * auto cmd = command_util::make_command("for i in 1 2 3; do echo $i; done"); + * auto cmd = command_util::make_command("for i in 1 2 3; do echo $i; done"); * cmd->exec(); * cout << cmd->readline(); // 1 * cout << cmd->readline() << cmd->readline(); // 23 * \endcode + * + * \code cpp + * auto cmd = command_util::make_command("ping kernel.org"); + * int status = cmd->exec(); + * \endcode */ -class command { +template +class command; + +template <> +class command { public: explicit command(const logger& logger, string cmd); - + command(const command&) = delete; ~command(); + command& operator=(const command&) = delete; + int exec(bool wait_for_completion = true); void terminate(); bool is_running(); int wait(); - void tail(callback cb); - int writeline(string data); - string readline(); - - int get_stdout(int c); - int get_stdin(int c); pid_t get_pid(); int get_exit_status(); @@ -65,20 +76,46 @@ class command { string m_cmd; - int m_stdout[2]{}; - int m_stdin[2]{}; - pid_t m_forkpid{}; int m_forkstatus{}; +}; + +template <> +class command : private command { + public: + explicit command(const logger& logger, string cmd); + command(const command&) = delete; + ~command(); + + command& operator=(const command&) = delete; + + int exec(bool wait_for_completion = true); + using command::terminate; + using command::is_running; + using command::wait; + + using command::get_pid; + using command::get_exit_status; + + void tail(callback cb); + int writeline(string data); + string readline(); + + int get_stdout(int c); + int get_stdin(int c); + + protected: + int m_stdout[2]{}; + int m_stdin[2]{}; std::mutex m_pipelock{}; }; namespace command_util { - template - unique_ptr make_command(Args&&... args) { - return factory_util::unique(logger::make(), forward(args)...); + template + unique_ptr> make_command(Args&&... args) { + return factory_util::unique>(logger::make(), forward(args)...); } -} +} // namespace command_util POLYBAR_NS_END diff --git a/src/adapters/net.cpp b/src/adapters/net.cpp index 97fafb14..d9a9ca59 100644 --- a/src/adapters/net.cpp +++ b/src/adapters/net.cpp @@ -129,7 +129,7 @@ namespace net { bool network::ping() const { try { auto exec = "ping -c 2 -W 2 -I " + m_interface + " " + string(CONNECTION_TEST_IP); - auto ping = command_util::make_command(exec); + auto ping = command_util::make_command(exec); return ping && ping->exec(true) == EXIT_SUCCESS; } catch (const std::exception& err) { return false; diff --git a/src/components/controller.cpp b/src/components/controller.cpp index d98cd9fd..5d2a86b9 100644 --- a/src/components/controller.cpp +++ b/src/components/controller.cpp @@ -408,7 +408,7 @@ void controller::process_inputdata() { } m_log.info("Executing shell command: %s", cmd); - m_command = command_util::make_command(move(cmd)); + m_command = command_util::make_command(move(cmd)); m_command->exec(); m_command.reset(); process_update(true); diff --git a/src/modules/ipc.cpp b/src/modules/ipc.cpp index 18a6c3e1..f18b62db 100644 --- a/src/modules/ipc.cpp +++ b/src/modules/ipc.cpp @@ -60,7 +60,7 @@ namespace modules { */ void ipc_module::start() { if (m_initial) { - auto command = command_util::make_command(m_hooks.at(m_initial - 1)->command); + auto command = command_util::make_command(m_hooks.at(m_initial - 1)->command); command->exec(false); command->tail([this](string line) { m_output = line; }); } @@ -114,7 +114,7 @@ namespace modules { try { // Clear the output in case the command produces no output m_output.clear(); - auto command = command_util::make_command(hook->command); + auto command = command_util::make_command(hook->command); command->exec(false); command->tail([this](string line) { m_output = line; }); } catch (const exception& err) { diff --git a/src/modules/script.cpp b/src/modules/script.cpp index 3883eb19..de8d3479 100644 --- a/src/modules/script.cpp +++ b/src/modules/script.cpp @@ -22,7 +22,7 @@ namespace modules { if (!m_command || !m_command->is_running()) { string exec{string_util::replace_all(m_exec, "%counter%", to_string(++m_counter))}; m_log.info("%s: Invoking shell command: \"%s\"", name(), exec); - m_command = command_util::make_command(exec); + m_command = command_util::make_command(exec); try { m_command->exec(false); @@ -59,7 +59,7 @@ namespace modules { try { auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter)); m_log.info("%s: Invoking shell command: \"%s\"", name(), exec); - m_command = command_util::make_command(exec); + m_command = command_util::make_command(exec); m_command->exec(true); } catch (const exception& err) { m_log.err("%s: %s", name(), err.what()); @@ -143,7 +143,7 @@ namespace modules { bool script_module::check_condition() { if (m_exec_if.empty()) { return true; - } else if (command_util::make_command(m_exec_if)->exec(true) == 0) { + } else if (command_util::make_command(m_exec_if)->exec(true) == 0) { return true; } else if (!m_output.empty()) { broadcast(); diff --git a/src/utils/command.cpp b/src/utils/command.cpp index 8513763e..1d08ae4b 100644 --- a/src/utils/command.cpp +++ b/src/utils/command.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "errors.hpp" #include "utils/command.hpp" @@ -18,7 +19,95 @@ POLYBAR_NS -command::command(const logger& logger, string cmd) : m_log(logger), m_cmd(move(cmd)) { +command::command(const logger& logger, string cmd) : m_log(logger), m_cmd(move(cmd)) {} + +command::~command() { + if (is_running()) { + terminate(); + } +} + +/** + * Execute the command + */ +int command::exec(bool wait_for_completion) { + if ((m_forkpid = fork()) == -1) { + throw system_error("Failed to fork process"); + } + + if (process_util::in_forked_process(m_forkpid)) { + setpgid(m_forkpid, 0); + process_util::exec_sh(m_cmd.c_str()); + } else { + if (wait_for_completion) { + auto status = wait(); + m_forkpid = -1; + return status; + } + } + + return EXIT_SUCCESS; +} + +void command::terminate() { + if (is_running()) { + m_log.trace("command: Sending SIGTERM to running child process (%d)", m_forkpid); + killpg(m_forkpid, SIGTERM); + wait(); + } + m_forkpid = -1; +} + +/** + * Check if command is running + */ +bool command::is_running() { + return m_forkpid > 0 && process_util::wait_for_completion_nohang(m_forkpid, &m_forkstatus) > -1; +} + +/** + * Wait for the child processs to finish + */ +int command::wait() { + do { + m_log.trace("command: Waiting for pid %d to finish...", m_forkpid); + + process_util::wait_for_completion(m_forkpid, &m_forkstatus, WCONTINUED | WUNTRACED); + + if (WIFEXITED(m_forkstatus) && m_forkstatus > 0) { + m_log.trace("command: Exited with failed status %d", WEXITSTATUS(m_forkstatus)); + } else if (WIFEXITED(m_forkstatus)) { + m_log.trace("command: Exited with status %d", WEXITSTATUS(m_forkstatus)); + } else if (WIFSIGNALED(m_forkstatus)) { + m_log.trace("command: killed by signal %d", WTERMSIG(m_forkstatus)); + } else if (WIFSTOPPED(m_forkstatus)) { + m_log.trace("command: Stopped by signal %d", WSTOPSIG(m_forkstatus)); + } else if (WIFCONTINUED(m_forkstatus)) { + m_log.trace("command: Continued"); + } else { + break; + } + } while (!WIFEXITED(m_forkstatus) && !WIFSIGNALED(m_forkstatus)); + + return m_forkstatus; +} + +/** + * Get command pid + */ +pid_t command::get_pid() { + return m_forkpid; +} + +/** + * Get command exit status + */ +int command::get_exit_status() { + return m_forkstatus; +} + +command::command(const polybar::logger& logger, std::string cmd) + : command(logger, move(cmd)) { if (pipe(m_stdin) != 0) { throw command_error("Failed to allocate input stream"); } @@ -27,10 +116,7 @@ command::command(const logger& logger, string cmd) : m_log(logger), m_cmd(move(c } } -command::~command() { - if (is_running()) { - terminate(); - } +command::~command() { if (m_stdin[PIPE_READ] > 0) { close(m_stdin[PIPE_READ]); } @@ -48,7 +134,7 @@ command::~command() { /** * Execute the command */ -int command::exec(bool wait_for_completion) { +int command::exec(bool wait_for_completion) { if ((m_forkpid = fork()) == -1) { throw system_error("Failed to fork process"); } @@ -99,71 +185,28 @@ int command::exec(bool wait_for_completion) { return EXIT_SUCCESS; } -void command::terminate() { - if (is_running()) { - m_log.trace("command: Sending SIGTERM to running child process (%d)", m_forkpid); - killpg(m_forkpid, SIGTERM); - wait(); - } - m_forkpid = -1; -} - -/** - * Check if command is running - */ -bool command::is_running() { - return m_forkpid > 0 && process_util::wait_for_completion_nohang(m_forkpid, &m_forkstatus) > -1; -} - -/** - * Wait for the child processs to finish - */ -int command::wait() { - do { - m_log.trace("command: Waiting for pid %d to finish...", m_forkpid); - - process_util::wait_for_completion(m_forkpid, &m_forkstatus, WCONTINUED | WUNTRACED); - - if (WIFEXITED(m_forkstatus) && m_forkstatus > 0) { - m_log.trace("command: Exited with failed status %d", WEXITSTATUS(m_forkstatus)); - } else if (WIFEXITED(m_forkstatus)) { - m_log.trace("command: Exited with status %d", WEXITSTATUS(m_forkstatus)); - } else if (WIFSIGNALED(m_forkstatus)) { - m_log.trace("command: killed by signal %d", WTERMSIG(m_forkstatus)); - } else if (WIFSTOPPED(m_forkstatus)) { - m_log.trace("command: Stopped by signal %d", WSTOPSIG(m_forkstatus)); - } else if (WIFCONTINUED(m_forkstatus)) { - m_log.trace("command: Continued"); - } else { - break; - } - } while (!WIFEXITED(m_forkstatus) && !WIFSIGNALED(m_forkstatus)); - - return m_forkstatus; -} - /** * Tail command output * * \note: This is a blocking call and will not * end until the stream is closed */ -void command::tail(callback cb) { - io_util::tail(m_stdout[PIPE_READ], move(cb)); +void command::tail(callback cb) { + io_util::tail(m_stdout[PIPE_READ], cb); } /** * Write line to command input channel */ -int command::writeline(string data) { +int command::writeline(string data) { std::lock_guard lck(m_pipelock); - return io_util::writeline(m_stdin[PIPE_WRITE], move(data)); + return static_cast(io_util::writeline(m_stdin[PIPE_WRITE], data)); } /** * Read a line from the commands output stream */ -string command::readline() { +string command::readline() { std::lock_guard lck(m_pipelock); return io_util::readline(m_stdout[PIPE_READ]); } @@ -171,29 +214,15 @@ string command::readline() { /** * Get command output channel */ -int command::get_stdout(int c) { +int command::get_stdout(int c) { return m_stdout[c]; } /** * Get command input channel */ -int command::get_stdin(int c) { +int command::get_stdin(int c) { return m_stdin[c]; } -/** - * Get command pid - */ -pid_t command::get_pid() { - return m_forkpid; -} - -/** - * Get command exit status - */ -int command::get_exit_status() { - return m_forkstatus; -} - POLYBAR_NS_END