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

@@ -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<thread> m_threads;
   map<alignment, vector<module_t>> m_modules;
+  command_util::command_t m_command;
 
   unique_ptr<throttle_util::event_throttler> m_throttler;
   throttle_util::strategy::try_once_or_leave_yolo m_throttle_strategy;

include/modules/meta.hpp

@@ -294,7 +294,6 @@ namespace modules {
 
     void wakeup() {
       m_log.trace("%s: Release sleep lock", name());
-      // std::unique_lock<std::mutex> lck(m_sleeplock);
       m_sleephandler.notify_all();
     }
 

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<threading_util::spin_lock> 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...");

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<string> 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<string> 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<void(string)> callback) {
       io_util::tail(m_stdout[PIPE_READ], callback);
@@ -238,8 +240,7 @@ namespace command_util {
 
   template <typename... Args>
   command_t make_command(Args&&... args) {
-    return make_unique<command>(
-        configure_logger().create<const logger&>(), forward<Args>(args)...);
+    return make_unique<command>(configure_logger().create<const logger&>(), forward<Args>(args)...);
   }
 }