EventManager: avoid erasing an event handler while it may be in use

src/client_manager.cc

@@ -26,8 +26,6 @@ void ClientManager::create_client(std::unique_ptr<UserInterface>&& ui,
         catch (Kakoune::client_removed&)
         {
             ClientManager::instance().remove_client_by_context(*context);
-            // do not forget, after this line, the current closure
-            // is dead, do not access captured data.
             EventManager::instance().unwatch(fd);
             close(fd);
         }

src/commands.cc

@@ -260,8 +260,6 @@ Buffer* open_fifo(const String& name , const String& filename, Context& context)
         {
             assert(buffer->flags() & Buffer::Flags::Fifo);
             buffer->flags() &= ~Buffer::Flags::Fifo;
-            // after that this closure will be dead, do not access
-            // any captured data.
             EventManager::instance().unwatch(fd);
             close(fd);
         }

src/event_manager.cc

@@ -5,8 +5,6 @@
 namespace Kakoune
 {
 
-EventManager::EventManager() { m_forced.reserve(4); }
-
 void EventManager::watch(int fd, EventHandler handler)
 {
     auto event = std::find_if(m_events.begin(), m_events.end(),
@@ -20,27 +18,32 @@ void EventManager::watch(int fd, EventHandler handler)
 
 void EventManager::unwatch(int fd)
 {
-    for (size_t i = 0; i < m_events.size(); ++i)
-    {
-        if (m_events[i].fd == fd)
-        {
-            m_events.erase(m_events.begin() + i);
-            m_handlers.erase(fd);
-            return;
-        }
-    }
+    // do not unwatch now, do that at the end of handle_next_events,
+    // so that if unwatch(fd) is called from fd event handler,
+    // it is not deleted now.
+    m_unwatched.push_back(fd);
 }
 
 void EventManager::handle_next_events()
 {
     const int timeout_ms = 100;
-    int res = poll(m_events.data(), m_events.size(), timeout_ms);
-    for (size_t i = 0; i < m_events.size(); ++i)
+    poll(m_events.data(), m_events.size(), timeout_ms);
+    for (auto& event : m_events)
     {
-        if ((res > 0 and m_events[i].revents) or
-            contains(m_forced, m_events[i].fd))
-            m_handlers[m_events[i].fd](m_events[i].fd);
+        const int fd = event.fd;
+        if ((event.revents or contains(m_forced, fd)) and not contains(m_unwatched, fd))
+            m_handlers[fd](fd);
     }
+
+    // remove unwatched.
+    for (auto fd : m_unwatched)
+    {
+        auto it = std::find_if(m_events.begin(), m_events.end(),
+                               [fd](pollfd& p) { return p.fd == fd; });
+        m_events.erase(it);
+        m_handlers.erase(fd);
+    }
+    m_unwatched.clear();
     m_forced.clear();
 }
 

src/event_manager.hh

@@ -11,22 +11,34 @@ namespace Kakoune
 
 using EventHandler = std::function<void (int fd)>;
 
+// The EventManager provides an interface to file descriptor
+// based event handling.
+//
+// The program main loop should call handle_next_events()
+// until it's time to quit.
 class EventManager : public Singleton<EventManager>
 {
 public:
-    EventManager();
-
+    // Watch the given file descriptor, when data becomes
+    // ready, handler will be called with fd as parameter.
+    // It is an error to register multiple handlers on the
+    // same file descriptor.
     void watch(int fd, EventHandler handler);
+
+    // stop watching fd
     void unwatch(int fd);
 
     void handle_next_events();
 
+    // force the handler associated with fd to be executed
+    // on next handle_next_events call.
     void force_signal(int fd);
 
 private:
-    std::vector<pollfd> m_events;
+    std::vector<pollfd>                   m_events;
     std::unordered_map<int, EventHandler> m_handlers;
-    std::vector<int>    m_forced;
+    std::vector<int>                      m_forced;
+    std::vector<int>                      m_unwatched;
 };
 
 }