ircd::net: Various fixes / error handling / api.

include/ircd/net/net.h

@@ -42,6 +42,7 @@ namespace ircd::net
 	struct remote;
 	struct socket;
 	struct listener;
+	enum class dc;
 
 	// SNOMASK 'N' "net"
 	extern struct log::log log;
@@ -50,6 +51,16 @@ namespace ircd::net
 #include "remote.h"
 #include "listener.h"
 
+enum class ircd::net::dc
+{
+	RST,                ///< hardest immediate termination
+	FIN,                ///< sd graceful shutdown both directions
+	FIN_SEND,           ///< sd graceful shutdown send side
+	FIN_RECV,           ///< sd graceful shutdown recv side
+	SSL_NOTIFY,         ///< SSL close_notify (async, errors ignored)
+	SSL_NOTIFY_YIELD,   ///< SSL close_notify (yields context, throws)
+};
+
 // Public interface to socket.h because it is not included here.
 namespace ircd::net
 {
@@ -69,6 +80,9 @@ namespace ircd::net
 	size_t read(socket &, const iov<mutable_buffer> &);      // read_all
 	size_t read(socket &, const mutable_buffer &);           // read_all
 	size_t read(socket &, iov<mutable_buffer> &);            // read_some
+
+	std::shared_ptr<socket> connect(const remote &, const milliseconds &timeout = 30000ms);
+	bool disconnect(socket &, const dc &type = dc::SSL_NOTIFY) noexcept;
 }
 
 namespace ircd

include/ircd/net/socket.h

@@ -42,9 +42,9 @@ namespace ircd::net
 	uint16_t port(const ip::tcp::endpoint &);
 	ip::address addr(const ip::tcp::endpoint &);
 	std::string host(const ip::tcp::endpoint &);
-
 	std::string string(const ip::address &);
 	std::string string(const ip::tcp::endpoint &);
+	std::shared_ptr<socket> connect(const ip::tcp::endpoint &remote, const milliseconds &timeout);
 }
 
 namespace ircd
@@ -62,7 +62,6 @@ struct ircd::net::socket
 	struct io;
 	struct stat;
 	struct scope_timeout;
-	enum class dc;
 
 	struct stat
 	{
@@ -127,21 +126,8 @@ struct ircd::net::socket
 	void connect(const ip::tcp::endpoint &ep, const milliseconds &timeout, handler callback);
 	void connect(const ip::tcp::endpoint &ep, const milliseconds &timeout = 30000ms);
 	void connect(const net::remote &, const milliseconds &timeout = 30000ms);
-	void disconnect(const dc &type);
+	bool disconnect(const dc &type);
 
-	// Construct, resolve and connect client socket to remote host (yields)
-	socket(const net::remote &,
-	       const milliseconds &timeout           = 30000ms,
-	       asio::ssl::context &ssl               = sslv23_client,
-	       boost::asio::io_service *const &ios   = ircd::ios);
-
-	// Construct and connect client socket to remote host (yields)
-	socket(const ip::tcp::endpoint &remote,
-	       const milliseconds &timeout           = 30000ms,
-	       asio::ssl::context &ssl               = sslv23_client,
-	       boost::asio::io_service *const &ios   = ircd::ios);
-
-	// Construct socket only
 	socket(asio::ssl::context &ssl               = sslv23_client,
 	       boost::asio::io_service *const &ios   = ircd::ios);
 
@@ -182,16 +168,6 @@ class ircd::net::socket::io
 	io(struct socket &, struct stat &, const std::function<size_t ()> &closure);
 };
 
-enum class ircd::net::socket::dc
-{
-	RST,                // hardest disconnect
-	FIN,                // graceful shutdown both directions
-	FIN_SEND,           // graceful shutdown send side
-	FIN_RECV,           // graceful shutdown recv side
-	SSL_NOTIFY,         // SSL close_notify (async, errors ignored)
-	SSL_NOTIFY_YIELD,   // SSL close_notify (yields context, throws)
-};
-
 template<class iov>
 auto
 ircd::net::socket::write(const iov &bufs)

ircd/client.cc

@@ -54,15 +54,11 @@ ctx::pool request
 // Container for all active clients (connections) for iteration purposes.
 client::list client::clients;
 
