From c3359dec1b3a711f2912aab6c11009a8c46d2fe2 Mon Sep 17 00:00:00 2001
From: excellentname <excellentname@protonmail.com>
Date: Sat, 25 Jul 2020 21:02:59 +1000
Subject: [PATCH] Replace signal handler with signal handling thread

---
 include/util/command.hpp | 22 ++++++++++---
 src/main.cpp             | 70 +++++++++++++++++++++++++++++++---------
 2 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/include/util/command.hpp b/include/util/command.hpp
index 9db8d833..52655581 100644
--- a/include/util/command.hpp
+++ b/include/util/command.hpp
@@ -7,7 +7,7 @@
 
 #include <array>
 
-extern sig_atomic_t is_inserting_pid;
+extern std::mutex reap_mtx;
 extern std::list<pid_t> reap;
 
 namespace waybar::util::command {
@@ -71,6 +71,12 @@ inline FILE* open(const std::string& cmd, int& pid) {
   }
 
   if (!child_pid) {
+    int err;
+    sigset_t mask;
+    sigfillset(&mask);
+    // Reset sigmask
+    err = pthread_sigmask(SIG_UNBLOCK, &mask, nullptr);
+    if (err != 0) spdlog::error("pthread_sigmask in open failed: {}", strerror(err));
     ::close(fd[0]);
     dup2(fd[1], 1);
     setpgid(child_pid, child_pid);
@@ -103,7 +109,7 @@ inline struct res execNoRead(const std::string& cmd) {
 inline int32_t forkExec(const std::string& cmd) {
   if (cmd == "") return -1;
 
-  int32_t pid = fork();
+  pid_t pid = fork();
 
   if (pid < 0) {
     spdlog::error("Unable to exec cmd {}, error {}", cmd.c_str(), strerror(errno));
@@ -112,14 +118,20 @@ inline int32_t forkExec(const std::string& cmd) {
 
   // Child executes the command
   if (!pid) {
+    int err;
+    sigset_t mask;
+    sigfillset(&mask);
+    // Reset sigmask
+    err = pthread_sigmask(SIG_UNBLOCK, &mask, nullptr);
+    if (err != 0) spdlog::error("pthread_sigmask in forkExec failed: {}", strerror(err));
     setpgid(pid, pid);
-    signal(SIGCHLD, SIG_DFL);
     execl("/bin/sh", "sh", "-c", cmd.c_str(), (char*)0);
     exit(0);
   } else {
-    is_inserting_pid = true;
+    reap_mtx.lock();
     reap.push_back(pid);
-    is_inserting_pid = false;
+    reap_mtx.unlock();
+    spdlog::debug("Added child to reap list: {}", pid);
   }
 
   return pid;
diff --git a/src/main.cpp b/src/main.cpp
index 5350ec09..19a8de1e 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -1,30 +1,70 @@
 #include <csignal>
 #include <list>
+#include <mutex>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <spdlog/spdlog.h>
 #include "client.hpp"
 
-sig_atomic_t is_inserting_pid = false;
+std::mutex reap_mtx;
 std::list<pid_t> reap;
 
-static void handler(int sig) {
-  int saved_errno = errno;
-  if (!is_inserting_pid) {
-    for (auto it = reap.begin(); it != reap.end(); ++it) {
-      if (waitpid(*it, nullptr, WNOHANG) == *it) {
-        it = reap.erase(it);
-      }
+void* signalThread(void* args) {
+  int err, signum;
+  sigset_t mask;
+  sigemptyset(&mask);
+  sigaddset(&mask, SIGCHLD);
+
+  while (true) {
+    err = sigwait(&mask, &signum);
+    if (err != 0) {
+      spdlog::error("sigwait failed: {}", strerror(errno));
+      continue;
+    }
+
+    switch (signum) {
+      case SIGCHLD:
+        spdlog::debug("Received SIGCHLD in signalThread");
+        if (!reap.empty()) {
+          reap_mtx.lock();
+          for (auto it = reap.begin(); it != reap.end(); ++it) {
+            if (waitpid(*it, nullptr, WNOHANG) == *it) {
+              spdlog::debug("Reaped child with PID: {}", *it);
+              it = reap.erase(it);
+            }
+          }
+          reap_mtx.unlock();
+        }
+        break;
+      default:
+        spdlog::debug("Received signal with number {}, but not handling",
+                      signum);
+        break;
     }
   }
-  errno = saved_errno;
 }
 
-inline void installSigChldHandler(void) {
-  struct sigaction sa;
-  sigemptyset(&sa.sa_mask);
-  sa.sa_handler = handler;
-  sigaction(SIGCHLD, &sa, nullptr);
+void startSignalThread(void) {
+  int err;
+  sigset_t mask;
+  sigemptyset(&mask);
+  sigaddset(&mask, SIGCHLD);
+
+  // Block SIGCHLD so it can be handled by the signal thread
+  // Any threads created by this one (the main thread) should not
+  // modify their signal mask to unblock SIGCHLD
+  err = pthread_sigmask(SIG_BLOCK, &mask, nullptr);
+  if (err != 0) {
+    spdlog::error("pthread_sigmask failed in startSignalThread: {}", strerror(err));
+    exit(1);
+  }
+
+  pthread_t thread_id;
+  err = pthread_create(&thread_id, nullptr, signalThread, nullptr);
+  if (err != 0) {
+    spdlog::error("pthread_create failed in startSignalThread: {}", strerror(err));
+    exit(1);
+  }
 }
 
 int main(int argc, char* argv[]) {
@@ -43,7 +83,7 @@ int main(int argc, char* argv[]) {
         }
       });
     }
-    installSigChldHandler();
+    startSignalThread();
 
     auto ret = client->main(argc, argv);
     delete client;