Rework client quitting and handling of remote errors

Client quitting no longer immediately unwinds, client is just pushed
for deletion until we get back to the main loop, similarly to what
happens for buffer and window deletion.
This commit is contained in:
Maxime Coste 2016-09-04 17:54:07 +01:00
parent 563497ade7
commit 4fc20b8d7d
10 changed files with 107 additions and 100 deletions

View File

@ -74,34 +74,27 @@ void Client::handle_available_input(EventMode mode)
try try
{ {
try while (Optional<Key> key = get_next_key(mode))
{ {
while (Optional<Key> key = get_next_key(mode)) if (*key == ctrl('c'))
killpg(getpgrp(), SIGINT);
else if (*key == Key::FocusIn)
context().hooks().run_hook("FocusIn", context().name(), context());
else if (*key == Key::FocusOut)
context().hooks().run_hook("FocusOut", context().name(), context());
else if (key->modifiers == Key::Modifiers::Resize)
{ {
if (*key == ctrl('c')) m_window->set_dimensions(m_ui->dimensions());
killpg(getpgrp(), SIGINT); force_redraw();
else if (*key == Key::FocusIn)
context().hooks().run_hook("FocusIn", context().name(), context());
else if (*key == Key::FocusOut)
context().hooks().run_hook("FocusOut", context().name(), context());
else if (key->modifiers == Key::Modifiers::Resize)
{
m_window->set_dimensions(m_ui->dimensions());
force_redraw();
}
else
m_input_handler.handle_key(*key);
} }
} else
catch (Kakoune::runtime_error& error) m_input_handler.handle_key(*key);
{
context().print_status({ error.what().str(), get_face("Error") });
context().hooks().run_hook("RuntimeError", error.what(), context());
} }
} }
catch (Kakoune::client_removed& removed) catch (Kakoune::runtime_error& error)
{ {
ClientManager::instance().remove_client(*this, removed.graceful); context().print_status({ error.what().str(), get_face("Error") });
context().hooks().run_hook("RuntimeError", error.what(), context());
} }
} }

View File

