controller: Detach shell commands from polybar

Shell commands triggered from action tags used to block polybar until
they finished.

Since we are not actually interested in the output of the commands, it
makes sense to run them completely detached from polybar and have
polybar not block when executing these commands.

Now the spawned child processes no longer get killed when polybar
exits. This is fine because polybar is not responsible for these
processes since they were explicitly started by the user through click
commands.

Ref: #770
Ref: #1680

include/components/controller.hpp

@@ -20,8 +20,6 @@ POLYBAR_NS
 
 enum class alignment;
 class bar;
-template <output_policy>
-class command;
 class config;
 class connection;
 class inotify_watch;

include/utils/process.hpp

@@ -8,19 +8,20 @@ namespace process_util {
   bool in_parent_process(pid_t pid);
   bool in_forked_process(pid_t pid);
 
-  void redirect_process_output_to_dev_null();
+  void redirect_stdio_to_dev_null();
+
+  pid_t fork_detached(std::function<void()> const& lambda);
 
   void exec(char* cmd, char** args);
   void exec_sh(const char* cmd);
 
-  pid_t wait_for_completion(pid_t process_id, int* status_addr, int waitflags = 0);
+  pid_t wait_for_completion(pid_t process_id, int* status_addr = nullptr, int waitflags = 0);
   pid_t wait_for_completion(int* status_addr, int waitflags = 0);
-  pid_t wait_for_completion(pid_t process_id);
   pid_t wait_for_completion_nohang(pid_t process_id, int* status);
   pid_t wait_for_completion_nohang(int* status);
   pid_t wait_for_completion_nohang();
 
   bool notify_childprocess();
-}
+}  // namespace process_util
 
 POLYBAR_NS_END

src/components/controller.cpp

@@ -15,9 +15,9 @@
 #include "modules/meta/event_handler.hpp"
 #include "modules/meta/factory.hpp"
 #include "utils/actions.hpp"
-#include "utils/command.hpp"
 #include "utils/factory.hpp"
 #include "utils/inotify.hpp"
+#include "utils/process.hpp"
 #include "utils/string.hpp"
 #include "utils/time.hpp"
 #include "x11/connection.hpp"