-bool handle_ec_timeout(client &);
+static bool handle_ec(client &, const net::error_code &);
-bool handle_ec_eof(client &);
-bool handle_ec_success(client &);
-bool handle_ec(client &, const net::error_code &);
-
 void async_recv_next(std::shared_ptr<client>, const milliseconds &timeout);
 void async_recv_next(std::shared_ptr<client>);
 
-void disconnect(client &, const socket::dc & = socket::dc::RST);
+void disconnect(client &, const net::dc & = net::dc::RST);
 void disconnect_all();
 
 template<class... args> std::shared_ptr<client> make_client(args&&...);
@@ -196,7 +192,7 @@ ircd::client::client(const hostport &host_port,
                      const seconds &timeout)
 :client
 {
-	std::make_shared<socket>(host_port, timeout)
+	net::connect(host_port, timeout)
 }
 {
 }
@@ -211,8 +207,7 @@ ircd::client::client(std::shared_ptr<socket> sock)
 ircd::client::~client()
 noexcept try
 {
-	if(sock)
+	disconnect(*this, net::dc::SSL_NOTIFY);
-		sock->disconnect(socket::dc::SSL_NOTIFY);
 }
 catch(const std::exception &e)
 {
@@ -232,7 +227,7 @@ noexcept try
 {
 	const auto header_max{8192};
 	const auto content_max{65536};
-	unique_buffer<mutable_buffer> buffer
+	const unique_buffer<mutable_buffer> buffer
 	{
 		header_max + content_max
 	};
@@ -251,18 +246,21 @@ noexcept try
 }
 catch(const boost::system::system_error &e)
 {
-	using boost::asio::error::eof;
-	using boost::asio::error::broken_pipe;
-	using boost::asio::error::connection_reset;
 	using namespace boost::system::errc;
+	using boost::system::get_system_category;
+	using boost::asio::error::get_ssl_category;
+	using boost::asio::error::get_misc_category;
 
-	switch(e.code().value())
+	const auto ec
+	{
+		e.code()
+	};
+
+	if(ec.category() == get_system_category()) switch(ec.value())
 	{
 		case success:
-			assert(0);
 			return true;
 
-		case eof:
 		case broken_pipe:
 		case connection_reset:
 		case not_connected:
@@ -272,8 +270,27 @@ catch(const boost::system::system_error &e)
 		default:
 			break;
 	}
+	else if(ec.category() == get_misc_category()) switch(ec.value())
+	{
+		case boost::asio::error::eof:
+			return false;
+
+		default:
+			break;
+	}
+	else if(ec.category() == get_ssl_category()) switch(ec.value())
+	{
+		case SSL_R_SHORT_READ:
+			return false;
+
+		default:
+			break;
+	}
+
+	log::critical("client(%p): (unexpected) system_error: %s",
+	              (const void *)this,
+	              e.what());
 
-	log::critical("(unexpected) system_error: %s", e.what());
 	if(ircd::debugmode)
 		throw;
 
@@ -297,11 +314,11 @@ ircd::handle_request(client &client,
 try
 {
 	client.request_timer = ircd::timer{};
-	client.sock->set_timeout(request_timeout, [&client]
+	client.sock->set_timeout(request_timeout, [client(shared_from(client))]
 	(const net::error_code &ec)
 	{
 		if(!ec)
-			client.sock->cancel();
+			disconnect(*client, net::dc::SSL_NOTIFY_YIELD);
 	});
 
 	bool ret{true};
@@ -328,10 +345,13 @@ catch(const http::error &e)
 
 	switch(e.code)
 	{
-		case http::BAD_REQUEST:               return false;
+		case http::BAD_REQUEST:
-		case http::INTERNAL_SERVER_ERROR:     return false;
+		case http::REQUEST_TIMEOUT:
-		case http::REQUEST_TIMEOUT:           return false;
+		case http::INTERNAL_SERVER_ERROR:
-		default:                              return true;
+			return false;
+
+		default:
+			return true;
 	}
 }
 
@@ -382,7 +402,7 @@ ircd::disconnect_all()
 {
 	for(auto &client : client::clients) try
 	{
-		disconnect(*client, socket::dc::RST);
+		disconnect(*client, net::dc::RST);
 	}
 	catch(const std::exception &e)
 	{
@@ -392,10 +412,10 @@ ircd::disconnect_all()
 
 void
 ircd::disconnect(client &client,
-                 const socket::dc &type)
+                 const net::dc &type)
 {
-	auto &sock(*client.sock);
+	if(likely(client.sock))
-	sock.disconnect(type);
+		disconnect(*client.sock, type);
 }
 
 void
@@ -439,46 +459,85 @@ ircd::async_recv_next(std::shared_ptr<client> client,
 		// of the ircd::context system. The context the closure ends up getting is the next
 		// available from the request pool, which may not be available immediately so this
 		// handler might be queued for some time after this call returns.
-		request([client(std::move(client)), timeout]
+		request([ec, client, timeout]
 		{
 			// Right here this handler is executing on an ircd::context with its own
 			// stack dedicated to the lifetime of this request. If client::main()
 			// returns true, we bring the client back into async mode to wait for
-			// the next request. Otherwise, unless the client was preserved by
+			// the next request.
-			// functionality in main(), it will go out of scope after this which
-			// will disconnect the socket and destroy the client and return this
-			// context to the request pool.
 			if(client->main())
 				async_recv_next(client, timeout);
 		});
 	});
 }
 
+namespace ircd
+{
+	static bool handle_ec_success(client &);
+	static bool handle_ec_timeout(client &);
+	static bool handle_ec_eof(client &);
+	static bool handle_ec_short_read(client &);
+	static bool handle_ec_default(client &, const net::error_code &);
+}
+
 bool
 ircd::handle_ec(client &client,
                 const net::error_code &ec)
 {
 	using namespace boost::system::errc;
-	using boost::asio::error::eof;
+	using boost::system::get_system_category;
+	using boost::asio::error::get_ssl_category;
+	using boost::asio::error::get_misc_category;
 
-	switch(ec.value())
+	if(ec.category() == get_system_category()) switch(ec.value())
 	{
 		case success:                return handle_ec_success(client);
-		case eof:                    return handle_ec_eof(client);
 		case operation_canceled:     return handle_ec_timeout(client);
-		default:
+		default:                     return handle_ec_default(client, ec);
-		{
-			log::debug("client(%p): %s", &client, ec.message());
-			disconnect(client, socket::dc::RST);
-			return false;
-		}
 	}
+	else if(ec.category() == get_misc_category()) switch(ec.value())
+	{
+		case asio::error::eof:       return handle_ec_eof(client);
+		default:                     return handle_ec_default(client, ec);
+	}
+	else if(ec.category() == get_ssl_category()) switch(ec.value())
+	{
+		case SSL_R_SHORT_READ:       return handle_ec_short_read(client);
+		default:                     return handle_ec_default(client, ec);
+	}
+	else return handle_ec_default(client, ec);
 }
 
 bool
-ircd::handle_ec_success(client &client)
+ircd::handle_ec_default(client &client,
+                        const net::error_code &ec)
 {
-	return true;
+	log::debug("client(%p): %s: %s",
+	           &client,
+	           ec.category().name(),
+	           ec.message());
+
+	disconnect(client, net::dc::SSL_NOTIFY);
+	return false;
+}
+
+bool
+ircd::handle_ec_short_read(client &client)
+try
+{
+	log::debug("client[%s]: short_read",
+	           string(remote(client)));
+
+	disconnect(client, net::dc::RST);
+	return false;
+}
+catch(const std::exception &e)
+{
+	log::warning("client(%p): short_read: %s",
+	             &client,
+	             e.what());
+
+	return false;
 }
 
 bool
@@ -488,7 +547,7 @@ try
 	log::debug("client[%s]: EOF",
 	           string(remote(client)));
 
-	disconnect(client, socket::dc::RST);
+	disconnect(client, net::dc::RST);
 	return false;
 }
 catch(const std::exception &e)
@@ -508,7 +567,7 @@ try
 	log::debug("client[%s]: disconnecting after inactivity timeout",
 	           string(remote(client)));
 
-	disconnect(client, socket::dc::SSL_NOTIFY);
+	disconnect(client, net::dc::SSL_NOTIFY);
 	return false;
 }
 catch(const std::exception &e)
@@ -519,3 +578,9 @@ catch(const std::exception &e)
 
 	return false;
 }
+
+bool
+ircd::handle_ec_success(client &client)
+{
+	return true;
+}

ircd/net.cc

@@ -293,17 +293,18 @@ noexcept try
 		this->next();
 	}};
 