@ -23,6 +23,7 @@ void ClientManager::clear()
// So that clients destructor find the client manager empty // So that clients destructor find the client manager empty
// so that local UI does not fork. // so that local UI does not fork.
ClientList clients = std::move(m_clients); ClientList clients = std::move(m_clients);
m_client_trash.clear();
} }
String ClientManager::generate_name() const String ClientManager::generate_name() const
@ -47,24 +48,17 @@ Client* ClientManager::create_client(std::unique_ptr<UserInterface>&& ui,
m_clients.emplace_back(client); m_clients.emplace_back(client);
try try
{ {
try CommandManager::instance().execute(init_commands, client->context());
{
CommandManager::instance().execute(init_commands, client->context());
}
catch (Kakoune::runtime_error& error)
{
client->context().print_status({ error.what().str(), get_face("Error") });
client->context().hooks().run_hook("RuntimeError", error.what(),
client->context());
}
} }
catch (Kakoune::client_removed& removed) catch (Kakoune::runtime_error& error)
{ {
remove_client(*client, removed.graceful); client->context().print_status({ error.what().str(), get_face("Error") });
return nullptr; client->context().hooks().run_hook("RuntimeError", error.what(),
client->context());
} }
return client; // Do not return the client if it already got moved to the trash
return contains(m_clients, client) ? client : nullptr;
} }
void ClientManager::handle_pending_inputs() const void ClientManager::handle_pending_inputs() const
@ -75,10 +69,13 @@ void ClientManager::handle_pending_inputs() const
void ClientManager::remove_client(Client& client, bool graceful) void ClientManager::remove_client(Client& client, bool graceful)
{ {
auto it = find_if(m_clients, auto it = find(m_clients, &client);
[&](const std::unique_ptr<Client>& ptr) if (it == m_clients.end())
{ return ptr.get() == &client; }); {
kak_assert(it != m_clients.end()); kak_assert(contains(m_client_trash, &client));
return;
}
m_client_trash.push_back(std::move(*it));
m_clients.erase(it); m_clients.erase(it);
if (not graceful and m_clients.empty()) if (not graceful and m_clients.empty())
@ -155,6 +152,11 @@ void ClientManager::clear_window_trash()
m_window_trash.clear(); m_window_trash.clear();
} }
void ClientManager::clear_client_trash()
{
m_client_trash.clear();
}
bool ClientManager::validate_client_name(StringView name) const bool ClientManager::validate_client_name(StringView name) const
{ {
return const_cast<ClientManager*>(this)->get_client_ifp(name) == nullptr; return const_cast<ClientManager*>(this)->get_client_ifp(name) == nullptr;

View File

@ -7,13 +7,6 @@
namespace Kakoune namespace Kakoune
{ {
struct client_removed
{
client_removed(bool graceful) : graceful{graceful} {}
const bool graceful;
};
struct WindowAndSelections struct WindowAndSelections
{ {
std::unique_ptr<Window> window; std::unique_ptr<Window> window;
@ -58,10 +51,12 @@ public:
ByteCount cursor_pos = -1) const; ByteCount cursor_pos = -1) const;
void clear_window_trash(); void clear_window_trash();
void clear_client_trash();
private: private:
String generate_name() const; String generate_name() const;
ClientList m_clients; ClientList m_clients;
ClientList m_client_trash;
Vector<WindowAndSelections, MemoryDomain::Client> m_free_windows; Vector<WindowAndSelections, MemoryDomain::Client> m_free_windows;
Vector<std::unique_ptr<Window>> m_window_trash; Vector<std::unique_ptr<Window>> m_window_trash;
}; };

View File

@ -344,12 +344,12 @@ const CommandDesc force_kill_cmd = {
}; };
template<bool force> template<bool force>
void quit() void quit(Context& context)
{ {
if (not force and ClientManager::instance().count() == 1) if (not force and ClientManager::instance().count() == 1)
ensure_all_buffers_are_saved(); ensure_all_buffers_are_saved();
// unwind back to this client event handler.
throw client_removed{ true }; ClientManager::instance().remove_client(context.client(), true);
} }
const CommandDesc quit_cmd = { const CommandDesc quit_cmd = {
@ -361,7 +361,7 @@ const CommandDesc quit_cmd = {
CommandFlags::None, CommandFlags::None,
CommandHelper{}, CommandHelper{},
CommandCompleter{}, CommandCompleter{},
[](const ParametersParser&, Context&, const ShellContext&){ quit<false>(); } [](const ParametersParser&, Context& context, const ShellContext&){ quit<false>(context); }
}; };
const CommandDesc force_quit_cmd = { const CommandDesc force_quit_cmd = {
@ -374,7 +374,7 @@ const CommandDesc force_quit_cmd = {
CommandFlags::None, CommandFlags::None,
CommandHelper{}, CommandHelper{},
CommandCompleter{}, CommandCompleter{},
[](const ParametersParser&, Context&, const ShellContext&){ quit<true>(); } [](const ParametersParser&, Context& context, const ShellContext&){ quit<true>(context); }
}; };
template<bool force> template<bool force>
@ -382,7 +382,7 @@ void write_quit(const ParametersParser& parser, Context& context,
const ShellContext& shell_context) const ShellContext& shell_context)
{ {
write_buffer(parser, context, shell_context); write_buffer(parser, context, shell_context);
quit<force>(); quit<force>(context);
} }
const CommandDesc write_quit_cmd = { const CommandDesc write_quit_cmd = {
@ -419,7 +419,7 @@ const CommandDesc writeall_quit_cmd = {
[](const ParametersParser& parser, Context& context, const ShellContext&) [](const ParametersParser& parser, Context& context, const ShellContext&)
{ {
write_all_buffers(); write_all_buffers();
quit<false>(); quit<false>(context);
} }
}; };

View File

@ -451,7 +451,7 @@ int run_client(StringView session, StringView init_command, UIType ui_type)
while (true) while (true)
event_manager.handle_next_events(EventMode::Normal); event_manager.handle_next_events(EventMode::Normal);
} }
catch (peer_disconnected&) catch (remote_error&)
{ {
write_stderr("disconnected from server\n"); write_stderr("disconnected from server\n");
return -1; return -1;
@ -524,11 +524,6 @@ int run_server(StringView session, StringView init_command,
startup_error = true; startup_error = true;
write_to_debug_buffer(format("error while parsing kakrc:\n {}", error.what())); write_to_debug_buffer(format("error while parsing kakrc:\n {}", error.what()));
} }
catch (Kakoune::client_removed&)
{
startup_error = true;
write_to_debug_buffer("error while parsing kakrc: asked to quit");
}
{ {
Context empty_context{Context::EmptyContextFlag{}}; Context empty_context{Context::EmptyContextFlag{}};
@ -589,6 +584,7 @@ int run_server(StringView session, StringView init_command,
client_manager.redraw_clients(); client_manager.redraw_clients();
event_manager.handle_next_events(EventMode::Normal); event_manager.handle_next_events(EventMode::Normal);
client_manager.handle_pending_inputs(); client_manager.handle_pending_inputs();
client_manager.clear_client_trash();
client_manager.clear_window_trash(); client_manager.clear_window_trash();
buffer_manager.clear_buffer_trash(); buffer_manager.clear_buffer_trash();
string_registry.purge_unused(); string_registry.purge_unused();

View File

@ -80,6 +80,8 @@ public:
template<typename U> template<typename U>
T value_or(U&& fallback) const { return m_valid ? m_value : T{fallback}; } T value_or(U&& fallback) const { return m_valid ? m_value : T{fallback}; }
void reset() { destruct_ifn(); m_valid = false; }
private: private:
void destruct_ifn() { if (m_valid) m_value.~T(); } void destruct_ifn() { if (m_valid) m_value.~T(); }

View File

@ -54,7 +54,7 @@ public:
*reinterpret_cast<uint32_t*>(m_stream.data()+1) = (uint32_t)m_stream.size(); *reinterpret_cast<uint32_t*>(m_stream.data()+1) = (uint32_t)m_stream.size();
int res = ::write(m_socket, m_stream.data(), m_stream.size()); int res = ::write(m_socket, m_stream.data(), m_stream.size());
if (res == 0) if (res == 0)
throw peer_disconnected{}; throw remote_error{"peer disconnected"};
} }
void write(const char* val, size_t size) void write(const char* val, size_t size)
@ -171,8 +171,8 @@ public:
void read(char* buffer, size_t size) void read(char* buffer, size_t size)
{ {
if (m_stream.size() - m_read_pos < size) if (m_read_pos + size > m_stream.size())
throw peer_disconnected{}; throw remote_error{"tried to read after message end"};
memcpy(buffer, m_stream.data() + m_read_pos, size); memcpy(buffer, m_stream.data() + m_read_pos, size);
m_read_pos += size; m_read_pos += size;
} }
@ -229,7 +229,7 @@ private:
{ {
int res = ::read(sock, m_stream.data() + m_write_pos, size); int res = ::read(sock, m_stream.data() + m_write_pos, size);
if (res == 0) if (res == 0)
throw peer_disconnected{}; throw remote_error{"peer disconnected"};
if (res < 0) if (res < 0)
throw socket_error{}; throw socket_error{};
m_write_pos += res; m_write_pos += res;
@ -325,23 +325,51 @@ public:
void set_ui_options(const Options& options) override; void set_ui_options(const Options& options) override;
void set_client(Client* client) { m_client = client; }
Client* client() const { return m_client.get(); }
private: private:
FDWatcher m_socket_watcher; FDWatcher m_socket_watcher;
MsgReader m_reader; MsgReader m_reader;
CharCoord m_dimensions; CharCoord m_dimensions;
InputCallback m_input_callback; InputCallback m_input_callback;
SafePtr<Client> m_client;
Optional<Key> m_pending_key;
}; };
RemoteUI::RemoteUI(int socket, CharCoord dimensions) RemoteUI::RemoteUI(int socket, CharCoord dimensions)
: m_socket_watcher(socket, [this](FDWatcher& watcher, EventMode mode) { : m_socket_watcher(socket, [this](FDWatcher& watcher, EventMode mode) {
const int sock = watcher.fd(); const int sock = watcher.fd();
while (fd_readable(sock) and not m_reader.ready()) bool disconnect = false;
m_reader.read_available(sock); try
{
while (fd_readable(sock) and not m_reader.ready())
m_reader.read_available(sock);
if (m_reader.ready() and m_input_callback) if (m_reader.ready() and not m_pending_key)
m_input_callback(mode); {
}), if (m_reader.type() == MessageType::Key)
{
m_pending_key = m_reader.read<Key>();
m_reader.reset();
}
else
disconnect = true;
}
}
catch (remote_error& err)
{
write_to_debug_buffer(format("Error while reading remote message: {}", err.what()));
disconnect = true;
}
if (disconnect)
ClientManager::instance().remove_client(*m_client, false);
else if (m_pending_key and m_input_callback)
m_input_callback(mode);
}),
m_dimensions(dimensions) m_dimensions(dimensions)
{ {
write_to_debug_buffer(format("remote client connected: {}", m_socket_watcher.fd())); write_to_debug_buffer(format("remote client connected: {}", m_socket_watcher.fd()));
@ -427,32 +455,17 @@ void RemoteUI::set_ui_options(const Options& options)
bool RemoteUI::is_key_available() bool RemoteUI::is_key_available()
{ {
return m_reader.ready(); return (bool)m_pending_key;
} }
Key RemoteUI::get_key() Key RemoteUI::get_key()
{ {
kak_assert(m_reader.ready()); kak_assert(m_pending_key);
try auto key = *m_pending_key;
{ m_pending_key.reset();
if (m_reader.type() != MessageType::Key) if (key.modifiers == Key::Modifiers::Resize)
throw client_removed{ false }; m_dimensions = key.coord();
return key;
Key key = m_reader.read<Key>();
m_reader.reset();
if (key.modifiers == Key::Modifiers::Resize)
m_dimensions = key.coord();
return key;
}
catch (peer_disconnected&)
{
throw client_removed{ false };
}
catch (socket_error&)
{
write_to_debug_buffer("ungraceful deconnection detected");
throw client_removed{ false };
}
} }
CharCoord RemoteUI::dimensions() CharCoord RemoteUI::dimensions()
@ -632,10 +645,12 @@ private:
auto init_command = m_reader.read<String>(); auto init_command = m_reader.read<String>();
auto dimensions = m_reader.read<CharCoord>(); auto dimensions = m_reader.read<CharCoord>();
auto env_vars = m_reader.read_idmap<String, MemoryDomain::EnvVars>(); auto env_vars = m_reader.read_idmap<String, MemoryDomain::EnvVars>();
std::unique_ptr<UserInterface> ui{new RemoteUI{sock, dimensions}}; RemoteUI* ui = new RemoteUI{sock, dimensions};
ClientManager::instance().create_client(std::move(ui), if (auto* client = ClientManager::instance().create_client(
std::move(env_vars), std::unique_ptr<UserInterface>{ui},
init_command); std::move(env_vars), init_command))
ui->set_client(client);
Server::instance().remove_accepter(this); Server::instance().remove_accepter(this);
return; return;
} }
@ -652,7 +667,6 @@ private:
write_to_debug_buffer(format("error running command '{}': {}", write_to_debug_buffer(format("error running command '{}': {}",
command, e.what())); command, e.what()));
} }
catch (client_removed&) {}
close(sock); close(sock);
Server::instance().remove_accepter(this); Server::instance().remove_accepter(this);
return; return;

View File

@ -12,7 +12,12 @@
namespace Kakoune namespace Kakoune
{ {
struct peer_disconnected {}; struct remote_error : runtime_error
{
remote_error(String error)
: runtime_error{std::move(error)}
{}
};
struct connection_failed : runtime_error struct connection_failed : runtime_error
{ {

View File

@ -1 +1 @@
<c-l>:q<ret>

View File

@ -1 +1 @@
w<c-l>:q<ret> w