@@ -561,12 +561,8 @@ void controller::process_inputdata() {
   try {
     // Run input as command if it's not an input for a module
     m_log.info("Forwarding command to shell... (input: %s)", cmd);
-
     m_log.info("Executing shell command: %s", cmd);
-
-    auto shell_cmd = command_util::make_command<output_policy::IGNORED>(move(cmd));
-    shell_cmd->exec();
-    shell_cmd.reset();
+    process_util::fork_detached([cmd] { process_util::exec_sh(cmd.c_str()); });
     process_update(true);
   } catch (const application_error& err) {
     m_log.err("controller: Error while forwarding input to shell -> %s", err.what());

src/utils/command.cpp

@@ -1,12 +1,14 @@
+#include "utils/command.hpp"
+
 #include <sys/wait.h>
 #include <unistd.h>
+
 #include <csignal>
 #include <cstdlib>
 #include <utility>
 #include <utils/string.hpp>
 
 #include "errors.hpp"
-#include "utils/command.hpp"
 #include "utils/io.hpp"
 #include "utils/process.hpp"
 
@@ -31,20 +33,11 @@ command<output_policy::IGNORED>::~command() {
  * Execute the command
  */
 int command<output_policy::IGNORED>::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::redirect_process_output_to_dev_null();
-    process_util::exec_sh(m_cmd.c_str());
-  } else {
-    if (wait_for_completion) {
-      auto status = wait();
-      m_forkpid = -1;
-      return status;
-    }
+  m_forkpid = process_util::fork_detached([m_cmd = m_cmd] { process_util::exec_sh(m_cmd.c_str()); });
+  if (wait_for_completion) {
+    auto status = wait();
+    m_forkpid = -1;
+    return status;
   }
 
   return EXIT_SUCCESS;
@@ -90,7 +83,7 @@ int command<output_policy::IGNORED>::wait() {
     }
   } while (!WIFEXITED(m_forkstatus) && !WIFSIGNALED(m_forkstatus));
 
-  return m_forkstatus;
+  return WEXITSTATUS(m_forkstatus);
 }
 
 /**
@@ -104,7 +97,7 @@ pid_t command<output_policy::IGNORED>::get_pid() {
  * Get command exit status
  */
 int command<output_policy::IGNORED>::get_exit_status() {
-  return m_forkstatus;
+  return WEXITSTATUS(m_forkstatus);
 }
 
 command<output_policy::REDIRECTED>::command(const polybar::logger& logger, std::string cmd)

src/utils/process.cpp

@@ -1,6 +1,8 @@
 #include "utils/process.hpp"
 
 #include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
@@ -25,7 +27,10 @@ namespace process_util {
     return pid == 0;
   }
 
-  void redirect_process_output_to_dev_null() {
+  /**
+   * Redirects all io fds (stdin, stdout, stderr) of the current process to /dev/null.
+   */
+  void redirect_stdio_to_dev_null() {
     auto redirect = [](int fd_to_redirect) {
       int fd = open("/dev/null", O_WRONLY);
       if (fd < 0 || dup2(fd, fd_to_redirect) < 0) {
@@ -34,10 +39,38 @@ namespace process_util {
       close(fd);
     };
 
+    redirect(STDIN_FILENO);
     redirect(STDOUT_FILENO);
     redirect(STDERR_FILENO);
   }
 
+  /**
+   * Forks a child process and completely detaches it.
+   *
+   * In the child process, the given lambda function is executed.
+   *
+   * Use this if you want to run a command and just forget about it.
+   *
+   * \returns The PID of the child process
+   */
+  pid_t fork_detached(std::function<void()> const& lambda) {
+    pid_t pid = fork();
+    switch (pid) {
+      case -1:
+        throw runtime_error("fork_detached: Unable to fork: " + string(strerror(errno)));
+      case 0:
+        // Child
+        setsid();
+        umask(0);
+        redirect_stdio_to_dev_null();
+        lambda();
+        _Exit(0);
+        break;
+      default:
+        return pid;
+    }
+  }
+
   /**
    * Execute command
    */
@@ -76,14 +109,6 @@ namespace process_util {
     return wait_for_completion(-1, status_addr, waitflags);
   }
 
-  /**
-   * Wait for child process
-   */
-  pid_t wait_for_completion(pid_t process_id) {
-    int status = 0;
-    return wait_for_completion(process_id, &status);
-  }
-
   /**
    * Non-blocking wait
    *

tests/CMakeLists.txt

@@ -55,6 +55,7 @@ add_unit_test(utils/memory unit_tests)
 add_unit_test(utils/scope unit_tests)
 add_unit_test(utils/string unit_tests)
 add_unit_test(utils/file)
+add_unit_test(utils/process)
 add_unit_test(components/command_line)
 add_unit_test(components/bar)
 add_unit_test(components/parser)

tests/unit_tests/utils/process.cpp

@@ -0,0 +1,34 @@
+#include "utils/process.hpp"
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <chrono>
+#include <thread>
+
+#include "common/test.hpp"
+
+using namespace polybar;
+using namespace process_util;
+
+TEST(ForkDetached, is_detached) {
+  pid_t pid = fork_detached([] { exec_sh("sleep 0.1"); });
+  int status;
+
+  pid_t res = process_util::wait_for_completion_nohang(pid, &status);
+
+  ASSERT_NE(res, -1);
+
+  EXPECT_FALSE(WIFEXITED(status));
+}
+
+TEST(ForkDetached, exit_code) {
+  pid_t pid = fork_detached([] { exec_sh("exit 42"); });
+  int status = 0;
+  pid_t res = waitpid(pid, &status, 0);
+
+  EXPECT_EQ(res, pid);
+
+  EXPECT_EQ(WEXITSTATUS(status), 42);
+}