From e5852f68221c0193df4444ea71f23565ec84c8c9 Mon Sep 17 00:00:00 2001
From: Maxime Coste <frrrwww@gmail.com>
Date: Tue, 9 Jun 2015 20:28:24 +0100
Subject: [PATCH] Fix race condition ShellManager::eval with SIGCHLD

---
 src/event_manager.cc | 16 ++++++++--------
 src/event_manager.hh |  3 ++-
 src/main.cc          |  1 +
 src/shell_manager.cc | 15 +++++++++++++--
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/event_manager.cc b/src/event_manager.cc
index 92d87245..e1a4accd 100644
--- a/src/event_manager.cc
+++ b/src/event_manager.cc
@@ -68,7 +68,7 @@ EventManager::~EventManager()
     kak_assert(m_timers.empty());
 }
 
-void EventManager::handle_next_events(EventMode mode)
+void EventManager::handle_next_events(EventMode mode, sigset_t* sigmask)
 {
     int max_fd = 0;
     fd_set rfds;
@@ -84,7 +84,7 @@ void EventManager::handle_next_events(EventMode mode)
     }
 
     bool with_timeout = false;
-    timeval tv{};
+    timespec ts{};
     if (not m_timers.empty())
     {
         auto next_date = (*std::min_element(
@@ -95,14 +95,14 @@ void EventManager::handle_next_events(EventMode mode)
         if (next_date != TimePoint::max())
         {
             with_timeout = true;
-            using namespace std::chrono; using us = std::chrono::microseconds;
-            auto usecs = std::max(us(0), duration_cast<us>(next_date - Clock::now()));
-            auto secs = duration_cast<seconds>(usecs);
-            tv = timeval{ (time_t)secs.count(), (suseconds_t)(usecs - secs).count() };
+            using namespace std::chrono; using ns = std::chrono::nanoseconds;
+            auto nsecs = std::max(ns(0), duration_cast<ns>(next_date - Clock::now()));
+            auto secs = duration_cast<seconds>(nsecs);
+            ts = timespec{ (time_t)secs.count(), (long)(nsecs - secs).count() };
         }
     }
-    int res = select(max_fd + 1, &rfds, nullptr, nullptr,
-                     with_timeout ? &tv : nullptr);
+    int res = pselect(max_fd + 1, &rfds, nullptr, nullptr,
+                      with_timeout ? &ts : nullptr, sigmask);
 
     // copy forced fds *after* select, so that signal handlers can write to
     // m_forced_fd, interupt select, and directly be serviced.
diff --git a/src/event_manager.hh b/src/event_manager.hh
index 070fbff4..2c030b7e 100644
--- a/src/event_manager.hh
+++ b/src/event_manager.hh
@@ -9,6 +9,7 @@
 #include <functional>
 
 #include <sys/select.h>
+#include <signal.h>
 
 namespace Kakoune
 {
@@ -75,7 +76,7 @@ public:
     EventManager();
     ~EventManager();
 
-    void handle_next_events(EventMode mode);
+    void handle_next_events(EventMode mode, sigset_t* sigmask = nullptr);
 
     // force the watchers associated with fd to be executed
     // on next handle_next_events call.
diff --git a/src/main.cc b/src/main.cc
index a0bbf0f8..4eacbcb9 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -544,6 +544,7 @@ int main(int argc, char* argv[])
     signal(SIGQUIT, signal_handler);
     signal(SIGTERM, signal_handler);
     signal(SIGPIPE, SIG_IGN);
+    signal(SIGCHLD, [](int){});
 
     Vector<String> params;
     for (size_t i = 1; i < argc; ++i)
diff --git a/src/shell_manager.cc b/src/shell_manager.cc
index 3a64eef2..37221204 100644
--- a/src/shell_manager.cc
+++ b/src/shell_manager.cc
@@ -45,7 +45,6 @@ std::pair<String, int> ShellManager::eval(
 
         String child_stdout, child_stderr;
         int status = 0;
-        bool terminated = false;
         {
             auto pipe_reader = [](String& output) {
                 return [&output](FDWatcher& watcher, EventMode) {
@@ -60,18 +59,30 @@ std::pair<String, int> ShellManager::eval(
             FDWatcher stdout_watcher{read_pipe[0], pipe_reader(child_stdout)};
             FDWatcher stderr_watcher{error_pipe[0], pipe_reader(child_stderr)};
 
+            // block SIGCHLD to make sure we wont receive it before
+            // our call to pselect, that will end up blocking indefinitly.
+            sigset_t mask, orig_mask;
+            sigemptyset(&mask);
+            sigaddset(&mask, SIGCHLD);
+            sigprocmask(SIG_BLOCK, &mask, &orig_mask);
+
+            // check for termination now that SIGCHLD is blocked
+            bool terminated = waitpid(pid, &status, WNOHANG);
+
             while (not terminated or
                    ((flags & Flags::WaitForStdout) and
                     (not stdout_watcher.closed() or
                      not stderr_watcher.closed())))
             {
-                EventManager::instance().handle_next_events(EventMode::Urgent);
+                EventManager::instance().handle_next_events(EventMode::Urgent, &orig_mask);
                 if (not terminated)
                     terminated = waitpid(pid, &status, WNOHANG);
             }
 
             stdout_watcher.close_fd();
             stderr_watcher.close_fd();
+
+            sigprocmask(SIG_SETMASK, &orig_mask, nullptr);
         }
 
         if (not child_stderr.empty())