From 985464bee52b687ab531d8ae2562465f8e7fcc92 Mon Sep 17 00:00:00 2001
From: Maxime Coste <frrrwww@gmail.com>
Date: Tue, 20 Nov 2012 19:48:27 +0100
Subject: [PATCH] Fix FIFO double deregistering issue when closing the buffer
 after EOF

When a fifo was closed, the fifo event handler would close the fd and
unregister it from the event handler, however the hook on BufClose
did that as well without checking if the fd was still refering to the
fifo.

Now we use a Buffer flag Fifo to tag the buffer as still linked to
a fifo so that the BufClose hook do not close and unregister a second
time
---
 src/client_manager.cc |  2 ++
 src/commands.cc       | 42 ++++++++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/src/client_manager.cc b/src/client_manager.cc
index 5dadc45f..3490ebcb 100644
--- a/src/client_manager.cc
+++ b/src/client_manager.cc
@@ -26,6 +26,8 @@ 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);
         }
diff --git a/src/commands.cc b/src/commands.cc
index af2f8b50..ef36b275 100644
--- a/src/commands.cc
+++ b/src/commands.cc
@@ -228,29 +228,39 @@ Buffer* open_or_create(const String& filename, Context& context)
 
 Buffer* open_fifo(const String& name , const String& filename, Context& context)
 {
-    int fd = open(filename.c_str(), O_RDONLY);
+    int fd = open(filename.c_str(), O_RDONLY | O_CLOEXEC);
     if (fd < 0)
        throw runtime_error("unable to open " + filename);
     Buffer* buffer = new Buffer(name, Buffer::Flags::Fifo);
 
-    buffer->hook_manager().add_hook(
-        "BufClose", [=](const String&, const Context&)
-        { EventManager::instance().unwatch(fd); close(fd); }
-    );
+    buffer->hook_manager().add_hook("BufClose",
+        [fd, buffer](const String&, const Context&) {
+            // Check if fifo is still alive, else fd may
+            // refer to another file/socket
+            if (buffer->flags() & Buffer::Flags::Fifo)
+            {
+                EventManager::instance().unwatch(fd);
+                close(fd);
+            }
+        });
 
     EventManager::instance().watch(fd, [buffer](int fd) {
-         char data[4096];
-         ssize_t count = read(fd, data, 4096);
-         buffer->insert(buffer->end()-1,
-                        count > 0 ? String(data, data+count)
+        char data[4096];
+        ssize_t count = read(fd, data, 4096);
+        buffer->insert(buffer->end()-1,
+                       count > 0 ? String(data, data+count)
                                   : "*** kak: fifo closed ***\n");
-         buffer->reset_undo_data();
-         ClientManager::instance().redraw_clients();
-         if (count <= 0)
-         {
-             close(fd);
-             EventManager::instance().unwatch(fd);
-         }
+        buffer->reset_undo_data();
+        ClientManager::instance().redraw_clients();
+        if (count <= 0)
+        {
+            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);
+        }
     });
 
     return buffer;