summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatric Stout <truebrain@openttd.org>2021-04-27 20:18:43 +0200
committerGitHub <noreply@github.com>2021-04-27 20:18:43 +0200
commit8fa53f543a5929bdbb12c8776ae9577594f9eba7 (patch)
treed5a06916b0281b45ddee6323dbab0b7f5ee6d861
parent015e3b412ebe709e1596179e86fde364cf19f52a (diff)
downloadopenttd-8fa53f543a5929bdbb12c8776ae9577594f9eba7.tar.xz
Change: [Network] lower TCP connect() timeout to 3s (#9112)
Currently we use default OS timeout for TCP connections, which is around 30s. 99% of the users will never notice this, but there are a few cases where this is an issue: - If you have a broken IPv6 connection, using Content Service is first tried over IPv6. Only after 30s it times out and tries IPv4. Nobody is waiting for that 30s. - Upcoming STUN support has several methods of establishing a connection between client and server. This requires feedback from connect() to know if any method worked (they have to be tried one by one). With 30s, this would take a very long time. What is good to mention, is that there is no good value here. Any value will have edge-cases where the experience is suboptimal. But with 3s we support most of the stable connections, and if it fails, the user can just retry. On the other side of the spectrum, with 30s, it means the user has no possibility to use the service. So worst case we annoy a few users with them having the retry vs annoying a few users which have no means of resolving the situation.
-rw-r--r--src/network/core/address.cpp48
-rw-r--r--src/network/core/os_abstraction.h16
2 files changed, 53 insertions, 11 deletions
diff --git a/src/network/core/address.cpp b/src/network/core/address.cpp
index fc19439e0..e53566c0b 100644
--- a/src/network/core/address.cpp
+++ b/src/network/core/address.cpp
@@ -14,6 +14,8 @@
#include "../../safeguards.h"
+static const int DEFAULT_CONNECT_TIMEOUT_SECONDS = 3; ///< Allow connect() three seconds to connect.
+
/**
* Get the hostname; in case it wasn't given the
* IPv4 dotted representation is given.
@@ -322,23 +324,47 @@ static SOCKET ConnectLoopProc(addrinfo *runp)
if (!SetNoDelay(sock)) DEBUG(net, 1, "[%s] setting TCP_NODELAY failed", type);
+ if (!SetNonBlocking(sock)) DEBUG(net, 0, "[%s] setting non-blocking mode failed", type);
+
int err = connect(sock, runp->ai_addr, (int)runp->ai_addrlen);
-#ifdef __EMSCRIPTEN__
- /* Emscripten is asynchronous, and as such a connect() is still in
- * progress by the time the call returns. */
- if (err != 0 && errno != EINPROGRESS)
-#else
- if (err != 0)
-#endif
- {
- DEBUG(net, 1, "[%s] could not connect %s socket: %s", type, family, NetworkGetLastErrorString());
+ if (err != 0 && NetworkGetLastError() != EINPROGRESS) {
+ DEBUG(net, 1, "[%s] could not connect to %s over %s: %s", type, address, family, NetworkGetLastErrorString());
closesocket(sock);
return INVALID_SOCKET;
}
- /* Connection succeeded */
- if (!SetNonBlocking(sock)) DEBUG(net, 0, "[%s] setting non-blocking mode failed", type);
+ fd_set write_fd;
+ struct timeval tv;
+
+ FD_ZERO(&write_fd);
+ FD_SET(sock, &write_fd);
+
+ /* Wait for connect() to either connect, timeout or fail. */
+ tv.tv_usec = 0;
+ tv.tv_sec = DEFAULT_CONNECT_TIMEOUT_SECONDS;
+ int n = select(FD_SETSIZE, NULL, &write_fd, NULL, &tv);
+ if (n < 0) {
+ DEBUG(net, 1, "[%s] could not connect to %s: %s", type, address, NetworkGetLastErrorString());
+ closesocket(sock);
+ return INVALID_SOCKET;
+ }
+
+ /* If no fd is selected, the timeout has been reached. */
+ if (n == 0) {
+ DEBUG(net, 1, "[%s] timed out while connecting to %s", type, address);
+ closesocket(sock);
+ return INVALID_SOCKET;
+ }
+
+ /* Retrieve last error, if any, on the socket. */
+ err = GetSocketError(sock);
+ if (err != 0) {
+ DEBUG(net, 1, "[%s] could not connect to %s: %s", type, address, NetworkGetErrorString(err));
+ closesocket(sock);
+ return INVALID_SOCKET;
+ }
+ /* Connection succeeded. */
DEBUG(net, 1, "[%s] connected to %s", type, address);
return sock;
diff --git a/src/network/core/os_abstraction.h b/src/network/core/os_abstraction.h
index 7af3fd163..9bd0e321f 100644
--- a/src/network/core/os_abstraction.h
+++ b/src/network/core/os_abstraction.h
@@ -33,6 +33,8 @@
#define EWOULDBLOCK WSAEWOULDBLOCK
#undef ECONNRESET
#define ECONNRESET WSAECONNRESET
+#undef EINPROGRESS
+#define EINPROGRESS WSAEWOULDBLOCK
const char *NetworkGetErrorString(int error);
@@ -230,6 +232,20 @@ static inline bool SetNoDelay(SOCKET d)
#endif
}
+/**
+ * Get the error from a socket, if any.
+ * @param d The socket to get the error from.
+ * @return The errno on the socket.
+ */
+static inline int GetSocketError(SOCKET d)
+{
+ int err;
+ socklen_t len = sizeof(err);
+ getsockopt(d, SOL_SOCKET, SO_ERROR, (char *)&err, &len);
+
+ return err;
+}
+
/* Make sure these structures have the size we expect them to be */
static_assert(sizeof(in_addr) == 4); ///< IPv4 addresses should be 4 bytes.
static_assert(sizeof(in6_addr) == 16); ///< IPv6 addresses should be 16 bytes.