From 9d9d7674b63127edd67379081783231355a771bb Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 29 Nov 2017 12:47:50 +0000 Subject: [PATCH] Fix socket timeouts to work consistently on POSIX and Win32 * This should prevent any bugs with socket communication getting out of sync or breaking from completely locking up the UI. --- renderdoc/os/os_specific.h | 5 +- renderdoc/os/posix/posix_network.cpp | 79 ++++++++++++++++--- renderdoc/os/win32/win32_network.cpp | 110 +++++++++++++++++++++------ 3 files changed, 157 insertions(+), 37 deletions(-) diff --git a/renderdoc/os/os_specific.h b/renderdoc/os/os_specific.h index 0fe24ab7a..111ca53b2 100644 --- a/renderdoc/os/os_specific.h +++ b/renderdoc/os/os_specific.h @@ -133,12 +133,14 @@ namespace Network class Socket { public: - Socket(ptrdiff_t s) : socket(s) {} + Socket(ptrdiff_t s) : socket(s), timeoutMS(5000) {} ~Socket(); void Shutdown(); bool Connected() const; + uint32_t GetTimeout() const { return timeoutMS; } + void SetTimeout(uint32_t milliseconds) { timeoutMS = milliseconds; } Socket *AcceptClient(bool wait); uint32_t GetRemoteIP() const; @@ -151,6 +153,7 @@ public: private: ptrdiff_t socket; + uint32_t timeoutMS; }; Socket *CreateServerSocket(const char *addr, uint16_t port, int queuesize); diff --git a/renderdoc/os/posix/posix_network.cpp b/renderdoc/os/posix/posix_network.cpp index 82d967eed..84541b173 100644 --- a/renderdoc/os/posix/posix_network.cpp +++ b/renderdoc/os/posix/posix_network.cpp @@ -39,6 +39,33 @@ using std::string; +// because strerror_r is a complete mess... +static std::string errno_string(int err) +{ + switch(err) + { +#if EAGAIN != EWOULDBLOCK + case EAGAIN: +#endif + case EWOULDBLOCK: return "EWOULDBLOCK: Operation would block."; + case EINVAL: return "EINVAL: Invalid argument."; + case EADDRINUSE: return "EADDRINUSE: Address already in use."; + case ECONNRESET: return "ECONNRESET: A connection was forcibly closed by a peer."; + case EINPROGRESS: return "EINPROGRESS: Operation now in progress."; + case EINTR: + return "EINTR: The function was interrupted by a signal that was caught, before any data was " + "available."; + case ETIMEDOUT: return "ETIMEDOUT: A socket operation timed out."; + case ECONNABORTED: return "ECONNABORTED: A connection has been aborted."; + case ECONNREFUSED: return "ECONNREFUSED: A connection was refused."; + case EHOSTDOWN: return "EHOSTDOWN: Host is down."; + case EHOSTUNREACH: return "EHOSTUNREACH: No route to host."; + default: break; + } + + return StringFormat::Fmt("Unknown error %d", err); +} + namespace Network { void Init() @@ -100,7 +127,7 @@ Socket *Socket::AcceptClient(bool wait) if(err != EWOULDBLOCK && err != EAGAIN) { - RDCWARN("accept: %d", err); + RDCWARN("accept: %s", errno_string(err).c_str()); Shutdown(); } @@ -122,6 +149,15 @@ bool Socket::SendDataBlocking(const void *buf, uint32_t length) int flags = fcntl(socket, F_GETFL, 0); fcntl(socket, F_SETFL, flags & ~O_NONBLOCK); + timeval oldtimeout = {0}; + socklen_t len = sizeof(oldtimeout); + getsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (char *)&oldtimeout, &len); + + timeval timeout = {0}; + timeout.tv_sec = (timeoutMS / 1000); + timeout.tv_usec = (timeoutMS % 1000) * 1000; + setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (const char *)&timeout, sizeof(timeout)); + while(sent < length) { int ret = send(socket, src, length - sent, 0); @@ -132,11 +168,13 @@ bool Socket::SendDataBlocking(const void *buf, uint32_t length) if(err == EWOULDBLOCK || err == EAGAIN) { - ret = 0; + RDCWARN("Timeout in send"); + Shutdown(); + return false; } else { - RDCWARN("send: %d", err); + RDCWARN("send: %s", errno_string(err).c_str()); Shutdown(); return false; } @@ -149,6 +187,8 @@ bool Socket::SendDataBlocking(const void *buf, uint32_t length) flags = fcntl(socket, F_GETFL, 0); fcntl(socket, F_SETFL, flags | O_NONBLOCK); + setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (const char *)&oldtimeout, sizeof(oldtimeout)); + RDCASSERT(sent == length); return true; @@ -174,7 +214,7 @@ bool Socket::IsRecvDataWaiting() } else { - RDCWARN("recv: %d", err); + RDCWARN("recv: %s", errno_string(err).c_str()); Shutdown(); return false; } @@ -206,7 +246,7 @@ bool Socket::RecvDataNonBlocking(void *buf, uint32_t &length) } else { - RDCWARN("recv: %d", err); + RDCWARN("recv: %s", errno_string(err).c_str()); Shutdown(); return false; } @@ -227,6 +267,15 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) int flags = fcntl(socket, F_GETFL, 0); fcntl(socket, F_SETFL, flags & ~O_NONBLOCK); + timeval oldtimeout = {0}; + socklen_t len = sizeof(oldtimeout); + getsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (char *)&oldtimeout, &len); + + timeval timeout = {0}; + timeout.tv_sec = (timeoutMS / 1000); + timeout.tv_usec = (timeoutMS % 1000) * 1000; + setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (const char *)&timeout, sizeof(timeout)); + while(received < length) { int ret = recv(socket, dst, length - received, 0); @@ -242,11 +291,13 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) if(err == EWOULDBLOCK || err == EAGAIN) { - ret = 0; + RDCWARN("Timeout in recv"); + Shutdown(); + return false; } else { - RDCWARN("recv: %d", err); + RDCWARN("recv: %s", errno_string(err).c_str()); Shutdown(); return false; } @@ -259,6 +310,8 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) flags = fcntl(socket, F_GETFL, 0); fcntl(socket, F_SETFL, flags | O_NONBLOCK); + setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (const char *)&oldtimeout, sizeof(oldtimeout)); + RDCASSERT(received == length); return true; @@ -347,14 +400,18 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) if(result <= 0) { - RDCDEBUG("connect timed out"); + RDCDEBUG("Timed out"); close(s); continue; } + + socklen_t len = sizeof(err); + getsockopt(s, SOL_SOCKET, SO_ERROR, &err, &len); } - else + + if(err != 0) { - RDCWARN("Error connecting to %s:%d - %d", host, port, err); + RDCDEBUG("%s", errno_string(err).c_str()); close(s); continue; } @@ -366,7 +423,7 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) return new Socket((ptrdiff_t)s); } - RDCWARN("Failed to connect to %s:%d", host, port); + RDCDEBUG("Failed to connect to %s:%d", host, port); return NULL; } diff --git a/renderdoc/os/win32/win32_network.cpp b/renderdoc/os/win32/win32_network.cpp index 3c4e44135..d9b902203 100644 --- a/renderdoc/os/win32/win32_network.cpp +++ b/renderdoc/os/win32/win32_network.cpp @@ -31,6 +31,45 @@ #define WSA_FLAG_NO_HANDLE_INHERIT 0x80 #endif +#ifndef WSA_FLAG_OVERLAPPED +#define WSA_FLAG_OVERLAPPED 0x01 +#endif + +static std::string wsaerr_string(int err) +{ + switch(err) + { + case WSAEWOULDBLOCK: + return "WSAEWOULDBLOCK: A non-blocking socket operation could not be completed immediately"; + case WSAEADDRINUSE: + return "WSAEADDRINUSE: Only one usage of each socket address (protocol/network address/port) " + "is normally permitted."; + case WSAENETDOWN: return "WSAENETDOWN: A socket operation encountered a dead network."; + case WSAENETUNREACH: + return "WSAENETUNREACH: A socket operation was attempted to an unreachable network."; + case WSAENETRESET: + return "WSAENETRESET: The connection has been broken due to keep-alive activity detecting a " + "failure while the operation was in progress."; + case WSAECONNABORTED: + return "WSAECONNABORTED: An established connection was aborted by the software in your host " + "machine."; + case WSAECONNRESET: + return "WSAECONNRESET: An existing connection was forcibly closed by the remote host."; + case WSAETIMEDOUT: return "WSAETIMEDOUT: A socket operation timed out."; + case WSAECONNREFUSED: + return "WSAECONNREFUSED: No connection could be made because the target machine actively " + "refused " + "it."; + case WSAEHOSTDOWN: + return "WSAEHOSTDOWN: A socket operation failed because the destination host was down."; + case WSAEHOSTUNREACH: + return "WSAETIMEDOUT: A socket operation was attempted to an unreachable host."; + default: break; + } + + return StringFormat::Fmt("Unknown error %d", err); +} + namespace Network { void Init() @@ -95,7 +134,7 @@ Socket *Socket::AcceptClient(bool wait) if(err != WSAEWOULDBLOCK) { - RDCWARN("accept: %d", err); + RDCWARN("accept: %s", wsaerr_string(err).c_str()); Shutdown(); } @@ -117,7 +156,11 @@ bool Socket::SendDataBlocking(const void *buf, uint32_t length) u_long enable = 0; ioctlsocket(socket, FIONBIO, &enable); - DWORD timeout = 3000; + DWORD oldtimeout = 0; + int len = sizeof(oldtimeout); + getsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (char *)&oldtimeout, &len); + + DWORD timeout = timeoutMS; setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (const char *)&timeout, sizeof(timeout)); while(sent < length) @@ -130,11 +173,13 @@ bool Socket::SendDataBlocking(const void *buf, uint32_t length) if(err == WSAEWOULDBLOCK) { - ret = 0; + RDCWARN("Timeout in send"); + Shutdown(); + return false; } else { - RDCWARN("send: %d", err); + RDCWARN("send: %s", wsaerr_string(err).c_str()); Shutdown(); return false; } @@ -147,8 +192,7 @@ bool Socket::SendDataBlocking(const void *buf, uint32_t length) enable = 1; ioctlsocket(socket, FIONBIO, &enable); - timeout = 600000; - setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (const char *)&timeout, sizeof(timeout)); + setsockopt(socket, SOL_SOCKET, SO_SNDTIMEO, (const char *)&oldtimeout, sizeof(oldtimeout)); RDCASSERT(sent == length); @@ -175,7 +219,7 @@ bool Socket::IsRecvDataWaiting() } else { - RDCWARN("recv: %d", err); + RDCWARN("recv: %s", wsaerr_string(err).c_str()); Shutdown(); return false; } @@ -207,7 +251,7 @@ bool Socket::RecvDataNonBlocking(void *buf, uint32_t &length) } else { - RDCWARN("recv: %d", err); + RDCWARN("recv: %s", wsaerr_string(err).c_str()); Shutdown(); return false; } @@ -228,7 +272,11 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) u_long enable = 0; ioctlsocket(socket, FIONBIO, &enable); - DWORD timeout = 3000; + DWORD oldtimeout = 0; + int len = sizeof(oldtimeout); + getsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (char *)&oldtimeout, &len); + + DWORD timeout = timeoutMS; setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (const char *)&timeout, sizeof(timeout)); while(received < length) @@ -246,11 +294,13 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) if(err == WSAEWOULDBLOCK) { - ret = 0; + RDCWARN("Timeout in recv"); + Shutdown(); + return false; } else { - RDCWARN("recv: %d", err); + RDCWARN("recv: %s", wsaerr_string(err).c_str()); Shutdown(); return false; } @@ -263,8 +313,7 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) enable = 1; ioctlsocket(socket, FIONBIO, &enable); - timeout = 600000; - setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (const char *)&timeout, sizeof(timeout)); + setsockopt(socket, SOL_SOCKET, SO_RCVTIMEO, (const char *)&oldtimeout, sizeof(oldtimeout)); RDCASSERT(received == length); @@ -273,7 +322,8 @@ bool Socket::RecvDataBlocking(void *buf, uint32_t length) Socket *CreateServerSocket(const char *bindaddr, uint16_t port, int queuesize) { - SOCKET s = WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, NULL, 0, WSA_FLAG_NO_HANDLE_INHERIT); + SOCKET s = WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, NULL, 0, + WSA_FLAG_NO_HANDLE_INHERIT | WSA_FLAG_OVERLAPPED); if(s == INVALID_SOCKET) return NULL; @@ -331,7 +381,8 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) for(addrinfoW *ptr = addrResult; ptr != NULL; ptr = ptr->ai_next) { - SOCKET s = WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, NULL, 0, WSA_FLAG_NO_HANDLE_INHERIT); + SOCKET s = WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, NULL, 0, + WSA_FLAG_NO_HANDLE_INHERIT | WSA_FLAG_OVERLAPPED); if(s == INVALID_SOCKET) return NULL; @@ -342,13 +393,15 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) int result = connect(s, ptr->ai_addr, (int)ptr->ai_addrlen); if(result == SOCKET_ERROR) { - fd_set set; - FD_ZERO(&set); + fd_set setW, setE; + FD_ZERO(&setW); + FD_ZERO(&setE); // macro FD_SET contains the do { } while(0) idiom, which warns #pragma warning(push) #pragma warning(disable : 4127) // conditional expression is constant - FD_SET(s, &set); + FD_SET(s, &setW); + FD_SET(s, &setE); #pragma warning(pop) int err = WSAGetLastError(); @@ -358,18 +411,24 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) timeval timeout; timeout.tv_sec = (timeoutMS / 1000); timeout.tv_usec = (timeoutMS % 1000) * 1000; - result = select((int)s + 1, NULL, &set, NULL, &timeout); + result = select((int)s + 1, NULL, &setW, &setE, &timeout); - if(result <= 0) + socklen_t len = sizeof(err); + getsockopt(s, SOL_SOCKET, SO_ERROR, (char *)&err, &len); + + // if select never returned, if the timeout is less than 1 second we treat it as a + // connection refused. This is inaccurate but we don't want to have to wait a full second + // for the connect to time out. On Winsock there seems to be a minimum of 1 second before + // it will actually return connection refused. + if(result <= 0 && timeoutMS <= 1000) { - RDCDEBUG("connect timed out"); - closesocket(s); - continue; + err = WSAECONNREFUSED; } } - else + + if(err != 0) { - RDCWARN("Error connecting to %s:%d - %d", host, port, err); + RDCDEBUG("%s", wsaerr_string(err).c_str()); closesocket(s); continue; } @@ -381,6 +440,7 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) return new Socket((ptrdiff_t)s); } + RDCDEBUG("Failed to connect to %s:%d", host, port); return NULL; }