netplay: use WzConnectionProvider to create instances of descriptor…

… sets Get rid of `IDescriptorSet::create()` crutch and move the code to create IDescriptorSet:s to `WzConnectionProvider`, where it belongs. Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
Warzone2100 · Feb 23, 2025 · 7502514 · 7502514
1 parent 0b57a36
commit 7502514
Show file tree

Hide file tree

Showing 14 changed files with 95 additions and 90 deletions.
diff --git a/lib/netplay/descriptor_set.cpp b/lib/netplay/descriptor_set.cpp
diff --git a/lib/netplay/descriptor_set.h b/lib/netplay/descriptor_set.h
@@ -43,11 +43,6 @@ class IDescriptorSet
 {
 public:
 
-	/// <summary>
-	/// Factory method to create descriptor sets for a given poll event type (e.g., readable or writable check).
-	/// </summary>
-	static std::unique_ptr<IDescriptorSet> create(PollEventType eventType);
-
 	virtual ~IDescriptorSet() = default;
 
 	virtual bool add(IClientConnection* conn) = 0;

diff --git a/lib/netplay/netplay.cpp b/lib/netplay/netplay.cpp
@@ -1560,8 +1560,9 @@ int NETinit(bool bFirstCall)
 	NET_InitPlayers(true, true);
 
 	ConnectionProviderRegistry::Instance().Register(ConnectionProviderType::TCP_DIRECT);
-	ConnectionProviderRegistry::Instance().Get(ConnectionProviderType::TCP_DIRECT).initialize();
-	PendingWritesManager::instance().initialize();
+	auto& connProvider = ConnectionProviderRegistry::Instance().Get(ConnectionProviderType::TCP_DIRECT);
+	connProvider.initialize();
+	PendingWritesManager::instance().initialize(connProvider);
 
 	if (bFirstCall)
 	{

diff --git a/lib/netplay/pending_writes_manager.cpp b/lib/netplay/pending_writes_manager.cpp
@@ -24,6 +24,7 @@
 #include "lib/framework/frame.h" // for ASSERT
 #include "lib/netplay/descriptor_set.h"
 #include "lib/netplay/client_connection.h"
+#include "lib/netplay/wz_connection_provider.h"
 
 #include <system_error>
 
@@ -46,6 +47,7 @@ void PendingWritesManager::deinitialize()
 		return;
 	}
 	wzMutexLock(mtx_);
+	connProvider_ = nullptr;
 	stopRequested_ = true;
 	pendingWrites_.clear();
 	wzMutexUnlock(mtx_);
@@ -56,13 +58,14 @@ void PendingWritesManager::deinitialize()
 	thread_ = nullptr;
 }
 
