From acb9ed910daa98029d6d88a007874c6edca9711f Mon Sep 17 00:00:00 2001
From: Michael Carlberg <c@rlberg.se>
Date: Sat, 31 Dec 2016 14:08:44 +0100
Subject: [PATCH] fix(script): Unlock mutex before terminating command

Refs #281
---
 include/modules/script.hpp |  3 +++
 include/utils/process.hpp  |  2 --
 src/modules/script.cpp     | 30 ++++++++++++------------------
 src/utils/command.cpp      | 22 +++++-----------------
 src/utils/process.cpp      |  9 ---------
 5 files changed, 20 insertions(+), 46 deletions(-)

diff --git a/include/modules/script.hpp b/include/modules/script.hpp
index 88a1168d..d682a155 100644
--- a/include/modules/script.hpp
+++ b/include/modules/script.hpp
@@ -14,6 +14,9 @@ namespace chrono = std::chrono;
   m_builder->cmd(BUTTON, string_util::replace_all(m_actions[BUTTON], "%counter%", counter_str))
 
 namespace modules {
+  /**
+   * TODO: Split into timed-/streaming modules
+   */
   class script_module : public event_module<script_module> {
    public:
     explicit script_module(const bar_settings&, string);
diff --git a/include/utils/process.hpp b/include/utils/process.hpp
index 84fa1d91..8210ac49 100644
--- a/include/utils/process.hpp
+++ b/include/utils/process.hpp
@@ -19,8 +19,6 @@ namespace process_util {
   pid_t wait_for_completion_nohang();
 
   bool notify_childprocess();
-
-  void unblock_signal(int sig);
 }
 
 POLYBAR_NS_END
diff --git a/src/modules/script.cpp b/src/modules/script.cpp
index 39abc2c5..188512b2 100644
--- a/src/modules/script.cpp
+++ b/src/modules/script.cpp
@@ -34,12 +34,13 @@ namespace modules {
   }
 
   void script_module::stop() {
+    m_updatelock.unlock();
+    event_module::stop();
+
     if (m_command && m_command->is_running()) {
       m_log.warn("%s: Stopping shell command", name());
       m_command->terminate();
     }
-    wakeup();
-    event_module::stop();
   }
 
   void script_module::idle() {
@@ -53,26 +54,19 @@ namespace modules {
   bool script_module::has_event() {
     if (!m_tail) {
       return true;
-    }
-
-    try {
-      if (!m_command || !m_command->is_running()) {
-        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(exec);
+    } else if (!m_command || !m_command->is_running()) {
+      try {
+        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(move(exec));
         m_command->exec(false);
+      } catch (const std::exception& err) {
+        m_log.err("%s: %s", name(), err.what());
+        throw module_error("Failed to execute tail command, stopping module...");
       }
-    } catch (const std::exception& err) {
-      m_log.err("%s: %s", name(), err.what());
-      throw module_error("Failed to execute tail command, stopping module...");
     }
 
-    if (!m_command) {
-      return false;
-    }
-
-    if ((m_output = m_command->readline()) == m_prev) {
+    if (!m_command || (m_output = m_command->readline()) == m_prev) {
       return false;
     }
 
diff --git a/src/utils/command.cpp b/src/utils/command.cpp
index 144a211a..c4c2e064 100644
--- a/src/utils/command.cpp
+++ b/src/utils/command.cpp
@@ -23,7 +23,6 @@ command::~command() {
   if (is_running()) {
     terminate();
   }
-
   if (m_stdin[PIPE_READ] > 0) {
     close(m_stdin[PIPE_READ]);
   }
@@ -71,9 +70,6 @@ int command::exec(bool wait_for_completion) {
       throw command_error("Failed to close fd");
     }
 
-    // Make sure SIGTERM is raised
-    process_util::unblock_signal(SIGTERM);
-
     setpgid(m_forkpid, 0);
     process_util::exec_sh(m_cmd.c_str());
   } else {
@@ -96,16 +92,11 @@ int command::exec(bool wait_for_completion) {
 }
 
 void command::terminate() {
-  try {
-    if (is_running()) {
-      m_log.trace("command: Sending SIGTERM to running child process (%d)", m_forkpid);
-      killpg(m_forkpid, SIGTERM);
-      wait();
-    }
-  } catch (const command_error& err) {
-    m_log.warn("%s", err.what());
+  if (is_running()) {
+    m_log.trace("command: Sending SIGTERM to running child process (%d)", m_forkpid);
+    killpg(m_forkpid, SIGTERM);
+    wait();
   }
-
   m_forkpid = -1;
 }
 
@@ -113,10 +104,7 @@ void command::terminate() {
  * Check if command is running
  */
 bool command::is_running() {
-  if (m_forkpid > 0) {
-    return process_util::wait_for_completion_nohang(m_forkpid, &m_forkstatus) > -1;
-  }
-  return false;
+  return m_forkpid > 0 && process_util::wait_for_completion_nohang(m_forkpid, &m_forkstatus) > -1;
 }
 
 /**
diff --git a/src/utils/process.cpp b/src/utils/process.cpp
index 47b0d5da..56db644f 100644
--- a/src/utils/process.cpp
+++ b/src/utils/process.cpp
@@ -106,15 +106,6 @@ namespace process_util {
   bool notify_childprocess() {
     return wait_for_completion_nohang() > 0;
   }
-
-  void unblock_signal(int sig) {
-    sigset_t sigmask{};
-    sigemptyset(&sigmask);
-    sigaddset(&sigmask, sig);
-    if (pthread_sigmask(SIG_UNBLOCK, &sigmask, nullptr) == -1) {
-      throw system_error("Failed to change pthread_sigmask");
-    }
-  }
 }
 
 POLYBAR_NS_END