netplay: move code for async opening of client connections to the bas…

…e `WzConnectionProvider` Provide the default implementation for `openClientConnectionAsync()` in `WzConnectionProvider`, which just spawns a new thread and piggibacks on the `resolveHost()` + `openClientConnectionAny()` combination. The `TCPConnectionProvider` is now simpler because it can use the default implementation from the base class. Signed-off-by: Pavel Solodovnikov <[email protected]>
Warzone2100 · Nov 16, 2024 · 08721fb · 08721fb
1 parent 6d1f975
commit 08721fb
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 86 deletions.
diff --git a/lib/netplay/tcp/tcp_connection_provider.cpp b/lib/netplay/tcp/tcp_connection_provider.cpp
@@ -78,80 +78,6 @@ net::result<IClientConnection*> TCPConnectionProvider::openClientConnectionAny(c
 	return new TCPClientConnection(res.value());
 }
 
-namespace
-{
-
-OpenConnectionResult socketOpenTCPConnectionSync(const char* host, uint32_t port)
-{
-	const auto hostsResult = resolveHost(host, port);
-	SocketAddress* hosts = hostsResult.value_or(nullptr);
-	if (hosts == nullptr)
-	{
-		const auto hostsErr = hostsResult.error();
-		const auto hostsErrMsg = hostsErr.message();
-		return OpenConnectionResult(hostsErr, astringf("Cannot resolve host \"%s\": [%d]: %s", host, hostsErr.value(), hostsErrMsg.c_str()));
-	}
-
-	auto sockResult = socketOpenAny(hosts, 15000);
-	Socket* client_transient_socket = sockResult.value_or(nullptr);
-	deleteSocketAddress(hosts);
-	hosts = nullptr;
-
-	if (client_transient_socket == nullptr)
-	{
-		const auto errValue = sockResult.error();
-		const auto errMsg = errValue.message();
-		return OpenConnectionResult(errValue, astringf("Cannot connect to [%s]:%d, [%d]:%s", host, port, errValue.value(), errMsg.c_str()));
-	}
-
-	return OpenConnectionResult(new TCPClientConnection(client_transient_socket));
-}
-
-struct OpenConnectionRequest
-{
-	std::string host;
-	uint32_t port = 0;
-	OpenConnectionToHostResultCallback callback;
-};
-
-static int openDirectTCPConnectionAsyncImpl(void* data)
-{
-	OpenConnectionRequest* pRequestInfo = (OpenConnectionRequest*)data;
-	if (!pRequestInfo)
-	{
-		return 1;
-	}
-
-	pRequestInfo->callback(socketOpenTCPConnectionSync(pRequestInfo->host.c_str(), pRequestInfo->port));
-	delete pRequestInfo;
-	return 0;
-}
-
-} // anonymous namespace
-
-bool TCPConnectionProvider::openClientConnectionAsync(const std::string& host, uint32_t port, OpenConnectionToHostResultCallback callback)
-{
-	// spawn background thread to handle this
-	auto pRequest = new OpenConnectionRequest();
-	pRequest->host = host;
-	pRequest->port = port;
-	pRequest->callback = callback;
-
-	WZ_THREAD* pOpenConnectionThread = wzThreadCreate(openDirectTCPConnectionAsyncImpl, pRequest);
-	if (pOpenConnectionThread == nullptr)
-	{
-		debug(LOG_ERROR, "Failed to create thread for opening connection");
-		delete pRequest;
-		return false;
-	}
-
-	wzThreadDetach(pOpenConnectionThread);
-	// the thread handles deleting pRequest
-	pOpenConnectionThread = nullptr;
-
-	return true;
-}
-
 IConnectionPollGroup* TCPConnectionProvider::newConnectionPollGroup()
 {
 	auto* sset = allocSocketSet();

diff --git a/lib/netplay/tcp/tcp_connection_provider.h b/lib/netplay/tcp/tcp_connection_provider.h
@@ -40,7 +40,6 @@ class TCPConnectionProvider final : public WzConnectionProvider
 	virtual net::result<IListenSocket*> openListenSocket(uint16_t port) override;
 
 	virtual net::result<IClientConnection*> openClientConnectionAny(const IConnectionAddress& addr, unsigned timeout) override;
-	virtual bool openClientConnectionAsync(const std::string& host, uint32_t port, OpenConnectionToHostResultCallback callback) override;
 
 	virtual IConnectionPollGroup* newConnectionPollGroup() override;
 };

