Rework the way UI can trigger a client quitting

Add a UserInterface::is_ok method and return false on
SIGHUP/stdin closing/socket dropping

This should be cleaner and more robust than the previous SIGHUP
handling code.

Fixes #1594

src/client.cc

@@ -71,6 +71,11 @@ Client::~Client()
                                               context().selections());
 }
 
+bool Client::is_ui_ok() const
+{
+    return m_ui->is_ok();
+}
+
 bool Client::process_pending_inputs()
 {
     const bool debug_keys = (bool)(context().options()["debug"].get<DebugFlags>() & DebugFlags::Keys);

src/client.hh

@@ -37,6 +37,8 @@ public:
 
     Client(Client&&) = delete;
 
+    bool is_ui_ok() const;
+
     bool process_pending_inputs();
     bool has_pending_inputs() const { return not m_pending_keys.empty(); }
 

src/json_ui.cc

@@ -411,7 +411,10 @@ void JsonUI::parse_requests(EventMode mode)
     {
         ssize_t size = ::read(0, buf, bufsize);
         if (size == -1 or size == 0)
+        {
+            m_stdin_watcher.close_fd();
             break;
+        }
 
         m_requests += StringView{buf, buf + size};
     }

src/json_ui.hh

@@ -19,6 +19,8 @@ public:
     JsonUI(const JsonUI&) = delete;
     JsonUI& operator=(const JsonUI&) = delete;
 
+    bool is_ok() const override { return m_stdin_watcher.fd() != -1; }
+
     void draw(const DisplayBuffer& display_buffer,
               const Face& default_face,
               const Face& buffer_padding) override;

src/main.cc

@@ -378,7 +378,6 @@ void register_options()
 
 static Client* local_client = nullptr;
 static int local_client_exit = 0;
-static sig_atomic_t sighup_raised = 0;
 static UserInterface* local_ui = nullptr;
 static bool convert_to_client_pending = false;
 
@@ -403,6 +402,7 @@ std::unique_ptr<UserInterface> make_ui(UIType ui_type)
     struct DummyUI : UserInterface
     {
         DummyUI() { set_signal_handler(SIGINT, SIG_DFL); }
+        bool is_ok() const override { return true; }
         void menu_show(ConstArrayView<DisplayLine>, DisplayCoord,
                        Face, Face, MenuStyle) override {}
         void menu_select(int) override {}
@@ -453,10 +453,6 @@ std::unique_ptr<UserInterface> create_local_ui(UIType ui_type)
         {
             kak_assert(not local_ui);
             local_ui = this;
-            m_old_sighup = set_signal_handler(SIGHUP, [](int) {
-                static_cast<LocalUI*>(local_ui)->on_sighup();
-                sighup_raised = 1;
-            });
 
             m_old_sigtstp = set_signal_handler(SIGTSTP, [](int) {
                 if (ClientManager::instance().count() == 1 and
@@ -482,7 +478,6 @@ std::unique_ptr<UserInterface> create_local_ui(UIType ui_type)
 
         ~LocalUI() override
         {
-            set_signal_handler(SIGHUP, m_old_sighup);
             set_signal_handler(SIGTSTP, m_old_sigtstp);
             local_client = nullptr;
             local_ui = nullptr;
@@ -499,7 +494,6 @@ std::unique_ptr<UserInterface> create_local_ui(UIType ui_type)
 
     private:
         using SigHandler = void (*)(int);
-        SigHandler m_old_sighup;
         SigHandler m_old_sigtstp;
     };
 
@@ -692,12 +686,11 @@ int run_server(StringView session, StringView server_init,
             client_manager.clear_window_trash();
             buffer_manager.clear_buffer_trash();
 
-            if (sighup_raised and local_client)
+            if (local_client and not local_client->is_ui_ok())
             {
-                ClientManager::instance().remove_client(*local_client, false, 0);
+                ClientManager::instance().remove_client(*local_client, false, -1);
                 if (not client_manager.empty() and fork_server_to_background())
                     return 0;
-                sighup_raised = 0;
             }
             else if (convert_to_client_pending)
             {

src/ncurses_ui.cc

@@ -219,10 +219,12 @@ void NCursesUI::set_face(NCursesWin* window, Face face, const Face& default_face
 }
 
 static sig_atomic_t resize_pending = 0;
+static sig_atomic_t sighup_raised = 0;
 
-void on_term_resize(int)
+template<sig_atomic_t* signal_flag>
+static void signal_handler(int)
 {
-    resize_pending = 1;
+    *signal_flag = 1;
     EventManager::instance().force_signal(0);
 }
 
@@ -271,7 +273,8 @@ NCursesUI::NCursesUI()
 
     enable_mouse(true);
 
-    set_signal_handler(SIGWINCH, on_term_resize);
+    set_signal_handler(SIGWINCH, &signal_handler<&resize_pending>);
+    set_signal_handler(SIGHUP, &signal_handler<&sighup_raised>);
 
     check_resize(true);
 
@@ -516,18 +519,15 @@ void NCursesUI::check_resize(bool force)
     werase(curscr);
 }
 
-void NCursesUI::on_sighup()
-{
-    set_signal_handler(SIGWINCH, SIG_DFL);
-    set_signal_handler(SIGCONT, SIG_DFL);
-
-    m_window = nullptr;
-}
-
 Optional<Key> NCursesUI::get_next_key()
 {
-    if (not m_window)
+    if (sighup_raised)
+    {
+        set_signal_handler(SIGWINCH, SIG_DFL);
+        set_signal_handler(SIGCONT, SIG_DFL);
+        m_window = nullptr;
         return {};
+    }
 
     check_resize();
 

src/ncurses_ui.hh

@@ -24,6 +24,8 @@ public:
     NCursesUI(const NCursesUI&) = delete;
     NCursesUI& operator=(const NCursesUI&) = delete;
 
+    bool is_ok() const override { return m_window != nullptr; }
+
     void draw(const DisplayBuffer& display_buffer,
               const Face& default_face,
               const Face& padding_face) override;
@@ -59,9 +61,6 @@ public:
         DisplayCoord size;
     };
 
-protected:
-    void on_sighup();
-
 private:
     void check_resize(bool force = false);
     void redraw();

src/normal.cc

@@ -1982,7 +1982,7 @@ template<typename Type, Direction direction, SelectMode mode = SelectMode::Repla
 void move_cursor(Context& context, NormalParams params)
 {
     kak_assert(mode == SelectMode::Replace or mode == SelectMode::Extend);
-    const Type offset(direction * std::max(params.count,1));
+    const Type offset{direction * std::max(params.count,1)};
     const ColumnCount tabstop = context.options()["tabstop"].get<int>();
     auto& selections = context.selections();
     for (auto& sel : selections)

src/remote.cc

@@ -315,6 +315,7 @@ public:
     RemoteUI(int socket, DisplayCoord dimensions);
     ~RemoteUI() override;
 
+    bool is_ok() const override { return m_socket_watcher.fd() != -1; }
     void menu_show(ConstArrayView<DisplayLine> choices,
                    DisplayCoord anchor, Face fg, Face bg,
                    MenuStyle style) override;
@@ -345,9 +346,6 @@ public:
 
     void set_ui_options(const Options& options) override;
 
-    void set_client(Client* client) { m_client = client; }
-    Client* client() const { return m_client.get(); }
-
     void exit(int status);
 
 private:
@@ -356,8 +354,6 @@ private:
     DisplayCoord  m_dimensions;
     OnKeyCallback m_on_key;
     RemoteBuffer  m_send_buffer;
-
-    SafePtr<Client> m_client;
 };
 
 static bool send_data(int fd, RemoteBuffer& buffer)
@@ -390,8 +386,8 @@ RemoteUI::RemoteUI(int socket, DisplayCoord dimensions)
 
                    if (m_reader.type() != MessageType::Key)
                    {
-                      ClientManager::instance().remove_client(*m_client, false, -1);
+                       m_socket_watcher.close_fd();
-                      return;
+                       return;
                    }
 
                    auto key = m_reader.read<Key>();
@@ -404,7 +400,7 @@ RemoteUI::RemoteUI(int socket, DisplayCoord dimensions)
           catch (const disconnected& err)
           {
               write_to_debug_buffer(format("Error while transfering remote messages: {}", err.what()));
-              ClientManager::instance().remove_client(*m_client, false, -1);
+              m_socket_watcher.close_fd();
           }
       }),
       m_dimensions(dimensions)
@@ -725,11 +721,10 @@ private:
                 auto dimensions = m_reader.read<DisplayCoord>();
                 auto env_vars = m_reader.read_hash_map<String, String, MemoryDomain::EnvVars>();
                 auto* ui = new RemoteUI{sock, dimensions};
-                if (auto* client = ClientManager::instance().create_client(
+                ClientManager::instance().create_client(
-                                       std::unique_ptr<UserInterface>(ui), pid, std::move(name),
+                    std::unique_ptr<UserInterface>(ui), pid, std::move(name),
-                                       std::move(env_vars), init_cmds, init_coord,
+                    std::move(env_vars), init_cmds, init_coord,
-                                       [ui](int status) { ui->exit(status); }))
+                    [ui](int status) { ui->exit(status); });
-                    ui->set_client(client);
 
                 Server::instance().remove_accepter(this);
                 break;

src/user_interface.hh

@@ -47,6 +47,8 @@ class UserInterface
 public:
     virtual ~UserInterface() = default;
 
+    virtual bool is_ok() const = 0;
+
     virtual void menu_show(ConstArrayView<DisplayLine> choices,
                            DisplayCoord anchor, Face fg, Face bg,
                            MenuStyle style) = 0;