-void PendingWritesManager::initialize()
+void PendingWritesManager::initialize(WzConnectionProvider& connProvider)
 {
 	if (thread_ != nullptr)
 	{
 		// No-op in case of a repeated `initialize()` call
 		return;
 	}
+	connProvider_ = &connProvider;
 	stopRequested_ = false;
 	mtx_ = wzMutexCreate();
 	sema_ = wzSemaphoreCreate(0);
@@ -126,7 +129,7 @@ void PendingWritesManager::threadImplFunction()
 	while (!stopRequested_)
 	{
 		static constexpr std::chrono::milliseconds WRITABLE_CHECK_TIMEOUT{ 50 };
-		static std::unique_ptr<IDescriptorSet> writableSet = IDescriptorSet::create(PollEventType::WRITABLE);
+		static std::unique_ptr<IDescriptorSet> writableSet = connProvider_->newDescriptorSet(PollEventType::WRITABLE);
 
 		// Check if we can write to some connections.
 		writableSet->clear();

diff --git a/lib/netplay/pending_writes_manager.h b/lib/netplay/pending_writes_manager.h
@@ -35,6 +35,7 @@ struct WZ_SEMAPHORE;
 
 class IClientConnection;
 class IDescriptorSet;
+class WzConnectionProvider;
 
 /// This is a wrapper function that acts as a proxy to `PendingWritesManager::threadImplFunction`.
 /// Argument is `PendingWritesManager*` cast to `void*`.
@@ -70,7 +71,7 @@ class PendingWritesManager
 
 	static PendingWritesManager& instance();
 
-	void initialize();
+	void initialize(WzConnectionProvider& connProvider);
 	void deinitialize();
 
 	template <typename Fn>
@@ -141,4 +142,5 @@ class PendingWritesManager
 	WZ_SEMAPHORE* sema_ = nullptr;
 	WZ_THREAD* thread_ = nullptr;
 	bool stopRequested_ = false;
+	WzConnectionProvider* connProvider_ = nullptr;
 };
diff --git a/lib/netplay/tcp/tcp_client_connection.cpp b/lib/netplay/tcp/tcp_client_connection.cpp
@@ -22,18 +22,20 @@
 #include "lib/framework/string_ext.h"
 #include "lib/netplay/error_categories.h"
 #include "lib/netplay/polling_util.h"
+#include "lib/netplay/wz_connection_provider.h"
 #include "lib/netplay/tcp/tcp_client_connection.h"
 #include "lib/netplay/tcp/netsocket.h"
 #include "lib/netplay/tcp/sock_error.h"
 
 namespace tcp
 {
 
-TCPClientConnection::TCPClientConnection(Socket* rawSocket)
+TCPClientConnection::TCPClientConnection(WzConnectionProvider& connProvider, Socket* rawSocket)
 	: socket_(rawSocket),
 	selfConnList_({ this }),
-	readAllDescriptorSet_(IDescriptorSet::create(PollEventType::READABLE)),
-	connStatusDescriptorSet_(IDescriptorSet::create(PollEventType::READABLE))
+	connProvider_(&connProvider),
+	readAllDescriptorSet_(connProvider_->newDescriptorSet(PollEventType::READABLE)),
+	connStatusDescriptorSet_(connProvider_->newDescriptorSet(PollEventType::READABLE))
 {
 	ASSERT(socket_ != nullptr, "Null socket passed to TCPClientConnection ctor");
 }

diff --git a/lib/netplay/tcp/tcp_client_connection.h b/lib/netplay/tcp/tcp_client_connection.h
@@ -23,6 +23,8 @@
 #include "lib/netplay/descriptor_set.h"
 #include "lib/netplay/tcp/netsocket.h" // for SOCKET
 
+class WzConnectionProvider;
+
 namespace tcp
 {
 
@@ -32,7 +34,7 @@ class TCPClientConnection : public IClientConnection
 {
 public:
 
-	explicit TCPClientConnection(Socket* rawSocket);
+	explicit TCPClientConnection(WzConnectionProvider& connProvider, Socket* rawSocket);
 	virtual ~TCPClientConnection() override;
 
 	virtual net::result<ssize_t> readAll(void* buf, size_t size, unsigned timeout) override;
@@ -63,6 +65,7 @@ class TCPClientConnection : public IClientConnection
 	// (like `readAll()` and `connectionStatus()`) to avoid extra
 	// memory allocations.
 	const std::vector<IClientConnection*> selfConnList_;
+	WzConnectionProvider* connProvider_;
 	std::unique_ptr<IDescriptorSet> readAllDescriptorSet_;
 	std::unique_ptr<IDescriptorSet> connStatusDescriptorSet_;
 };

diff --git a/lib/netplay/tcp/tcp_connection_poll_group.cpp b/lib/netplay/tcp/tcp_connection_poll_group.cpp
@@ -21,6 +21,7 @@
 #include "lib/netplay/tcp/tcp_client_connection.h"
 #include "lib/netplay/tcp/netsocket.h"
 #include "lib/netplay/polling_util.h"
+#include "lib/netplay/wz_connection_provider.h"
 #include "lib/framework/wzapp.h"
 #include "lib/framework/debug.h"
 
@@ -29,8 +30,9 @@
 namespace tcp
 {
 
-TCPConnectionPollGroup::TCPConnectionPollGroup()
-	: readableSet_(IDescriptorSet::create(PollEventType::READABLE))
+TCPConnectionPollGroup::TCPConnectionPollGroup(WzConnectionProvider& connProvider)
+	: connProvider_(&connProvider),
+	readableSet_(connProvider_->newDescriptorSet(PollEventType::READABLE))
 {}
 
 net::result<int> TCPConnectionPollGroup::checkConnectionsReadable(std::chrono::milliseconds timeout)

diff --git a/lib/netplay/tcp/tcp_connection_poll_group.h b/lib/netplay/tcp/tcp_connection_poll_group.h
@@ -26,6 +26,7 @@
 
 class IClientConnection;
 class IDescriptorSet;
+class WzConnectionProvider;
 
 namespace tcp
 {
@@ -34,7 +35,7 @@ class TCPConnectionPollGroup : public IConnectionPollGroup
 {
 public:
 
-	explicit TCPConnectionPollGroup();
+	explicit TCPConnectionPollGroup(WzConnectionProvider& connProvider);
 	virtual ~TCPConnectionPollGroup() override = default;
 
 	virtual net::result<int> checkConnectionsReadable(std::chrono::milliseconds timeout) override;
@@ -44,6 +45,7 @@ class TCPConnectionPollGroup : public IConnectionPollGroup
 private:
 
 	std::vector<IClientConnection*> conns_;
+	WzConnectionProvider* connProvider_;
 	// Pre-allocated descriptor set for `checkConnectionsReadable` operation
 	// to avoid extra memory allocations.
 	std::unique_ptr<IDescriptorSet> readableSet_;

diff --git a/lib/netplay/tcp/tcp_connection_provider.cpp b/lib/netplay/tcp/tcp_connection_provider.cpp
@@ -28,6 +28,12 @@
 #include "lib/netplay/open_connection_result.h"
 #include "lib/framework/wzapp.h"
 
+#ifdef WZ_OS_WIN
+# include "lib/netplay/tcp/select_descriptor_set.h"
+#else
+# include "lib/netplay/tcp/poll_descriptor_set.h"
+#endif
+
 namespace tcp
 {
 
@@ -58,7 +64,7 @@ net::result<IListenSocket*> TCPConnectionProvider::openListenSocket(uint16_t por
 	{
 		return tl::make_unexpected(res.error());
 	}
-	return new TCPListenSocket(res.value());
+	return new TCPListenSocket(*this, res.value());
 }
 
 net::result<IClientConnection*> TCPConnectionProvider::openClientConnectionAny(const IConnectionAddress& addr, unsigned timeout)
@@ -75,12 +81,48 @@ net::result<IClientConnection*> TCPConnectionProvider::openClientConnectionAny(c
 	{
 		return tl::make_unexpected(res.error());
 	}
-	return new TCPClientConnection(res.value());
+	return new TCPClientConnection(*this, res.value());
 }
 
 IConnectionPollGroup* TCPConnectionProvider::newConnectionPollGroup()
 {
-	return new TCPConnectionPollGroup();
+	return new TCPConnectionPollGroup(*this);
+}
+
+std::unique_ptr<IDescriptorSet> TCPConnectionProvider::newDescriptorSet(PollEventType eventType)
+{
+	// For now, use `select()` on Windows instead of `poll()` because of a bug in
+	// Windows versions prior to "Windows 10 2004", which can lead to `poll()`
+	// function timing out on socket connection errors instead of returning an error early.
+	//
+	// For more information on the bug, see: https://stackoverflow.com/questions/21653003/is-this-wsapoll-bug-for-non-blocking-sockets-fixed
+	// and also https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsapoll#remarks
+	switch (eventType)
+	{
+	case PollEventType::READABLE:
+	{
+#ifdef WZ_OS_WIN
+		IDescriptorSet* rawPtr = new tcp::SelectDescriptorSet<PollEventType::READABLE>();
+		return std::unique_ptr<IDescriptorSet>(rawPtr);
+#else
+		IDescriptorSet* rawPtr = new tcp::PollDescriptorSet<PollEventType::READABLE>();
+		return std::unique_ptr<IDescriptorSet>(rawPtr);
+#endif
+	}
+	case PollEventType::WRITABLE:
+	{
+#ifdef WZ_OS_WIN
+		IDescriptorSet* rawPtr = new tcp::SelectDescriptorSet<PollEventType::WRITABLE>();
+		return std::unique_ptr<IDescriptorSet>(rawPtr);
+#else
+		IDescriptorSet* rawPtr = new tcp::PollDescriptorSet<PollEventType::WRITABLE>();
+		return std::unique_ptr<IDescriptorSet>(rawPtr);
+#endif
+	}
+	default:
+		ASSERT(false, "Unexpected PollEventType value: %d", static_cast<int>(eventType));
+		return nullptr;
+	}
 }
 
 } // namespace tcp
diff --git a/lib/netplay/tcp/tcp_connection_provider.h b/lib/netplay/tcp/tcp_connection_provider.h
@@ -42,6 +42,8 @@ class TCPConnectionProvider final : public WzConnectionProvider
 	virtual net::result<IClientConnection*> openClientConnectionAny(const IConnectionAddress& addr, unsigned timeout) override;
 
 	virtual IConnectionPollGroup* newConnectionPollGroup() override;
+
+	virtual std::unique_ptr<IDescriptorSet> newDescriptorSet(PollEventType eventType) override;
 };
 
 } // namespace tcp
diff --git a/lib/netplay/tcp/tcp_listen_socket.cpp b/lib/netplay/tcp/tcp_listen_socket.cpp
@@ -26,8 +26,9 @@
 namespace tcp
 {
 
-TCPListenSocket::TCPListenSocket(Socket* rawSocket)
-	: listenSocket_(rawSocket)
+TCPListenSocket::TCPListenSocket(WzConnectionProvider& connProvider, Socket* rawSocket)
+	: listenSocket_(rawSocket),
+	connProvider_(&connProvider)
 {}
 
 TCPListenSocket::~TCPListenSocket()
@@ -50,7 +51,7 @@ IClientConnection* TCPListenSocket::accept()
 	{
 		return nullptr;
 	}
-	return new TCPClientConnection(s);
+	return new TCPClientConnection(*connProvider_, s);
 }
 
 IListenSocket::IPVersionsMask TCPListenSocket::supportedIpVersions() const

diff --git a/lib/netplay/tcp/tcp_listen_socket.h b/lib/netplay/tcp/tcp_listen_socket.h
@@ -23,6 +23,8 @@
 
 #include "lib/netplay/listen_socket.h"
 
+class WzConnectionProvider;
+
 namespace tcp
 {
 
@@ -32,7 +34,7 @@ class TCPListenSocket : public IListenSocket
 {
 public:
 
-	explicit TCPListenSocket(tcp::Socket* rawSocket);
+	explicit TCPListenSocket(WzConnectionProvider& connProvider, tcp::Socket* rawSocket);
 	virtual ~TCPListenSocket() override;
 
 	virtual IClientConnection* accept() override;
@@ -41,6 +43,7 @@ class TCPListenSocket : public IListenSocket
 private:
 
 	tcp::Socket* listenSocket_;
+	WzConnectionProvider* connProvider_;
 };
 
 } // namespace tcp