(svn r15163) -Change/Fix: use a non-blocking method to resolve the hostname and connect to game servers.

source.list

@@ -768,6 +768,8 @@ misc/str.hpp
 misc/strapi.hpp
 
 # Network Core
+network/core/address.cpp
+network/core/address.h
 network/core/config.h
 network/core/core.cpp
 network/core/core.h
@@ -779,6 +781,7 @@ network/core/packet.cpp
 network/core/packet.h
 network/core/tcp.cpp
 network/core/tcp.h
+network/core/tcp_connect.cpp
 network/core/tcp_content.cpp
 network/core/tcp_content.h
 network/core/tcp_game.cpp

src/network/core/address.cpp

@@ -0,0 +1,30 @@
+/* $Id$ */
+
+/** @file core/address.cpp Implementation of the address. */
+
+#include "../../stdafx.h"
+
+#ifdef ENABLE_NETWORK
+
+#include "address.h"
+#include "host.h"
+
+const char *NetworkAddress::GetHostname() const
+{
+	if (this->hostname != NULL) return this->hostname;
+
+	in_addr addr;
+	addr.s_addr = this->ip;
+	return inet_ntoa(addr);
+}
+
+uint32 NetworkAddress::GetIP()
+{
+	if (!this->resolved) {
+		this->ip = NetworkResolveHost(this->hostname);
+		this->resolved = true;
+	}
+	return this->ip;
+}
+
+#endif /* ENABLE_NETWORK */

src/network/core/address.h

@@ -0,0 +1,103 @@
+/* $Id$ */
+
+/** @file core/address.h Wrapper for network addresses. */
+
+#ifndef NETWORK_ADDRESS_H
+#define NETWORK_ADDRESS_H
+
+#ifdef ENABLE_NETWORK
+
+#include "os_abstraction.h"
+
+/**
+ * Wrapper for (un)resolved network addresses; there's no reason to transform
+ * a numeric IP to a string and then back again to pass it to functions. It
+ * furthermore allows easier delaying of the hostname lookup.
+ */
+class NetworkAddress {
+private:
+	bool resolved;  ///< Has the IP address been resolved
+	char *hostname; ///< The hostname, NULL if there isn't one
+	uint32 ip;      ///< The resolved IP address
+	uint16 port;    ///< The port associated with the address
+
+public:
+	/**
+	 * Create a network address based on a resolved IP and port
+	 * @param ip the resolved ip
+	 * @param port the port
+	 */
+	NetworkAddress(in_addr_t ip, uint16 port) :
+		resolved(true),
+		hostname(NULL),
+		ip(ip),
+		port(port)
+	{
+	}
+
+	/**
+	 * Create a network address based on a unresolved host and port
+	 * @param ip the unresolved hostname
+	 * @param port the port
+	 */
+	NetworkAddress(const char *hostname, uint16 port) :
+		resolved(false),
+		hostname(strdup(hostname)),
+		ip(0),
+		port(port)
+	{
+	}
+
+	/**
+	 * Make a clone of another address
+	 * @param address the address to clone
+	 */
+	NetworkAddress(const NetworkAddress &address) :
+		resolved(address.resolved),
+		hostname(address.hostname == NULL ? NULL : strdup(address.hostname)),
+		ip(address.ip),
+		port(address.port)
+	{
+	}
+
+	/** Clean up our mess */
+	~NetworkAddress()
+	{
+		free(hostname);
+	}
+
+	/**
+	 * Get the hostname; in case it wasn't given the
+	 * IPv4 dotted representation is given.
+	 * @return the hostname
+	 */
+	const char *GetHostname() const;
+
+	/**
+	 * Get the IP address. If the IP has not been resolved yet this will resolve
+	 * it possibly blocking this function for a while
+	 * @return the IP address
+	 */
+	uint32 GetIP();
+
+	/**
+	 * Get the port
+	 * @return the port
+	 */
+	uint16 GetPort() const
+	{
+		return this->port;
+	}
+
+	/**
+	 * Check whether the IP address has been resolved already
+	 * @return true iff the port has been resolved
+	 */
+	bool IsResolved() const
+	{
+		return this->resolved;
+	}
+};
+
+#endif /* ENABLE_NETWORK */
+#endif /* NETWORK_ADDRESS_H */