diff --git a/lib/netplay/wz_connection_provider.cpp b/lib/netplay/wz_connection_provider.cpp
@@ -0,0 +1,93 @@
+/*
+	This file is part of Warzone 2100.
+	Copyright (C) 2024  Warzone 2100 Project
+
+	Warzone 2100 is free software; you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation; either version 2 of the License, or
+	(at your option) any later version.
+
+	Warzone 2100 is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with Warzone 2100; if not, write to the Free Software
+	Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+*/
+
+#include "wz_connection_provider.h"
+
+#include "lib/framework/wzapp.h"
+
+namespace
+{
+
+OpenConnectionResult openClientConnectionSyncImpl(const char* host, uint32_t port, std::chrono::milliseconds timeout, WzConnectionProvider* connProvider)
+{
+	auto addrResult = connProvider->resolveHost(host, port);
+	if (!addrResult.has_value())
+	{
+		const auto hostsErr = addrResult.error();
+		const auto hostsErrMsg = hostsErr.message();
+		return OpenConnectionResult(hostsErr, astringf("Cannot resolve host \"%s\": [%d]: %s", host, hostsErr.value(), hostsErrMsg.c_str()));
+	}
+	auto connRes = connProvider->openClientConnectionAny(*addrResult.value(), timeout.count());
+	if (!connRes.has_value())
+	{
+		const auto connErr = connRes.error();
+		const auto connErrMsg = connErr.message();
+		return OpenConnectionResult(connErr, astringf("Cannot resolve host \"%s\": [%d]: %s", host, connErr.value(), connErrMsg.c_str()));
+	}
+	return OpenConnectionResult(connRes.value());
+}
+
+struct OpenConnectionRequest
+{
+	std::string host;
+	uint32_t port = 0;
+	std::chrono::milliseconds timeout{ 15000 };
+	OpenConnectionToHostResultCallback callback;
+	WzConnectionProvider* connProvider;
+};
+
+int openDirectConnectionAsyncImpl(void* data)
+{
+	OpenConnectionRequest* pRequestInfo = (OpenConnectionRequest*)data;
+	if (!pRequestInfo)
+	{
+		return 1;
+	}
+	pRequestInfo->callback(openClientConnectionSyncImpl(
+		pRequestInfo->host.c_str(),
+		pRequestInfo->port,
+		pRequestInfo->timeout,
+		pRequestInfo->connProvider));
+	delete pRequestInfo;
+	return 0;
+}
+
+} // anonymous namespace
+
+bool WzConnectionProvider::openClientConnectionAsync(const std::string& host, uint32_t port, std::chrono::milliseconds timeout, OpenConnectionToHostResultCallback callback)
+{
+	// spawn background thread to handle this
+	auto pRequest = new OpenConnectionRequest();
+	pRequest->host = host;
+	pRequest->port = port;
+	pRequest->timeout = timeout;
+	pRequest->callback = callback;
+	pRequest->connProvider = this;
+	WZ_THREAD* pOpenConnectionThread = wzThreadCreate(openDirectConnectionAsyncImpl, pRequest);
+	if (pOpenConnectionThread == nullptr)
+	{
+		debug(LOG_ERROR, "Failed to create thread for opening connection");
+		delete pRequest;
+		return false;
+	}
+	wzThreadDetach(pOpenConnectionThread);
+	// the thread handles deleting pRequest
+	pOpenConnectionThread = nullptr;
+	return true;
+}
diff --git a/lib/netplay/wz_connection_provider.h b/lib/netplay/wz_connection_provider.h
@@ -20,7 +20,9 @@
 #pragma once
 
 #include <stdint.h>
+#include <chrono>
 #include <memory>
+#include <string>
 
 #include "lib/netplay/connection_address.h"
 #include "lib/netplay/net_result.h"
@@ -72,9 +74,10 @@ class WzConnectionProvider
 	/// <param name="timeout">Timeout in milliseconds.</param>
 	virtual net::result<IClientConnection*> openClientConnectionAny(const IConnectionAddress& addr, unsigned timeout) = 0;
 	/// <summary>
-	/// Async variant of `openClientConnectionAny()`.
+	/// Async variant of `openClientConnectionAny()` with the default implementation, which
+	/// spawns a new thread and piggybacks on the `resolveHost()` and `openClientConnectionAny()` combination.
 	/// </summary>
-	virtual bool openClientConnectionAsync(const std::string& host, uint32_t port, OpenConnectionToHostResultCallback callback) = 0;
+	virtual bool openClientConnectionAsync(const std::string& host, uint32_t port, std::chrono::milliseconds timeout, OpenConnectionToHostResultCallback callback);
 	/// <summary>
 	/// Create a group for polling client connections.
 	/// </summary>

diff --git a/src/screens/joiningscreen.cpp b/src/screens/joiningscreen.cpp
@@ -1239,16 +1239,19 @@ void WzJoiningGameScreen_HandlerRoot::attemptToOpenConnection(size_t connectionI
 			}
 			auto weakSelf = std::weak_ptr<WzJoiningGameScreen_HandlerRoot>(std::dynamic_pointer_cast<WzJoiningGameScreen_HandlerRoot>(shared_from_this()));
 
+			constexpr std::chrono::milliseconds CLIENT_OPEN_ASYNC_TIMEOUT{ 15000 }; // Default timeout of 15s
+
 			auto& connProvider = ConnectionProviderRegistry::Instance().Get(ConnectionProviderType::TCP_DIRECT);
-			connProvider.openClientConnectionAsync(description.host, description.port, [weakSelf, connectionIdx](OpenConnectionResult&& result) {
-				auto strongSelf = weakSelf.lock();
-				if (!strongSelf)
-				{
-					// background thread ultimately returned after the requester has gone away (join was cancelled?) - just return
-					return;
-				}
-				strongSelf->processOpenConnectionResultOnMainThread(connectionIdx, std::move(result));
-			});
+			connProvider.openClientConnectionAsync(description.host, description.port, CLIENT_OPEN_ASYNC_TIMEOUT,
+				[weakSelf, connectionIdx](OpenConnectionResult&& result) {
+					auto strongSelf = weakSelf.lock();
+					if (!strongSelf)
+					{
+						// background thread ultimately returned after the requester has gone away (join was cancelled?) - just return
+						return;
+					}
+					strongSelf->processOpenConnectionResultOnMainThread(connectionIdx, std::move(result));
+				});
 			break;
 	}
 	updateJoiningStatus(_("Establishing connection with host"));