From 178d2d3cd354bc93d82e5cd6269976b2077a66c3 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Sun, 29 Apr 2018 22:27:28 +1000 Subject: [PATCH] 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 | 5 +++++ src/client.hh | 2 ++ src/json_ui.cc | 3 +++ src/json_ui.hh | 2 ++ src/main.cc | 13 +++---------- src/ncurses_ui.cc | 24 ++++++++++++------------ src/ncurses_ui.hh | 5 ++--- src/normal.cc | 2 +- src/remote.cc | 21 ++++++++------------- src/user_interface.hh | 2 ++ 10 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/client.cc b/src/client.cc index 22836a25..8a0dfadb 100644 --- a/src/client.cc +++ b/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::Keys); diff --git a/src/client.hh b/src/client.hh index dadc0320..1eb1e5d3 100644 --- a/src/client.hh +++ b/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(); } diff --git a/src/json_ui.cc b/src/json_ui.cc index 55b21e81..6381cd74 100644 --- a/src/json_ui.cc +++ b/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}; } diff --git a/src/json_ui.hh b/src/json_ui.hh index ad887561..5e53ab42 100644 --- a/src/json_ui.hh +++ b/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; diff --git a/src/main.cc b/src/main.cc index 0879d619..196441c8 100644 --- a/src/main.cc +++ b/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 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, DisplayCoord, Face, Face, MenuStyle) override {} void menu_select(int) override {} @@ -453,10 +453,6 @@ std::unique_ptr create_local_ui(UIType ui_type) { kak_assert(not local_ui); local_ui = this; - m_old_sighup = set_signal_handler(SIGHUP, [](int) { - static_cast(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 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 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) { diff --git a/src/ncurses_ui.cc b/src/ncurses_ui.cc index a8a2cc1f..0bb1278d 100644 --- a/src/ncurses_ui.cc +++ b/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 +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 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(); diff --git a/src/ncurses_ui.hh b/src/ncurses_ui.hh index d2449d59..3b4e9e32 100644 --- a/src/ncurses_ui.hh +++ b/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(); diff --git a/src/normal.cc b/src/normal.cc index e7449b4e..92c9d863 100644 --- a/src/normal.cc +++ b/src/normal.cc @@ -1982,7 +1982,7 @@ template(); auto& selections = context.selections(); for (auto& sel : selections) diff --git a/src/remote.cc b/src/remote.cc index d3879f28..6a717805 100644 --- a/src/remote.cc +++ b/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 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 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); - return; + m_socket_watcher.close_fd(); + return; } auto key = m_reader.read(); @@ -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(); auto env_vars = m_reader.read_hash_map(); auto* ui = new RemoteUI{sock, dimensions}; - if (auto* client = ClientManager::instance().create_client( - std::unique_ptr(ui), pid, std::move(name), - std::move(env_vars), init_cmds, init_coord, - [ui](int status) { ui->exit(status); })) - ui->set_client(client); + ClientManager::instance().create_client( + std::unique_ptr(ui), pid, std::move(name), + std::move(env_vars), init_cmds, init_coord, + [ui](int status) { ui->exit(status); }); Server::instance().remove_accepter(this); break; diff --git a/src/user_interface.hh b/src/user_interface.hh index d8139336..c3313890 100644 --- a/src/user_interface.hh +++ b/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 choices, DisplayCoord anchor, Face fg, Face bg, MenuStyle style) = 0;