src/network/core/tcp.cpp

@@ -201,5 +201,4 @@ bool NetworkTCPSocketHandler::IsPacketQueueEmpty()
 	return this->packet_queue == NULL;
 }
 
-
 #endif /* ENABLE_NETWORK */

src/network/core/tcp.h

@@ -10,6 +10,7 @@
 #ifdef ENABLE_NETWORK
 
 #include "os_abstraction.h"
+#include "address.h"
 #include "core.h"
 #include "packet.h"
 
@@ -31,6 +32,62 @@ public:
 	~NetworkTCPSocketHandler();
 };
 
+/**
+ * "Helper" class for creating TCP connections in a non-blocking manner
+ */
+class TCPConnecter {
+private:
+	class ThreadObject *thread; ///< Thread used to create the TCP connection
+	bool connected;             ///< Whether we succeeded in making the connection
+	bool aborted;               ///< Whether we bailed out (i.e. connection making failed)
+	bool killed;                ///< Whether we got killed
+	SOCKET sock;                ///< The socket we're connecting with
+
+	/** The actual connection function */
+	void Connect();
+
+	/**
+	 * Entry point for the new threads.
+	 * @param param the TCPConnecter instance to call Connect on.
+	 */
+	static void ThreadEntry(void *param);
+
+protected:
+	/** Address we're connecting to */
+	NetworkAddress address;
+
+public:
+	/**
+	 * Create a new connecter for the given address
+	 * @param address the (un)resolved address to connect to
+	 */
+	TCPConnecter(const NetworkAddress &address);
+	/** Silence the warnings */
+	virtual ~TCPConnecter() {}
+
+	/**
+	 * Callback when the connection succeeded.
+	 * @param s the socket that we opened
+	 */
+	virtual void OnConnect(SOCKET s) {}
+
+	/**
+	 * Callback for when the connection attempt failed.
+	 */
+	virtual void OnFailure() {}
+
+	/**
+	 * Check whether we need to call the callback, i.e. whether we
+	 * have connected or aborted and call the appropriate callback
+	 * for that. It's done this way to ease on the locking that
+	 * would otherwise be needed everywhere.
+	 */
+	static void CheckCallbacks();
+
+	/** Kill all connection attempts. */
+	static void KillAll();
+};
+
 #endif /* ENABLE_NETWORK */
 
 #endif /* NETWORK_CORE_TCP_H */

src/network/core/tcp_connect.cpp