-	ip::tcp::socket &sd(*sock);
 	log.debug("%s: socket(%p) accepted %s",
 	          std::string(*this),
 	          sock.get(),
 	          string(sock->remote()));
 
+	//ip::tcp::socket &sd(*sock);
+
 	//static const asio::socket_base::keep_alive keep_alive(true);
 	//sd.set_option(keep_alive);
 
-	static const asio::socket_base::linger linger{true, 10};
+	//static const asio::socket_base::linger linger{true, 10};
-	sd.set_option(linger);
+	//sd.set_option(linger);
 
 	//sd.non_blocking(false);
 
@@ -330,7 +331,9 @@ catch(const std::exception &e)
 bool
 ircd::net::listener::acceptor::accept_error(const error_code &ec)
 {
-	switch(ec.value())
+	using boost::system::get_system_category;
+
+	if(ec.category() == get_system_category()) switch(ec.value())
 	{
 		using namespace boost::system::errc;
 
@@ -341,8 +344,10 @@ ircd::net::listener::acceptor::accept_error(const error_code &ec)
 			return true;
 
 		default:
-			throw boost::system::system_error(ec);
+			break;
 	}
+
+	throw boost::system::system_error(ec);
 }
 
 void
@@ -372,7 +377,9 @@ catch(const std::exception &e)
 bool
 ircd::net::listener::acceptor::handshake_error(const error_code &ec)
 {
-	switch(ec.value())
+	using boost::system::get_system_category;
+
+	if(ec.category() == get_system_category()) switch(ec.value())
 	{
 		using namespace boost::system::errc;
 
@@ -383,8 +390,10 @@ ircd::net::listener::acceptor::handshake_error(const error_code &ec)
 			return true;
 
 		default:
-			throw boost::system::system_error(ec);
+			break;
 	}
+
+	throw boost::system::system_error(ec);
 }
 
 void
