Mercurial > irccd

changeset 819:d2737ddd7e36

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
irccd: improve connection error and (forced) reconnections
author David Demelier <markand@malikania.fr>
date Mon, 26 Nov 2018 21:53:27 +0100
parents 49fa22f0b4b9
children 9109f19987fb
files libirccd-daemon/irccd/daemon/irc.cpp libirccd-daemon/irccd/daemon/irc.hpp libirccd-daemon/irccd/daemon/server.cpp libirccd-daemon/irccd/daemon/server.hpp libirccd-daemon/irccd/daemon/server_service.cpp libirccd-daemon/irccd/daemon/server_service.hpp
diffstat 6 files changed, 149 insertions(+), 89 deletions(-) [+]
line wrap: on
line diff
--- a/libirccd-daemon/irccd/daemon/irc.cpp	Fri Nov 23 21:50:20 2018 +0100
+++ b/libirccd-daemon/irccd/daemon/irc.cpp	Mon Nov 26 21:53:27 2018 +0100
@@ -103,7 +103,7 @@
 		if (*it == ':')
 			arg = string(++it, end);
 		else {
-			while (!isspace(*it) && it != end)
+			while (it != end && !isspace(*it))
 				arg.push_back(*it++);
 
 			// Skip space after param.
@@ -139,7 +139,7 @@
 
 #if defined(IRCCD_HAVE_SSL)
 	ssl_socket_.async_handshake(stream_base::client, [handler] (auto code) {
-		handler(std::move(code));
+		handler(move(code));
 	});
 #endif
 }
@@ -219,6 +219,11 @@
 	resolve(hostname, service, move(chain));
 }
 
+void connection::disconnect()
+{
+	socket_.close();
+}
+
 void connection::recv(recv_handler handler)
 {
 #if !defined(NDEBUG)
@@ -237,7 +242,7 @@
 			return handler(make_error_code(errc::argument_list_too_long), message());
 		if (code == boost::asio::error::eof || xfer == 0)
 			return handler(make_error_code(errc::connection_reset), message());
-		else if (code)
+		if (code)
 			return handler(move(code), message());
 
 		string data;
--- a/libirccd-daemon/irccd/daemon/irc.hpp	Fri Nov 23 21:50:20 2018 +0100
+++ b/libirccd-daemon/irccd/daemon/irc.hpp	Mon Nov 26 21:53:27 2018 +0100
@@ -1360,6 +1360,11 @@
 	void connect(std::string_view hostname, std::string_view service, connect_handler handler);
 
 	/**
+	 * Force disconnection.
+	 */
+	void disconnect();
+
+	/**
 	 * Start receiving data.
 	 *
 	 * The handler must not throw exceptions and `this` must be valid in the
--- a/libirccd-daemon/irccd/daemon/server.cpp	Fri Nov 23 21:50:20 2018 +0100
+++ b/libirccd-daemon/irccd/daemon/server.cpp	Mon Nov 26 21:53:27 2018 +0100
@@ -441,28 +441,47 @@
 	 */
 	if (code) {
 		disconnect();
-		handler(std::move(code), event(std::monostate()));
+		handler(std::move(code), std::monostate{});
 	} else if (!dispatch(message, handler))
 		handler({}, std::monostate{});
 }
 
 void server::recv(recv_handler handler) noexcept
 {
+	assert(state_ == state::identifying || state_ == state::connected);
+
 	const auto self = shared_from_this();
 
+	timer_.expires_from_now(boost::posix_time::seconds(timeout_));
+	timer_.async_wait([this, handler, self, c = conn_] (auto code) {
+		if (c != conn_)
+			return;
+
+		if (!code) {
+			disconnect();
+			handler(make_error_code(std::errc::timed_out), std::monostate{});
+		}
+	});
+
 	conn_->recv([this, handler, self, c = conn_] (auto code, auto message) {
+		if (c != conn_)
+			return;
+
 		handle_recv(std::move(code), message, handler);
 	});
 }
 
 void server::flush()
 {
-	if (queue_.empty())
+	if (!conn_ || queue_.empty())
 		return;
 
 	const auto self = shared_from_this();
 
 	conn_->send(queue_.front(), [this, self, c = conn_] (auto code) {
+		if (c != conn_)
+			return;
+
 		handle_send(std::move(code));
 	});
 }
@@ -478,12 +497,6 @@
 	send(str(format("USER %1% unknown unknown :%2%") % username_ % realname_));
 }
 