@@ -0,0 +1,99 @@
+/* $Id$ */
+
+/**
+ * @file tcp_connect.cpp Basic functions to create connections without blocking.
+ */
+
+#ifdef ENABLE_NETWORK
+
+#include "../../stdafx.h"
+#include "../../debug.h"
+#include "../../core/smallvec_type.hpp"
+#include "../../thread.h"
+
+#include "tcp.h"
+
+/** List of connections that are currently being created */
+static SmallVector<TCPConnecter *,  1> _tcp_connecters;
+
+TCPConnecter::TCPConnecter(const NetworkAddress &address) :
+	connected(false),
+	aborted(false),
+	killed(false),
+	sock(INVALID_SOCKET),
+	address(address)
+{
+	*_tcp_connecters.Append() = this;
+	if (!ThreadObject::New(TCPConnecter::ThreadEntry, this, &this->thread)) {
+		this->Connect();
+	}
+}
+
+void TCPConnecter::Connect()
+{
+	DEBUG(net, 1, "Connecting to %s %d", address.GetHostname(), address.GetPort());
+
+	this->sock = socket(AF_INET, SOCK_STREAM, 0);
+	if (this->sock == INVALID_SOCKET) {
+		this->aborted = true;
+		return;
+	}
+
+	if (!SetNoDelay(this->sock)) DEBUG(net, 1, "Setting TCP_NODELAY failed");
+
+	struct sockaddr_in sin;
+	sin.sin_family = AF_INET;
+	sin.sin_addr.s_addr = address.GetIP();
+	sin.sin_port = htons(address.GetPort());
+
+	/* We failed to connect for which reason what so ever */
+	if (connect(this->sock, (struct sockaddr*) &sin, sizeof(sin)) != 0) {
+		closesocket(this->sock);
+		this->sock = INVALID_SOCKET;
+		this->aborted = true;
+		return;
+	}
+
+	if (!SetNonBlocking(this->sock)) DEBUG(net, 0, "Setting non-blocking mode failed");
+
+	this->connected = true;
+}
+
+
+/* static */ void TCPConnecter::ThreadEntry(void *param)
+{
+	static_cast<TCPConnecter*>(param)->Connect();
+}
+
+/* static */ void TCPConnecter::CheckCallbacks()
+{
+	for (TCPConnecter **iter = _tcp_connecters.Begin(); iter < _tcp_connecters.End(); /* nothing */) {
+		TCPConnecter *cur = *iter;
+		if ((cur->connected || cur->aborted) && cur->killed) {
+			_tcp_connecters.Erase(iter);
+			if (cur->sock != INVALID_SOCKET) closesocket(cur->sock);
+			delete cur;
+			continue;
+		}
+		if (cur->connected) {
+			_tcp_connecters.Erase(iter);
+			cur->OnConnect(cur->sock);
+			delete cur;
+			continue;
+		}
+		if (cur->aborted) {
+			_tcp_connecters.Erase(iter);
+			cur->OnFailure();
+			delete cur;
+			continue;
+		}
+		iter++;
+	}
+}
+
+/* static */ void TCPConnecter::KillAll()
+{
+	for (TCPConnecter **iter = _tcp_connecters.Begin(); iter != _tcp_connecters.End(); iter++) (*iter)->killed = true;
+}
+
+#endif /* ENABLE_NETWORK */

src/network/network.cpp

@@ -72,7 +72,6 @@ uint32 _network_server_bind_ip;
 uint32 _sync_seed_1, _sync_seed_2;
 uint32 _sync_frame;
 bool _network_first_time;
-uint32 _network_last_host_ip;
 bool _network_udp_server;
 uint16 _network_udp_broadcast;
 uint8 _network_advertise_retries;
@@ -240,12 +239,6 @@ static void NetworkError(StringID error_string)
 	_switch_mode_errorstr = error_string;
 }
 
-static void ClientStartError(const char *error)
-{
-	DEBUG(net, 0, "[client] could not start network: %s",error);
-	NetworkError(STR_NETWORK_ERR_CLIENT_START);
-}
-
 static void ServerStartError(const char *error)
 {
 	DEBUG(net, 0, "[server] could not start network: %s",error);
@@ -450,41 +443,6 @@ void NetworkCloseClient(NetworkClientSocket *cs)
 	CheckMinActiveClients();
 }
 
-// A client wants to connect to a server
-static bool NetworkConnect(NetworkAddress address)
-{
-	SOCKET s;
-	struct sockaddr_in sin;
-
-	DEBUG(net, 1, "Connecting to %s %d", address.GetHostname(), address.GetPort());
-
-	s = socket(AF_INET, SOCK_STREAM, 0);
-	if (s == INVALID_SOCKET) {
-		ClientStartError("socket() failed");
-		return false;
-	}
-
-	if (!SetNoDelay(s)) DEBUG(net, 1, "Setting TCP_NODELAY failed");
-
-	sin.sin_family = AF_INET;
-	sin.sin_addr.s_addr = address.GetIP();
-	sin.sin_port = htons(address.GetPort());
-	_network_last_host_ip = sin.sin_addr.s_addr;
-
-	/* We failed to connect for which reason what so ever */
-	if (connect(s, (struct sockaddr*) &sin, sizeof(sin)) != 0) return false;
-
-	if (!SetNonBlocking(s)) DEBUG(net, 0, "Setting non-blocking mode failed"); // XXX should this be an error?
-
-	// in client mode, only the first client field is used. it's pointing to the server.
-	NetworkAllocClient(s);
-
-	_network_join_status = NETWORK_JOIN_STATUS_CONNECTING;
-	ShowJoinStatusWindow();
-
-	return true;
-}
-
 // For the server, to accept new clients
 static void NetworkAcceptClients()
 {
@@ -637,6 +595,8 @@ static void NetworkClose()
 	}
 	NetworkUDPCloseAll();
 