@@ -724,40 +733,57 @@ ircd::net::socket::scope_timeout::release()
 	return s != nullptr;
 }
 
+std::shared_ptr<ircd::net::socket>
+ircd::net::connect(const net::remote &remote,
+                   const milliseconds &timeout)
+{
+	const asio::ip::tcp::endpoint ep
+	{
+		is_v6(remote)? asio::ip::tcp::endpoint
+		{
+			asio::ip::address_v6 { std::get<remote.IP>(remote) }, port(remote)
+		}
+		: asio::ip::tcp::endpoint
+		{
+			asio::ip::address_v4 { host4(remote) }, port(remote)
+		},
+	};
+
+	return connect(ep, timeout);
+}
+
+std::shared_ptr<ircd::net::socket>
+ircd::net::connect(const ip::tcp::endpoint &remote,
+                   const milliseconds &timeout)
+{
+	const auto ret(std::make_shared<socket>());
+	ret->connect(remote, timeout);
+	return ret;
+}
+
+bool
+ircd::net::disconnect(socket &socket,
+                      const dc &type)
+noexcept try
+{
+	socket.disconnect(type);
+	return true;
+}
+catch(const std::exception &e)
+{
+/*
+	log::error("socket(%p): disconnect: type: %d: %s",
+	           this,
+	           int(type),
+	           e.what());
+*/
+	return false;
+}
+
 //
 // socket
 //
 
-ircd::net::socket::socket(const net::remote &remote,
-                          const milliseconds &timeout,
-                          asio::ssl::context &ssl,
-                          boost::asio::io_service *const &ios)
-:socket
-{
-	is_v6(remote)? asio::ip::tcp::endpoint
-	{
-		asio::ip::address_v6 { std::get<remote.IP>(remote) }, port(remote)
-	}
-	: asio::ip::tcp::endpoint
-	{
-		asio::ip::address_v4 { host4(remote) }, port(remote)
-	},
-	timeout,
-	ssl,
-	ios
-}
-{
-}
-
-ircd::net::socket::socket(const ip::tcp::endpoint &remote,
-                          const milliseconds &timeout,
-                          asio::ssl::context &ssl,
-                          boost::asio::io_service *const &ios)
-:socket{ssl, ios}
-{
-	connect(remote, timeout);
-}
-
 ircd::net::socket::socket(asio::ssl::context &ssl,
                           boost::asio::io_service *const &ios)
 :sd
