Fix race condition ShellManager::eval with SIGCHLD

This commit is contained in:
Maxime Coste 2015-06-09 20:28:24 +01:00
parent b4329dd643
commit e5852f6822
4 changed files with 24 additions and 11 deletions

View File

@ -68,7 +68,7 @@ EventManager::~EventManager()
kak_assert(m_timers.empty()); 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; int max_fd = 0;
fd_set rfds; fd_set rfds;
@ -84,7 +84,7 @@ void EventManager::handle_next_events(EventMode mode)
} }
bool with_timeout = false; bool with_timeout = false;
timeval tv{}; timespec ts{};
if (not m_timers.empty()) if (not m_timers.empty())
{ {
auto next_date = (*std::min_element( auto next_date = (*std::min_element(
@ -95,14 +95,14 @@ void EventManager::handle_next_events(EventMode mode)
if (next_date != TimePoint::max()) if (next_date != TimePoint::max())
{ {
with_timeout = true; with_timeout = true;
using namespace std::chrono; using us = std::chrono::microseconds; using namespace std::chrono; using ns = std::chrono::nanoseconds;
auto usecs = std::max(us(0), duration_cast<us>(next_date - Clock::now())); auto nsecs = std::max(ns(0), duration_cast<ns>(next_date - Clock::now()));
auto secs = duration_cast<seconds>(usecs); auto secs = duration_cast<seconds>(nsecs);
tv = timeval{ (time_t)secs.count(), (suseconds_t)(usecs - secs).count() }; ts = timespec{ (time_t)secs.count(), (long)(nsecs - secs).count() };
} }
} }
int res = select(max_fd + 1, &rfds, nullptr, nullptr, int res = pselect(max_fd + 1, &rfds, nullptr, nullptr,
with_timeout ? &tv : nullptr); with_timeout ? &ts : nullptr, sigmask);
// copy forced fds *after* select, so that signal handlers can write to // copy forced fds *after* select, so that signal handlers can write to
// m_forced_fd, interupt select, and directly be serviced. // m_forced_fd, interupt select, and directly be serviced.

View File

@ -9,6 +9,7 @@
#include <functional> #include <functional>
#include <sys/select.h> #include <sys/select.h>
#include <signal.h>
namespace Kakoune namespace Kakoune
{ {
@ -75,7 +76,7 @@ public:
EventManager(); EventManager();
~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 // force the watchers associated with fd to be executed
// on next handle_next_events call. // on next handle_next_events call.

View File

@ -544,6 +544,7 @@ int main(int argc, char* argv[])
signal(SIGQUIT, signal_handler); signal(SIGQUIT, signal_handler);
signal(SIGTERM, signal_handler); signal(SIGTERM, signal_handler);
signal(SIGPIPE, SIG_IGN); signal(SIGPIPE, SIG_IGN);
signal(SIGCHLD, [](int){});
Vector<String> params; Vector<String> params;
for (size_t i = 1; i < argc; ++i) for (size_t i = 1; i < argc; ++i)

View File

@ -45,7 +45,6 @@ std::pair<String, int> ShellManager::eval(
String child_stdout, child_stderr; String child_stdout, child_stderr;
int status = 0; int status = 0;
bool terminated = false;
{ {
auto pipe_reader = [](String& output) { auto pipe_reader = [](String& output) {
return [&output](FDWatcher& watcher, EventMode) { 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 stdout_watcher{read_pipe[0], pipe_reader(child_stdout)};
FDWatcher stderr_watcher{error_pipe[0], pipe_reader(child_stderr)}; 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 while (not terminated or
((flags & Flags::WaitForStdout) and ((flags & Flags::WaitForStdout) and
(not stdout_watcher.closed() or (not stdout_watcher.closed() or
not stderr_watcher.closed()))) not stderr_watcher.closed())))
{ {
EventManager::instance().handle_next_events(EventMode::Urgent); EventManager::instance().handle_next_events(EventMode::Urgent, &orig_mask);
if (not terminated) if (not terminated)
terminated = waitpid(pid, &status, WNOHANG); terminated = waitpid(pid, &status, WNOHANG);
} }
stdout_watcher.close_fd(); stdout_watcher.close_fd();
stderr_watcher.close_fd(); stderr_watcher.close_fd();
sigprocmask(SIG_SETMASK, &orig_mask, nullptr);
} }
if (not child_stderr.empty()) if (not child_stderr.empty())