-void server::handle_wait(const std::error_code& code, const connect_handler& handler)
-{
-	if (code && code != std::errc::operation_canceled)
-		handler(code);
-}
-
 void server::handle_connect(const std::error_code& code, const connect_handler& handler)
 {
 	timer_.cancel();
@@ -668,24 +681,52 @@
 	state_ = state::connecting;
 
 	timer_.expires_from_now(boost::posix_time::seconds(timeout_));
-	timer_.async_wait([this, handler] (auto code) {
-		handle_wait(code, handler);
+	timer_.async_wait([this, handler, c = conn_] (auto code) {
+		if (c != conn_)
+			return;
+
+		if (!code) {
+			disconnect();
+			handler(make_error_code(std::errc::timed_out));
+		}
 	});
 
 	const auto self = shared_from_this();
 
 	conn_->connect(hostname_, std::to_string(port_), [this, handler, c = conn_] (auto code) {
+		if (c != conn_)
+			return;
+
 		handle_connect(code, handler);
 	});
 }
 
-void server::disconnect() noexcept
+void server::disconnect()
 {
-	conn_ = nullptr;
 	state_ = state::disconnected;
+
+	if (conn_) {
+		conn_->disconnect();
+		conn_ = nullptr;
+	}
+
+	timer_.cancel();
 	queue_.clear();
 }
 
+void server::wait(connect_handler handler)
+{
+	assert(state_ == state::disconnected);
+
+	const auto self = shared_from_this();
+
+	timer_.expires_from_now(boost::posix_time::seconds(recodelay_));
+	timer_.async_wait([this, handler, self, c = conn_] (auto code) {
+		if (code != boost::asio::error::operation_aborted)
+			handler({});
+	});
+}
+
 void server::invite(std::string_view target, std::string_view channel)
 {
 	assert(!target.empty());
--- a/libirccd-daemon/irccd/daemon/server.hpp	Fri Nov 23 21:50:20 2018 +0100
+++ b/libirccd-daemon/irccd/daemon/server.hpp	Mon Nov 26 21:53:27 2018 +0100
@@ -354,7 +354,6 @@
 	void identify();
 	void handle_send(const std::error_code&);
 	void handle_recv(const std::error_code&, const irc::message&, const recv_handler&);
-	void handle_wait(const std::error_code&, const connect_handler&);
 	void handle_connect(const std::error_code&, const connect_handler&);
 
 public:
@@ -573,7 +572,15 @@
 	/**
 	 * Force disconnection.
 	 */
-	virtual void disconnect() noexcept;
+	virtual void disconnect();
+
+	/**
+	 * Wait for reconnect delay.
+	 *
+	 * \pre another wait operation must not be running
+	 * \pre get_state() == state::disconnected
+	 */
+	virtual void wait(connect_handler handler);
 
 	/**
 	 * Receive next event.
--- a/libirccd-daemon/irccd/daemon/server_service.cpp	Fri Nov 23 21:50:20 2018 +0100
+++ b/libirccd-daemon/irccd/daemon/server_service.cpp	Mon Nov 26 21:53:27 2018 +0100
@@ -458,6 +458,14 @@
 
 } // !namespace
 
+void server_service::handle_connect(const std::shared_ptr<server>& server, const std::error_code& code)
+{
+	if (code)
+		handle_error(server, code);
+	else
+		recv(server);
+}
+
 void server_service::handle_error(const std::shared_ptr<server>& server,
                                   const std::error_code& code)
 {
@@ -465,35 +473,19 @@
 
 	bot_.get_log().warning(*server) << code.message() << std::endl;
 
-	if ((server->get_options() & server::options::auto_reconnect) != server::options::auto_reconnect)
+	if ((server->get_options() & server::options::auto_reconnect) != server::options::auto_reconnect) {
 		remove(server->get_id());
-	else {
-		bot_.get_log().info(*server) << "reconnecting in "
-			<< server->get_reconnect_delay() << " second(s)" << std::endl;
-		wait(server);
-	}
-}
-
-void server_service::handle_wait(const std::shared_ptr<server>& server, const std::error_code& code)
-{
-	/*
-	 * The timer runs on his own control, it will complete either if the delay
-	 * was reached, there was an error or if the io_context was called to cancel
-	 * all pending operations.
-	 *
-	 * This means while the timer is running someone may already have ask a
-	 * server for explicit reconnection (e.g. remote command, plugin). Thus we
-	 * check for server state and if it is still present in service.
-	 */
-	if (code && code != std::errc::operation_canceled) {
-		bot_.get_log().warning(*server) << code.message() << std::endl;
 		return;
 	}
 
-	if (server->get_state() == server::state::connected || !has(server->get_id()))
-		return;
+	bot_.get_log().info(*server) << "reconnecting in " << server->get_reconnect_delay()
+	                             << " second(s)" << std::endl;
 
-	connect(server);
+	server->wait([this, server] (auto code) {
+		handle_wait(server, code);
+	});
+
+	dispatcher{bot_}(disconnect_event{server});
 }
 
 void server_service::handle_recv(const std::shared_ptr<server>& server,
@@ -502,32 +494,49 @@
 {
 	assert(server);
 
-	if (code)
+	if (code) {
 		handle_error(server, code);
-	else {
-		recv(server);
-		std::visit(dispatcher(bot_), event);
+		return;
+	}
+
+	recv(server);
+	std::visit(dispatcher(bot_), event);
+}
+
+void server_service::handle_wait(const std::shared_ptr<server>& server, const std::error_code& code)
+{
+	if (code == std::errc::operation_canceled || server->get_state() != server::state::disconnected)
+		return;
+
+	connect(server);
+}
+
+void server_service::connect(const std::shared_ptr<server>& server)
+{
+	assert(server);
+
+	server->connect([this, server] (auto code) {
+		handle_connect(server, code);
+	});
+}
+
+void server_service::disconnect(const std::shared_ptr<server>& server)
+{
+	if (server->get_state() != server::state::disconnected) {
+		server->disconnect();
+		servers_.erase(std::find(servers_.begin(), servers_.end(), server), servers_.end());
+		dispatcher{bot_}(disconnect_event{server});
 	}
 }
 
-void server_service::handle_connect(const std::shared_ptr<server>& server, const std::error_code& code)
+void server_service::reconnect(const std::shared_ptr<server>& server)
 {
-	if (code)
-		handle_error(server, code);
-	else
-		recv(server);
-}
+	disconnect(server);
 
-void server_service::wait(const std::shared_ptr<server>& server)
-{
-	assert(server);
-
-	auto timer = std::make_shared<boost::asio::deadline_timer>(bot_.get_service());
-
-	timer->expires_from_now(boost::posix_time::seconds(server->get_reconnect_delay()));
-	timer->async_wait([this, server, timer] (auto code) {
-		handle_wait(server, code);
-	});
+	if (has(server->get_id()))
+		connect(server);
+	else
+		add(server);
 }
 
 void server_service::recv(const std::shared_ptr<server>& server)
@@ -539,15 +548,6 @@
 	});
 }
 
-void server_service::connect(const std::shared_ptr<server>& server)
-{
-	assert(server);
-
-	server->connect([this, server] (auto code) {
-		handle_connect(server, code);
-	});
-}
-
 server_service::server_service(bot& bot)
 	: bot_(bot)
 {
@@ -558,7 +558,7 @@
 	return servers_;
 }
 
-auto server_service::has(const std::string& name) const noexcept -> bool
+auto server_service::has(std::string_view name) const noexcept -> bool
 {
 	return std::count_if(servers_.begin(), servers_.end(), [&] (const auto& server) {
 		return server->get_id() == name;
@@ -601,25 +601,26 @@
 
 void server_service::disconnect(std::string_view id)
 {
-	const auto s = require(id);
-
-	s->disconnect();
-	dispatcher{bot_}(disconnect_event{s});
+	disconnect(require(id));
 }
 
 void server_service::reconnect(std::string_view id)
 {
-	disconnect(id);
-	connect(require(id));
+	const auto save = get(id);
+
+	if (!save)
+		return;
+
+	reconnect(save);
 }
 
 void server_service::reconnect()
 {
-	for (const auto& s : servers_) {
+	const auto save = servers_;
+
+	for (const auto& s : save) {
 		try {
-			s->disconnect();
-			dispatcher{bot_}(disconnect_event{s});
-			connect(s);
+			reconnect(s);
 		} catch (const server_error& ex) {
 			bot_.get_log().warning(*s) << ex.what() << std::endl;
 		}
@@ -641,13 +642,13 @@
 void server_service::clear() noexcept
 {
 	/*
-	 * Copy the array, because disconnect() may trigger on_die signal which
-	 * erase the server from itself.
+	 * Copy the array, because disconnect() interrupted signals may remove the
+	 * server from the array.
 	 */
 	const auto save = servers_;
 
 	for (const auto& server : save)
-		server->disconnect();
+		disconnect(server);
 
 	servers_.clear();
 }
--- a/libirccd-daemon/irccd/daemon/server_service.hpp	Fri Nov 23 21:50:20 2018 +0100
+++ b/libirccd-daemon/irccd/daemon/server_service.hpp	Mon Nov 26 21:53:27 2018 +0100
@@ -49,14 +49,15 @@
 	bot& bot_;
 	std::vector<std::shared_ptr<server>> servers_;
 
+	void handle_connect(const std::shared_ptr<server>&, const std::error_code&);
 	void handle_error(const std::shared_ptr<server>&, const std::error_code&);
-	void handle_wait(const std::shared_ptr<server>&, const std::error_code&);
 	void handle_recv(const std::shared_ptr<server>&, const std::error_code&, const event&);
-	void handle_connect(const std::shared_ptr<server>&, const std::error_code&);
+	void handle_wait(const std::shared_ptr<server>&, const std::error_code&);
 
-	void wait(const std::shared_ptr<server>&);
+	void connect(const std::shared_ptr<server>&);
+	void disconnect(const std::shared_ptr<server>&);
+	void reconnect(const std::shared_ptr<server>&);
 	void recv(const std::shared_ptr<server>&);
-	void connect(const std::shared_ptr<server>&);
 
 public:
 	/**
@@ -79,7 +80,7 @@
 	 * \param name the name
 	 * \return true if exists
 	 */
-	auto has(const std::string& name) const noexcept -> bool;
+	auto has(std::string_view name) const noexcept -> bool;
 
 	/**
 	 * Add a new server to the application.