@@ -779,10 +805,18 @@ ircd::net::socket::socket(asio::ssl::context &ssl,
 {
 }
 
+/// The dtor asserts that the socket is not open/connected requiring a
+/// an SSL close_notify. There's no more room for async callbacks via
+/// shared_ptr after this dtor.
 ircd::net::socket::~socket()
 noexcept try
 {
-	//disconnect(dc::RST);
+	if(unlikely(RB_DEBUG_LEVEL && connected()))
+		log.critical("Failed to ensure socket(%p) is disconnected from %s before dtor.",
+		             this,
+		             string(remote()));
+
+	assert(!connected());
 }
 catch(const std::exception &e)
 {
@@ -797,13 +831,13 @@ ircd::net::socket::connect(const ip::tcp::endpoint &ep,
                            const milliseconds &timeout)
 try
 {
+	const life_guard<socket> lg{*this};
+	const scope_timeout ts{*this, timeout};
 	log.debug("socket(%p) attempting connect to remote: %s for the next %ld$ms",
 	          this,
 	          string(ep),
 	          timeout.count());
 
-	ip::tcp::socket &sd(*this);
-	const scope_timeout ts{*this, timeout};
 	sd.async_connect(ep, yield_context{to_asio{}});
 	log.debug("socket(%p) connected to remote: %s from local: %s; performing handshake...",
 	          this,
@@ -822,6 +856,9 @@ catch(const std::exception &e)
 	          this,
 	          string(ep),
 	          e.what());
+
+	disconnect(dc::RST);
+	throw;
 }
 
 /// Attempt to connect and ssl handshake remote; yields ircd::ctx; throws timeout
@@ -856,18 +893,37 @@ ircd::net::socket::connect(const ip::tcp::endpoint &ep,
 	(const error_code &ec)
 	noexcept
 	{
-		if(!timedout)
+		if(timedout)
-			cancel_timeout();
-		else
 			assert(ec == boost::system::errc::operation_canceled);
 
-		callback(ec);
+		if(!timedout)
+			cancel_timeout();
+
+		try
+		{
+			callback(ec);
+		}
+		catch(const std::exception &e)
+		{
+			log.error("socket(%p): connect: unhandled exception from user callback: %s",
+			          (const void *)this,
+			          e.what());
+		}
 	}};
 
 	auto connect_handler{[this, handshake_handler(std::move(handshake_handler))]
 	(const error_code &ec)
 	noexcept
 	{
+		// Even though the branch on ec below should cancel the timeout on
+		// error, the timeout still needs to be canceled if else anything bad
+		// happens in the remainder of this frame too.
+		const unwind::exceptional cancels{[this]
+		{
+			cancel_timeout();
+		}};
+
+		// A connect error
 		if(ec)
 		{
 			handshake_handler(ec);
@@ -878,12 +934,11 @@ ircd::net::socket::connect(const ip::tcp::endpoint &ep,
 		ssl.async_handshake(handshake, std::move(handshake_handler));
 	}};
 
-	set_timeout(timeout);
-	ip::tcp::socket &sd(*this);
 	sd.async_connect(ep, std::move(connect_handler));
+	set_timeout(timeout);
 }
 
-void
+bool
 ircd::net::socket::disconnect(const dc &type)
 try
 {
@@ -901,51 +956,58 @@ try
 		default:
 		case dc::RST:
 			sd.close();
-			break;
+			return true;
 
 		case dc::FIN:
 			sd.shutdown(ip::tcp::socket::shutdown_both);
-			break;
+			return true;
 
 		case dc::FIN_SEND:
 			sd.shutdown(ip::tcp::socket::shutdown_send);
-			break;
+			return true;
 
 		case dc::FIN_RECV:
 			sd.shutdown(ip::tcp::socket::shutdown_receive);
-			break;
+			return true;
 
 		case dc::SSL_NOTIFY_YIELD:
 		{
+			const life_guard<socket> lg{*this};
+			const scope_timeout ts{*this, 8s};
 			ssl.async_shutdown(yield_context{to_asio{}});
-			sd.close();
+			return true;
-			break;
 		}
 
 		case dc::SSL_NOTIFY:
 		{
+			set_timeout(8s);
 			ssl.async_shutdown([s(shared_from_this())]
 			(boost::system::error_code ec) noexcept
 			{
+				if(!s->timedout)
+					s->cancel_timeout();
+
 				if(ec)
-				{
+					log.warning("socket(%p): SSL_NOTIFY: %s: %s",
-					log.warning("socket(%p): close_notify: %s",
 					            s.get(),
+					            ec.category().name(),
 					            ec.message());
-					return;
-				}
 
-				if(s->sd.is_open())
+				if(!s->sd.is_open())
-					s->sd.close(ec);
+					return;
+
+				s->sd.close(ec);
 
 				if(ec)
-					log.warning("socket(%p): close(): %s",
+					log.warning("socket(%p): after SSL_NOTIFY: %s: %s",
 					            s.get(),
+					            ec.category().name(),
 					            ec.message());
 			});
-			break;
+			return true;
 		}
 	}
+	else return false;
 }
 catch(const boost::system::system_error &e)
 {
@@ -953,6 +1015,18 @@ catch(const boost::system::system_error &e)
 	            (const void *)this,
 	            uint(type),
 	            e.what());
+
+	if(!sd.is_open())
+		throw;
+
+	boost::system::error_code ec;
+	sd.close(ec);
+
+	if(ec)
+		log.warning("socket(%p): after disconnect: %s: %s",
+		            this,
+		            ec.category().name(),
+		            ec.message());
 	throw;
 }
 
@@ -1041,7 +1115,10 @@ noexcept try
 	// user's callback. Otherwise they are passed up.
 	if(!handle_error(ec))
 	{
-		log.debug("socket(%p): %s", this, ec.message());
+		log.warning("socket(%p): %s",
+		            this,
+		            ec.category().name(),
+		            ec.message());
 		return;
 	}
 
@@ -1083,8 +1160,17 @@ bool
 ircd::net::socket::handle_error(const error_code &ec)
 {
  	using namespace boost::system::errc;
+	using boost::system::get_system_category;
+	using boost::asio::error::get_ssl_category;
+	using boost::asio::error::get_misc_category;
 
-	switch(ec.value())
+	if(ec != success)
+		log.error("socket(%p): handle error: %s: %s",
+		          this,
+		          ec.category().name(),
+		          ec.message());
+
+	if(ec.category() == get_system_category()) switch(ec.value())
 	{
 		// A success is not an error; can call the user handler
 		case success:
@@ -1096,11 +1182,6 @@ ircd::net::socket::handle_error(const error_code &ec)
 		case operation_canceled:
 			return timedout;
 
-		// This indicates the remote closed the socket, we still
-		// pass this up to the user so they can handle it.
-		case boost::asio::error::eof:
-			return true;
-
 		// This is a condition which we hide from the user.
 		case bad_file_descriptor:
 			return false;
@@ -1109,6 +1190,28 @@ ircd::net::socket::handle_error(const error_code &ec)
 		default:
 			return true;
 	}
+	else if(ec.category() == get_misc_category()) switch(ec.value())
+	{
+		// This indicates the remote closed the socket, we still
+		// pass this up to the user so they can know that too.
+		case boost::asio::error::eof:
+			return true;
+
+		default:
+			return true;
+	}
+	else if(ec.category() == get_ssl_category()) switch(ec.value())
+	{
+		// Docs say this means we read less bytes off the socket than desired.
+		case SSL_R_SHORT_READ:
+			return true;
+
+		default:
+			return true;
+	}
+
+	assert(0);
+	return true;
 }
 
 void
@@ -1130,6 +1233,7 @@ noexcept try
 
 		// A cancelation means there was no timeout.
 		case operation_canceled:
+			assert(ec.category() == boost::system::get_system_category());
 			timedout = false;
 			break;
 
@@ -1142,7 +1246,7 @@ catch(const boost::system::system_error &e)
 {
 	log.error("socket(%p): handle_timeout: unexpected: %s\n",
 	          (const void *)this,
-		   e.what());
+	          e.what());
 }
 catch(const std::exception &e)
 {