From 0517a19e6d6911d8d516a7a554aad829bf78a431 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Wed, 3 Dec 2014 13:56:02 +0000 Subject: [PATCH] Use a select based event handling and fix deadlock --- src/buffer_utils.cc | 2 +- src/event_manager.cc | 46 +++++++++++++++++++++++++++++--------------- src/event_manager.hh | 7 ++++++- src/remote.cc | 4 ++-- src/shell_manager.cc | 21 +++++++------------- 5 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index 33b07b57..10c04aa7 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -95,7 +95,7 @@ Buffer* create_fifo_buffer(String name, int fd, bool scroll) auto watcher_deleter = [buffer](FDWatcher* watcher) { kak_assert(buffer->flags() & Buffer::Flags::Fifo); - close(watcher->fd()); + watcher->close_fd(); buffer->run_hook_in_own_context("BufCloseFifo", ""); buffer->flags() &= ~Buffer::Flags::Fifo; watcher->~FDWatcher(); diff --git a/src/event_manager.cc b/src/event_manager.cc index 1c89ee50..68c7c87b 100644 --- a/src/event_manager.cc +++ b/src/event_manager.cc @@ -1,6 +1,6 @@ #include "event_manager.hh" -#include +#include namespace Kakoune { @@ -21,6 +21,12 @@ void FDWatcher::run(EventMode mode) m_callback(*this, mode); } +void FDWatcher::close_fd() +{ + close(m_fd); + m_fd = -1; +} + Timer::Timer(TimePoint date, Callback callback, EventMode mode) : m_date{date}, m_callback{std::move(callback)}, m_mode(mode) { @@ -47,7 +53,7 @@ void Timer::run(EventMode mode) EventManager::EventManager() { - m_forced_fd.reserve(4); + FD_ZERO(&m_forced_fd); } EventManager::~EventManager() @@ -58,10 +64,18 @@ EventManager::~EventManager() void EventManager::handle_next_events(EventMode mode) { - std::vector events; - events.reserve(m_fd_watchers.size()); + int max_fd = 0; + fd_set rfds; + FD_ZERO(&rfds); for (auto& watcher : m_fd_watchers) - events.emplace_back(pollfd{ watcher->fd(), POLLIN | POLLPRI, 0 }); + { + const int fd = watcher->fd(); + if (fd != -1) + { + max_fd = std::max(fd, max_fd); + FD_SET(fd, &rfds); + } + } TimePoint next_timer = TimePoint::max(); for (auto& timer : m_timers) @@ -70,21 +84,23 @@ void EventManager::handle_next_events(EventMode mode) next_timer = timer->next_date(); } using namespace std::chrono; - auto timeout = duration_cast(next_timer - Clock::now()).count(); - poll(events.data(), events.size(), timeout < INT_MAX ? (int)timeout : INT_MAX); + auto timeout = duration_cast(next_timer - Clock::now()).count(); - // gather forced fds *after* poll, so that signal handlers can write to + constexpr auto us = 1000000000ll; + timeval tv{ (long)(timeout / us), (long)(timeout % us) }; + int res = select(max_fd + 1, &rfds, nullptr, nullptr, &tv); + + // copy forced fds *after* poll, so that signal handlers can write to // m_forced_fd, interupt poll, and directly be serviced. - std::vector forced; - std::swap(forced, m_forced_fd); + fd_set forced = m_forced_fd; + FD_ZERO(&m_forced_fd); - for (auto& event : events) + for (int fd = 0; fd < max_fd + 1; ++fd) { - const int fd = event.fd; - if (event.revents or contains(forced, fd)) + if ((res > 0 and FD_ISSET(fd, &rfds)) or FD_ISSET(fd, &forced)) { auto it = find_if(m_fd_watchers, - [fd](FDWatcher* w) { return w->fd() == fd; }); + [fd](const FDWatcher* w){return w->fd() == fd; }); if (it != m_fd_watchers.end()) (*it)->run(mode); } @@ -100,7 +116,7 @@ void EventManager::handle_next_events(EventMode mode) void EventManager::force_signal(int fd) { - m_forced_fd.push_back(fd); + FD_SET(fd, &m_forced_fd); } } diff --git a/src/event_manager.hh b/src/event_manager.hh index 35c83e01..ed7a37c7 100644 --- a/src/event_manager.hh +++ b/src/event_manager.hh @@ -7,6 +7,8 @@ #include #include +#include + namespace Kakoune { @@ -28,6 +30,9 @@ public: int fd() const { return m_fd; } void run(EventMode mode); + + void close_fd(); + bool closed() const { return m_fd == -1; } private: int m_fd; @@ -80,7 +85,7 @@ private: friend class Timer; std::unordered_set m_fd_watchers; std::unordered_set m_timers; - std::vector m_forced_fd; + fd_set m_forced_fd; TimePoint m_last; }; diff --git a/src/remote.cc b/src/remote.cc index 909f2c09..3f8cbde2 100644 --- a/src/remote.cc +++ b/src/remote.cc @@ -295,7 +295,7 @@ RemoteUI::~RemoteUI() { write_debug("remote client disconnected: " + to_string(m_socket_watcher.fd())); - close(m_socket_watcher.fd()); + m_socket_watcher.close_fd(); } void RemoteUI::menu_show(memoryview choices, @@ -646,7 +646,7 @@ Server::Server(String session_name) void Server::close_session() { unlink(("/tmp/kak-" + m_session).c_str()); - close(m_listener->fd()); + m_listener->close_fd(); m_listener.reset(); } diff --git a/src/shell_manager.cc b/src/shell_manager.cc index 0391dc4f..f037371c 100644 --- a/src/shell_manager.cc +++ b/src/shell_manager.cc @@ -66,27 +66,20 @@ String ShellManager::pipe(StringView input, String error; { - auto pipe_reader = [](String& output, bool& closed) { - return [&output, &closed](FDWatcher& watcher, EventMode) { - if (closed) - return; - const int fd = watcher.fd(); + auto pipe_reader = [](String& output) { + return [&output](FDWatcher& watcher, EventMode) { char buffer[1024]; - size_t size = read(fd, buffer, 1024); + size_t size = read(watcher.fd(), buffer, 1024); if (size <= 0) - { - close(fd); - closed = true; - } + watcher.close_fd(); output += String(buffer, buffer+size); }; }; - bool stdout_closed = false, stderr_closed = false; - FDWatcher stdout_watcher{read_pipe[0], pipe_reader(output, stdout_closed)}; - FDWatcher stderr_watcher{error_pipe[0], pipe_reader(error, stderr_closed)}; + FDWatcher stdout_watcher{read_pipe[0], pipe_reader(output)}; + FDWatcher stderr_watcher{error_pipe[0], pipe_reader(error)}; - while (not stdout_closed or not stderr_closed) + while (not stdout_watcher.closed() and not stderr_watcher.closed()) EventManager::instance().handle_next_events(EventMode::Urgent); }