+	TCPConnecter::KillAll();
+
 	_networking = false;
 	_network_server = false;
 
@@ -661,6 +621,24 @@ static void NetworkInitialize()
 	_network_reconnect = 0;
 }
 
+/** Non blocking connection create to query servers */
+class TCPQueryConnecter : TCPConnecter {
+public:
+	TCPQueryConnecter(const NetworkAddress &address) : TCPConnecter(address) {}
+
+	virtual void OnFailure()
+	{
+		NetworkDisconnect();
+	}
+
+	virtual void OnConnect(SOCKET s)
+	{
+		_networking = true;
+		NetworkAllocClient(s);
+		SEND_COMMAND(PACKET_CLIENT_COMPANY_INFO)();
+	}
+};
+
 // Query a server to fetch his game-info
 //  If game_info is true, only the gameinfo is fetched,
 //   else only the client_info is fetched
@@ -671,15 +649,7 @@ void NetworkTCPQueryServer(NetworkAddress address)
 	NetworkDisconnect();
 	NetworkInitialize();
 
-	// Try to connect
+	new TCPQueryConnecter(address);
-	_networking = NetworkConnect(address);
-
-	// We are connected
-	if (_networking) {
-		SEND_COMMAND(PACKET_CLIENT_COMPANY_INFO)();
-	} else { // No networking, close everything down again
-		NetworkDisconnect();
-	}
 }
 
 /* Validates an address entered as a string and adds the server to
@@ -726,6 +696,26 @@ void NetworkRebuildHostList()
 	}
 }
 
+/** Non blocking connection create to actually connect to servers */
+class TCPClientConnecter : TCPConnecter {
+public:
+	TCPClientConnecter(const NetworkAddress &address) : TCPConnecter(address) {}
+
+	virtual void OnFailure()
+	{
+		NetworkError(STR_NETWORK_ERR_NOCONNECTION);
+	}
+
+	virtual void OnConnect(SOCKET s)
+	{
+		_networking = true;
+		NetworkAllocClient(s);
+		IConsoleCmdExec("exec scripts/on_client.scr 0");
+		NetworkClient_Connected();
+	}
+};
+
+
 // Used by clients, to connect to a server
 void NetworkClientConnectGame(NetworkAddress address)
 {
@@ -739,17 +729,10 @@ void NetworkClientConnectGame(NetworkAddress address)
 	NetworkDisconnect();
 	NetworkInitialize();
 
-	// Try to connect
+	_network_join_status = NETWORK_JOIN_STATUS_CONNECTING;
-	_networking = NetworkConnect(address);
+	ShowJoinStatusWindow();
 
-	// We are connected
+	new TCPClientConnecter(address);
-	if (_networking) {
-		IConsoleCmdExec("exec scripts/on_client.scr 0");
-		NetworkClient_Connected();
-	} else {
-		// Connecting failed
-		NetworkError(STR_NETWORK_ERR_NOCONNECTION);
-	}
 }
 
 static void NetworkInitGameInfo()
