Refactor client-side wesnothd handshake process

This makes things a bit less confusing by making the worker thread handle the handshake. It also
removes the need for manually spinning the main thread to wait for the handshake.

src/game_initialization/multiplayer.cpp

@@ -57,7 +57,7 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 
 	wesnothd_connection_ptr sock(nullptr);
 	if(host.empty()) {
-		return std::make_pair(std::move(sock), config());
+		return std::make_pair(nullptr, config());
 	}
 
 	// shown_hosts is used to prevent the client being locked in a redirect loop.
@@ -75,17 +75,14 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 	// Initializes the connection to the server.
 	sock = std::make_unique<wesnothd_connection>(addr.first, addr.second);
 	if(!sock) {
-		return std::make_pair(std::move(sock), config());
+		return std::make_pair(nullptr, config());
 	}
 
 	// Start stage
 	gui2::dialogs::loading_screen::progress(loading_stage::connect_to_server);
 
 	// First, spin until we get a handshake from the server.
-	while(!sock->handshake_finished()) {
-		sock->poll();
-		SDL_Delay(1);
-	}
+	sock->wait_for_handshake();
 
 	gui2::dialogs::loading_screen::progress(loading_stage::waiting);
 
@@ -98,7 +95,7 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 	// Then, log in and wait for the lobby/game join prompt.
 	do {
 		if(!sock) {
-			return std::make_pair(std::move(sock), config());
+			return std::make_pair(nullptr, config());
 		}
 
 		data.clear();
@@ -140,10 +137,7 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 			sock = std::make_unique<wesnothd_connection>(redirect_host, redirect_port);
 
 			// Wait for new handshake.
-			while(!sock->handshake_finished()) {
-				sock->poll();
-				SDL_Delay(1);
-			}
+			sock->wait_for_handshake();
 
 			gui2::dialogs::loading_screen::progress(loading_stage::waiting);
 			continue;

src/wesnothd_connection.cpp

@@ -61,7 +61,7 @@ wesnothd_connection::wesnothd_connection(const std::string& host, const std::str
 	, socket_(io_service_)
 	, last_error_()
 	, last_error_mutex_()
-	, handshake_finished_(false)
+	, handshake_finished_()
 	, read_buf_()
 	, handshake_response_()
 	, recv_queue_()
@@ -76,6 +76,12 @@ wesnothd_connection::wesnothd_connection(const std::string& host, const std::str
 	resolver_.async_resolve(boost::asio::ip::tcp::resolver::query(host, service),
 		std::bind(&wesnothd_connection::handle_resolve, this, _1, _2));
 
+	// Starts the worker thread. Do this *after* the above async_resolve call or it will just exit immediately!
+	worker_thread_ = std::thread([this]() {
+		io_service_.run();
+		LOG_NW << "wesnothd_connection::io_service::run() returned\n";
+	});
+
 	LOG_NW << "Resolving hostname: " << host << '\n';
 }
 
@@ -84,13 +90,11 @@ wesnothd_connection::~wesnothd_connection()
 	MPTEST_LOG;
 
 	// Stop the io_service and wait for the worker thread to terminate.
-	if(worker_thread_) {
-		stop();
-		worker_thread_->join();
-	}
+	stop();
+	worker_thread_.join();
 }
 
-// main thread
+// worker thread
 void wesnothd_connection::handle_resolve(const error_code& ec, resolver::iterator iterator)
 {
 	MPTEST_LOG;
@@ -102,7 +106,7 @@ void wesnothd_connection::handle_resolve(const error_code& ec, resolver::iterato
 	connect(iterator);
 }
 
-// main thread
+// worker thread
 void wesnothd_connection::connect(resolver::iterator iterator)
 {
 	MPTEST_LOG;
@@ -110,7 +114,7 @@ void wesnothd_connection::connect(resolver::iterator iterator)
 	LOG_NW << "Connecting to " << iterator->endpoint().address() << '\n';
 }
 
-// main thread
+// worker thread
 void wesnothd_connection::handle_connect(const boost::system::error_code& ec, resolver::iterator iterator)
 {
 	MPTEST_LOG;
@@ -130,7 +134,7 @@ void wesnothd_connection::handle_connect(const boost::system::error_code& ec, re
 	}
 }
 
-// main thread
+// worker thread
 void wesnothd_connection::handshake()
 {
 	MPTEST_LOG;
@@ -143,7 +147,7 @@ void wesnothd_connection::handshake()
 		std::bind(&wesnothd_connection::handle_handshake, this, _1));
 }
 
-// main thread
+// worker thread
 void wesnothd_connection::handle_handshake(const error_code& ec)
 {
 	MPTEST_LOG;
@@ -152,13 +156,16 @@ void wesnothd_connection::handle_handshake(const error_code& ec)
 		throw system_error(ec);
 	}
 
-	handshake_finished_ = true;
+	handshake_finished_.set_value(true);
 	recv();
+}
 
-	worker_thread_.reset(new std::thread([this]() {
-		io_service_.run();
-		LOG_NW << "wesnothd_connection::io_service::run() returned\n";
-	}));
+// main thread
+void wesnothd_connection::wait_for_handshake()
+{
+	MPTEST_LOG;
+	LOG_NW << "Waiting for handshake" << std::endl;
+	handshake_finished_.get_future().wait();
 }
 
 // main thread
@@ -366,7 +373,6 @@ void wesnothd_connection::recv()
 std::size_t wesnothd_connection::poll()
 {
 	MPTEST_LOG;
-	assert(!worker_thread_);
 
 	try {
 		return io_service_.poll();

src/wesnothd_connection.hpp

@@ -36,6 +36,7 @@
 #include <boost/asio.hpp>
 
 #include <deque>
+#include <future>
 #include <list>
 #include <thread>
 #include <mutex>
@@ -80,6 +81,8 @@ public:
 	 */
 	bool wait_and_receive_data(config& data);
 
+	void wait_for_handshake();
+
 	/** Handles all pending asynchornous events and returns. */
 	std::size_t poll();
 
@@ -88,12 +91,6 @@ public:
 	// Destroys this object.
 	void stop();
 
-	/** True if connected and no high-level operation is in progress */
-	bool handshake_finished() const
-	{
-		return handshake_finished_;
-	}
-
 	std::size_t bytes_to_write() const
 	{
 		return bytes_to_write_;
@@ -125,7 +122,7 @@ public:
 	}
 
 private:
-	std::unique_ptr<std::thread> worker_thread_;
+	std::thread worker_thread_;
 
 	boost::asio::io_service io_service_;
 
@@ -139,7 +136,7 @@ private:
 
 	std::mutex last_error_mutex_;
 
-	bool handshake_finished_;
+	std::promise<bool> handshake_finished_;
 
 	boost::asio::streambuf read_buf_;