Refactor out custom wesnothd_connection_ptr class again

I originally did this in 699047766a and then reverted it in
08a866dc20 due to @gfgtdf pointing out the object was never
actually destroyed since io_service::stop was never called (issue #1927).

This new method foregoes the custom wrapper class and instead waits for the worker thread
to terminate in the wesnothd_connection dtor. The use of the wrapper class is also why using
thread::detach alone worked (@jyrkive) and using join did not.

Additionally, the ptr alias is now of a unique_ptr instead of a shared_ptr.

I have verified that the connection dtor is actually called. This new method makes it clearer
what's actually going on.

src/game_initialization/multiplayer.cpp

@@ -54,7 +54,7 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 {
 	DBG_MP << "opening connection" << std::endl;
 
-	wesnothd_connection_ptr sock;
+	wesnothd_connection_ptr sock(nullptr);
 	if(host.empty()) {
 		return std::make_pair(std::move(sock), config());
 	}
@@ -76,7 +76,7 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 	shown_hosts.emplace(host, port);
 
 	// Initializes the connection to the server.
-	sock = wesnothd_connection::create(host, std::to_string(port));
+	sock = std::make_unique<wesnothd_connection>(host, std::to_string(port));
 	if(!sock) {
 		return std::make_pair(std::move(sock), config());
 	}
@@ -139,8 +139,8 @@ std::pair<wesnothd_connection_ptr, config> open_connection(std::string host)
 			gui2::dialogs::loading_screen::progress(loading_stage::redirect);
 
 			// Open a new connection with the new host and port.
-			sock = wesnothd_connection_ptr();
+			sock.reset();
-			sock = wesnothd_connection::create(host, std::to_string(port));
+			sock = std::make_unique<wesnothd_connection>(host, std::to_string(port));
 
 			// Wait for new handshake.
 			while(!sock->handshake_finished()) {

src/wesnothd_connection.cpp

@@ -80,8 +80,11 @@ wesnothd_connection::wesnothd_connection(const std::string& host, const std::str
 
 wesnothd_connection::~wesnothd_connection()
 {
-	worker_thread_->detach();
 	MPTEST_LOG;
+
+	// Stop the io_service and wait for the worker thread to terminate.
+	stop();
+	worker_thread_->join();
 }
 
 // main thread
@@ -150,9 +153,6 @@ void wesnothd_connection::handle_handshake(const error_code& ec)
 	recv();
 
 	worker_thread_.reset(new std::thread([this]() {
-		// worker thread
-		std::shared_ptr<wesnothd_connection> this_ptr = this->shared_from_this();
-
 		io_service_.run();
 		LOG_NW << "wesnothd_connection::io_service::run() returned\n";
 	}));
@@ -422,31 +422,3 @@ bool wesnothd_connection::fetch_data_with_loading_screen(config& cfg, loading_st
 
 	return res;
 }
-
-//  wesnothd_connection_ptr
-
-wesnothd_connection_ptr& wesnothd_connection_ptr::operator=(wesnothd_connection_ptr&& other)
-{
-	MPTEST_LOG;
-	if(ptr_) {
-		ptr_->stop();
-		ptr_.reset();
-	}
-
-	ptr_ = std::move(other.ptr_);
-	return *this;
-}
-
-wesnothd_connection_ptr wesnothd_connection::create(const std::string& host, const std::string& service)
-{
-	// Can't use make_shared because the ctor is private
-	return wesnothd_connection_ptr(std::shared_ptr<wesnothd_connection>(new wesnothd_connection(host, service)));
-}
-
-wesnothd_connection_ptr::~wesnothd_connection_ptr()
-{
-	MPTEST_LOG;
-	if(ptr_) {
-		ptr_->stop();
-	}
-}

src/wesnothd_connection.hpp

@@ -45,7 +45,6 @@
 #include <mutex>
 
 class config;
-class wesnothd_connection_ptr;
 enum class loading_stage;
 
 union data_union {
@@ -54,7 +53,7 @@ union data_union {
 };
 
 /** A class that represents a TCP/IP connection to the wesnothd server. */
-class wesnothd_connection : public std::enable_shared_from_this<wesnothd_connection>
+class wesnothd_connection
 {
 public:
 	using error = wesnothd_connection_error;
@@ -64,20 +63,15 @@ public:
 
 	~wesnothd_connection();
 
-private:
+public:
 	/**
 	 * Constructor.
 	 *
-	 * May only be called via wesnothd_connection_ptr
-	 *
 	 * @param host    Name of the host to connect to
 	 * @param service Service identifier such as "80" or "http"
 	 */
 	wesnothd_connection(const std::string& host, const std::string& service);
 
-public:
-	static wesnothd_connection_ptr create(const std::string& host, const std::string& service);
-
 	bool fetch_data_with_loading_screen(config& cfg, loading_stage stage);
 
 	void send_data(const configr_of& request);
@@ -186,64 +180,4 @@ private:
 	std::size_t bytes_read_;
 };
 
-/**
+using wesnothd_connection_ptr = std::unique_ptr<wesnothd_connection>;
- * This class acts like a unique_ptr<wesnothd_connection>, wesnothd_connection objects may only be owned though this
- * pointer. The reason why we need this is that wesnothd_connection runs a workerthread so we use a shared_ptr to make
- * sure the wesnothd_connection isn't destroyed before the worker thread has finished. When this object is destroyed, it
- * calls wesnothd_connection::stop() which stops the worker thread which will then destroy the other
- * shared_ptr<wesnothd_connection> which destroys the wesnothd_connection object.
- */
-class wesnothd_connection_ptr
-{
-private:
-	friend class wesnothd_connection;
-
-	wesnothd_connection_ptr(std::shared_ptr<wesnothd_connection>&& ptr)
-		: ptr_(std::move(ptr))
-	{
-	}
-
-public:
-	wesnothd_connection_ptr() = default;
-
-	wesnothd_connection_ptr(const wesnothd_connection_ptr&) = delete;
-	wesnothd_connection_ptr& operator=(const wesnothd_connection_ptr&) = delete;
-
-	wesnothd_connection_ptr(wesnothd_connection_ptr&&) = default;
-	wesnothd_connection_ptr& operator=(wesnothd_connection_ptr&&);
-
-	~wesnothd_connection_ptr();
-
-	explicit operator bool() const
-	{
-		return !!ptr_;
-	}
-
-	wesnothd_connection& operator*()
-	{
-		return *ptr_;
-	}
-
-	const wesnothd_connection& operator*() const
-	{
-		return *ptr_;
-	}
-
-	wesnothd_connection* operator->()
-	{
-		return ptr_.get();
-	}
-
-	const wesnothd_connection* operator->() const
-	{
-		return ptr_.get();
-	}
-
-	wesnothd_connection* get() const
-	{
-		return ptr_.get();
-	}
-
-private:
-	std::shared_ptr<wesnothd_connection> ptr_;
-};