@@ -964,6 +947,7 @@ static bool NetworkDoClientLoop()
 void NetworkUDPGameLoop()
 {
 	NetworkContentLoop();
+	TCPConnecter::CheckCallbacks();
 
 	if (_network_udp_server) {
 		_udp_server_socket->ReceivePackets();
@@ -1154,24 +1138,6 @@ bool IsNetworkCompatibleVersion(const char *other)
 	return strncmp(_openttd_revision, other, NETWORK_REVISION_LENGTH - 1) == 0;
 }
 
-const char *NetworkAddress::GetHostname() const
-{
-	if (this->hostname != NULL) return this->hostname;
-
-	in_addr addr;
-	addr.s_addr = this->ip;
-	return inet_ntoa(addr);
-}
-
-uint32 NetworkAddress::GetIP()
-{
-	if (!this->resolved) {
-		this->ip = NetworkResolveHost(this->hostname);
-		this->resolved = true;
-	}
-	return this->ip;
-}
-
 #endif /* ENABLE_NETWORK */
 
 /* NOTE: this variable needs to be always available */

src/network/network_func.h

@@ -7,6 +7,7 @@
 
 #ifdef ENABLE_NETWORK
 
+#include "core/address.h"
 #include "network_type.h"
 #include "../console_type.h"
 

src/network/network_internal.h

@@ -114,7 +114,6 @@ extern uint8 _network_join_waiting;
 extern uint32 _network_join_bytes;
 extern uint32 _network_join_bytes_total;
 
-extern uint32 _network_last_host_ip;
 extern uint8 _network_reconnect;
 
 extern bool _network_udp_server;

src/network/network_type.h

@@ -11,7 +11,6 @@
 #include "../economy_type.h"
 #include "core/config.h"
 #include "core/game.h"
-#include "core/os_abstraction.h"
 
 enum {
 	/** How many clients can we have */
@@ -115,95 +114,5 @@ enum NetworkErrorCode {
 	NETWORK_ERROR_FULL,
 };
 
-/**
- * Wrapper for (un)resolved network addresses; there's no reason to transform
- * a numeric IP to a string and then back again to pass it to functions. It
- * furthermore allows easier delaying of the hostname lookup.
- */
-class NetworkAddress {
-private:
-	bool resolved;  ///< Has the IP address been resolved
-	char *hostname; ///< The hostname, NULL if there isn't one
-	uint32 ip;      ///< The resolved IP address
-	uint16 port;    ///< The port associated with the address
-
-public:
-	/**
-	 * Create a network address based on a resolved IP and port
-	 * @param ip the resolved ip
-	 * @param port the port
-	 */
-	NetworkAddress(in_addr_t ip, uint16 port) :
-		resolved(true),
-		hostname(NULL),
-		ip(ip),
-		port(port)
-	{
-	}
-
-	/**
-	 * Create a network address based on a unresolved host and port
-	 * @param ip the unresolved hostname
-	 * @param port the port
-	 */
-	NetworkAddress(const char *hostname, uint16 port) :
-		resolved(false),
-		hostname(strdup(hostname)),
-		ip(0),
-		port(port)
-	{
-	}
-
-	/**
-	 * Make a clone of another address
-	 * @param address the address to clone
-	 */
-	NetworkAddress(const NetworkAddress &address) :
-		resolved(address.resolved),
-		hostname(address.hostname == NULL ? NULL : strdup(address.hostname)),
-		ip(address.ip),
-		port(address.port)
-	{
-	}
-
-	/** Clean up our mess */
-	~NetworkAddress()
-	{
-		free(hostname);
-	}
-
-	/**
-	 * Get the hostname; in case it wasn't given the
-	 * IPv4 dotted representation is given.
-	 * @return the hostname
-	 */
-	const char *GetHostname() const;
-
-	/**
-	 * Get the IP address. If the IP has not been resolved yet this will resolve
-	 * it possibly blocking this function for a while
-	 * @return the IP address
-	 */
-	uint32 GetIP();
-
-	/**
-	 * Get the port
-	 * @return the port
-	 */
-	uint16 GetPort() const
-	{
-		return this->port;
-	}
-
-	/**
-	 * Check whether the IP address has been resolved already
-	 * @return true iff the port has been resolved
-	 */
-	bool IsResolved() const
-	{
-		return this->resolved;
-	}
-};
-
 #endif /* ENABLE_NETWORK */
 #endif /* NETWORK_TYPE_H */

src/network/network_udp.cpp

@@ -14,8 +14,8 @@
 #include "../date_func.h"
 #include "../map_func.h"
 #include "network_gamelist.h"
-#include "network_udp.h"
 #include "network_internal.h"
+#include "network_udp.h"
 #include "core/host.h"
 #include "../variables.h"
 #include "../newgrf_config.h"