From 47483a94f1302746cae31bea789ced95bc9aeffe Mon Sep 17 00:00:00 2001
From: patrick96 <p.ziegler96@gmail.com>
Date: Sat, 12 Dec 2020 02:21:36 +0100
Subject: [PATCH] fix(process): fork_detached created zombie processes

Since the forked processes are still our children, we need to wait on
them, otherwise they become zombie processes.

We now fork twice, let the first fork immediately return and wait on it.
This reparents the second fork, which runs the actual code, to the init
process which then collects it.

Ref #770
---
 include/utils/process.hpp          |  5 ++-
 src/utils/command.cpp              |  2 +-
 src/utils/process.cpp              | 63 ++++++++++++++++++++++++++----
 tests/unit_tests/utils/process.cpp |  8 ++--
 4 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/include/utils/process.hpp b/include/utils/process.hpp
index 72b21015..3ab5fd7b 100644
--- a/include/utils/process.hpp
+++ b/include/utils/process.hpp
@@ -12,11 +12,14 @@ namespace process_util {
 
   void redirect_stdio_to_dev_null();
 
-  pid_t fork_detached(std::function<void()> const& lambda);
+  pid_t spawn_async(std::function<void()> const& lambda);
+  void fork_detached(std::function<void()> const& lambda);
 
   void exec(char* cmd, char** args);
   void exec_sh(const char* cmd);
 
+  int wait(pid_t pid);
+
   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_nohang(pid_t process_id, int* status);
diff --git a/src/utils/command.cpp b/src/utils/command.cpp
index 2f2afc2c..7e8d3a9f 100644
--- a/src/utils/command.cpp
+++ b/src/utils/command.cpp
@@ -33,7 +33,7 @@ command<output_policy::IGNORED>::~command() {
  * Execute the command
  */
 int command<output_policy::IGNORED>::exec(bool wait_for_completion) {
-  m_forkpid = process_util::fork_detached([m_cmd = m_cmd] { process_util::exec_sh(m_cmd.c_str()); });
+  m_forkpid = process_util::spawn_async([m_cmd = m_cmd] { process_util::exec_sh(m_cmd.c_str()); });
   if (wait_for_completion) {
     auto status = wait();
     m_forkpid = -1;
diff --git a/src/utils/process.cpp b/src/utils/process.cpp
index f5ec0857..1127c606 100644
--- a/src/utils/process.cpp
+++ b/src/utils/process.cpp
@@ -45,15 +45,11 @@ namespace process_util {
   }
 
   /**
-   * Forks a child process and completely detaches it.
+   * Forks a child process and executes the given lambda function in 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
+   * Processes spawned this way need to be waited on by the caller.
    */
-  pid_t fork_detached(std::function<void()> const& lambda) {
+  pid_t spawn_async(std::function<void()> const& lambda) {
     pid_t pid = fork();
     switch (pid) {
       case -1:
@@ -71,6 +67,50 @@ namespace process_util {
     }
   }
 
+  /**
+   * Forks a child process and completely detaches it.
+   *
+   * In the child process, the given lambda function is executed.
+   * We fork twice so that the first forked process can exit and it's child is
+   * reparented to the init process.
+   *
+   * Ref: https://web.archive.org/web/20120914180018/http://www.steve.org.uk/Reference/Unix/faq_2.html#SEC16
+   *
+   * Use this if you want to run a command and just forget about it.
+   *
+   * \returns The PID of the child process
+   */
+  void 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();
+
+        pid = fork();
+        switch (pid) {
+          case -1:
+            throw runtime_error("fork_detached: Unable to fork: " + string(strerror(errno)));
+          case 0:
+            // Child
+            umask(0);
+            redirect_stdio_to_dev_null();
+            lambda();
+            _Exit(0);
+        }
+
+        _Exit(0);
+      default:
+        /*
+         * The first fork immediately exits and we have to collect its exit
+         * status
+         */
+        wait(pid);
+    }
+  }
+
   /**
    * Execute command
    */
@@ -92,6 +132,15 @@ namespace process_util {
     }
   }
 
+  int wait(pid_t pid) {
+    int forkstatus;
+    do {
+      process_util::wait_for_completion(pid, &forkstatus, WCONTINUED | WUNTRACED);
+    } while (!WIFEXITED(forkstatus) && !WIFSIGNALED(forkstatus));
+
+    return WEXITSTATUS(forkstatus);
+  }
+
   /**
    * Wait for child process
    */
diff --git a/tests/unit_tests/utils/process.cpp b/tests/unit_tests/utils/process.cpp
index cfd8351c..64ff21f6 100644
--- a/tests/unit_tests/utils/process.cpp
+++ b/tests/unit_tests/utils/process.cpp
@@ -12,8 +12,8 @@
 using namespace polybar;
 using namespace process_util;
 
-TEST(ForkDetached, is_detached) {
-  pid_t pid = fork_detached([] { exec_sh("sleep 0.1"); });
+TEST(SpawnAsync, is_async) {
+  pid_t pid = spawn_async([] { exec_sh("sleep 0.1"); });
   int status;
 
   pid_t res = process_util::wait_for_completion_nohang(pid, &status);
@@ -23,8 +23,8 @@ TEST(ForkDetached, is_detached) {
   EXPECT_FALSE(WIFEXITED(status));
 }
 
-TEST(ForkDetached, exit_code) {
-  pid_t pid = fork_detached([] { exec_sh("exit 42"); });
+TEST(SpawnAsync, exit_code) {
+  pid_t pid = spawn_async([] { exec_sh("exit 42"); });
   int status = 0;
   pid_t res = waitpid(pid, &status, 0);