Merge pull request #778 from excellentname/handle-sigchld

Handle SIGCHLD for exec/forkExec

include/util/command.hpp

@@ -7,6 +7,9 @@
 
 #include <array>
 
+extern std::mutex reap_mtx;
+extern std::list<pid_t> reap;
+
 namespace waybar::util::command {
 
 struct res {
@@ -32,10 +35,11 @@ inline std::string read(FILE* fp) {
 
 inline int close(FILE* fp, pid_t pid) {
   int stat = -1;
+  pid_t ret;
 
   fclose(fp);
   do {
-    waitpid(pid, &stat, WCONTINUED | WUNTRACED);
+    ret = waitpid(pid, &stat, WCONTINUED | WUNTRACED);
 
     if (WIFEXITED(stat)) {
       spdlog::debug("Cmd exited with code {}", WEXITSTATUS(stat));
@@ -45,6 +49,8 @@ inline int close(FILE* fp, pid_t pid) {
       spdlog::debug("Cmd stopped by {}", WSTOPSIG(stat));
     } else if (WIFCONTINUED(stat)) {
       spdlog::debug("Cmd continued");
+    } else if (ret == -1) {
+      spdlog::debug("waitpid failed: {}", strerror(errno));
     } else {
       break;
     }
@@ -65,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);
@@ -97,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));
@@ -106,12 +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 {
-    signal(SIGCHLD, SIG_IGN);
+    reap_mtx.lock();
+    reap.push_back(pid);
+    reap_mtx.unlock();
+    spdlog::debug("Added child to reap list: {}", pid);
   }
 
   return pid;

src/main.cpp

@@ -1,7 +1,72 @@
 #include <csignal>
+#include <list>
+#include <mutex>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include <spdlog/spdlog.h>
 #include "client.hpp"
 
+std::mutex reap_mtx;
+std::list<pid_t> reap;
+
+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;
+    }
+  }
+}
+
+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[]) {
   try {
     auto client = waybar::Client::inst();
@@ -18,6 +83,7 @@ int main(int argc, char* argv[]) {
         }
       });
     }
+    startSignalThread();
 
     auto ret = client->main(argc, argv);
     delete client;