From fa7e3d443075b29a6106a310e61e0ee4d521f0b7 Mon Sep 17 00:00:00 2001 From: Michael Carlberg Date: Fri, 28 Oct 2016 18:51:07 +0200 Subject: [PATCH] fix(script): Terminate running commands Make sure the previous command has ended before executing a new command. This also fixes the execution block that was caused by the tailing the command output of action commands. Fixes jaagr/lemonbuddy#131 --- include/components/controller.hpp | 28 ++++++++++++++++------------ include/modules/meta.hpp | 1 - include/modules/script.hpp | 24 ++++++++++++++---------- include/utils/command.hpp | 25 +++++++++++++------------ 4 files changed, 43 insertions(+), 35 deletions(-) diff --git a/include/components/controller.hpp b/include/components/controller.hpp index 1443940d..e1c00ceb 100644 --- a/include/components/controller.hpp +++ b/include/components/controller.hpp @@ -81,10 +81,9 @@ class controller { } } - if (m_traymanager) { - m_log.trace("controller: Deactivate tray manager"); - m_traymanager->deactivate(); - m_traymanager.reset(); + if (m_command) { + m_log.info("Terminating running shell command"); + m_command->terminate(); } m_log.trace("controller: Deconstruct bar instance"); @@ -515,17 +514,21 @@ class controller { } } - m_log.trace("controller: Unrecognized input '%s'", input); - m_log.trace("controller: Forwarding input to shell"); - - auto command = command_util::make_command("/usr/bin/env\nsh\n-c\n" + input); + m_clickmtx.unlock(); try { - command->exec(false); - command->tail([this](std::string output) { m_log.trace("> %s", output); }); - command->wait(); + if (m_command) { + m_log.warn("Terminating previous shell command"); + m_command->terminate(); + } + + m_log.info("Executing shell command: %s", input); + + m_command = command_util::make_command(input); + m_command->exec(); + m_command.reset(); } catch (const application_error& err) { - m_log.err(err.what()); + m_log.err("controller: Error while forwarding input to shell -> %s", err.what()); } } @@ -550,6 +553,7 @@ class controller { vector m_threads; map> m_modules; + command_util::command_t m_command; unique_ptr m_throttler; throttle_util::strategy::try_once_or_leave_yolo m_throttle_strategy; diff --git a/include/modules/meta.hpp b/include/modules/meta.hpp index aff4794b..7248697f 100644 --- a/include/modules/meta.hpp +++ b/include/modules/meta.hpp @@ -294,7 +294,6 @@ namespace modules { void wakeup() { m_log.trace("%s: Release sleep lock", name()); - // std::unique_lock lck(m_sleeplock); m_sleephandler.notify_all(); } diff --git a/include/modules/script.hpp b/include/modules/script.hpp index dfe6ad14..095824b9 100644 --- a/include/modules/script.hpp +++ b/include/modules/script.hpp @@ -5,7 +5,6 @@ LEMONBUDDY_NS -#define SHELL_CMD "/usr/bin/env\nsh\n-c\n%cmd%" #define OUTPUT_ACTION(BUTTON) \ if (!m_actions[BUTTON].empty()) \ m_builder->cmd(BUTTON, string_util::replace_all(m_actions[BUTTON], "%counter%", counter_str)) @@ -35,11 +34,12 @@ namespace modules { } void stop() { + if (m_command && m_command->is_running()) { + m_log.warn("%s: Stopping shell command", name()); + m_command->terminate(); + } wakeup(); - enable(false); - m_command.reset(); - std::lock_guard lck(m_updatelock); - wakeup(); + event_module::stop(); } void idle() { @@ -64,7 +64,7 @@ namespace modules { auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter)); m_log.trace("%s: Executing '%s'", name(), exec); - m_command = command_util::make_command(string_util::replace(SHELL_CMD, "%cmd%", exec)); + m_command = command_util::make_command(exec); m_command->exec(false); } } catch (const std::exception& err) { @@ -89,13 +89,17 @@ namespace modules { return true; try { - auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter)); - auto cmd = command_util::make_command(string_util::replace(SHELL_CMD, "%cmd%", exec)); + if (m_command && m_command->is_running()) { + m_log.warn("%s: Previous shell command is still running...", name()); + return false; + } + auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter)); m_log.trace("%s: Executing '%s'", name(), exec); - cmd->exec(true); - cmd->tail([&](string output) { m_output = output; }); + m_command = command_util::make_command(exec); + m_command->exec(); + m_command->tail([&](string output) { m_output = output; }); } catch (const std::exception& err) { m_log.err("%s: %s", name(), err.what()); throw module_error("Failed to execute command, stopping module..."); diff --git a/include/utils/command.hpp b/include/utils/command.hpp index e6b79678..772d4fd0 100644 --- a/include/utils/command.hpp +++ b/include/utils/command.hpp @@ -32,7 +32,7 @@ namespace command_util { * * @code cpp * auto cmd = command_util::make_command( - * "/bin/sh\n-c\n while read -r line; do echo data from parent process: $line; done"); + * "while read -r line; do echo data from parent process: $line; done"); * cmd->exec(false); * cmd->writeline("Test"); * cout << cmd->readline(); @@ -40,8 +40,7 @@ namespace command_util { * @endcode * * @code cpp - * vector exec{{"/bin/sh"}, {"-c"}, {"for i in 1 2 3; do echo $i; done"}}; - * auto cmd = command_util::make_command(exec); + * 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 @@ -49,10 +48,8 @@ namespace command_util { */ class command { public: - explicit command(const logger& logger, vector cmd) - : command(logger, string_util::join(cmd, "\n")) {} - - explicit command(const logger& logger, string cmd) : m_log(logger), m_cmd(cmd) { + explicit command(const logger& logger, string cmd) + : m_log(logger), m_cmd("/usr/bin/env\nsh\n-c\n" + cmd) { if (pipe(m_stdin) != 0) throw command_strerror("Failed to allocate input stream"); if (pipe(m_stdout) != 0) @@ -140,10 +137,10 @@ namespace command_util { * Wait for the child processs to finish */ int wait() { - auto waitflags = WCONTINUED | WUNTRACED; - do { - process_util::wait_for_completion(m_forkpid, &m_forkstatus, waitflags); + 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.warn("command: Exited with failed status %d", WEXITSTATUS(m_forkstatus)); @@ -155,6 +152,8 @@ namespace command_util { m_log.trace("command: Stopped by signal %d", WSTOPSIG(m_forkstatus)); else if (WIFCONTINUED(m_forkstatus) == true) m_log.trace("command: Continued"); + else + break; } while (!WIFEXITED(m_forkstatus) && !WIFSIGNALED(m_forkstatus)); return m_forkstatus; @@ -162,6 +161,9 @@ namespace command_util { /** * Tail command output + * + * @note: This is a blocking call and will not + * end until the stream is closed */ void tail(function callback) { io_util::tail(m_stdout[PIPE_READ], callback); @@ -238,8 +240,7 @@ namespace command_util { template command_t make_command(Args&&... args) { - return make_unique( - configure_logger().create(), forward(args)...); + return make_unique(configure_logger().create(), forward(args)...); } }