From 8a0afef8253c6cf8090b0d1343835c2661c61862 Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Mon, 7 Jan 2019 11:18:00 -0800 Subject: [PATCH] check select errors better --- ixwebsocket/IXSocket.cpp | 9 ++- ixwebsocket/IXSocketConnect.cpp | 60 +++++++++---------- ixwebsocket/IXSocketServer.cpp | 23 ++++--- ixwebsocket/IXWebSocketTransport.cpp | 3 +- ...IXWebSocketTestConnectionDisconnection.cpp | 3 +- 5 files changed, 55 insertions(+), 43 deletions(-) diff --git a/ixwebsocket/IXSocket.cpp b/ixwebsocket/IXSocket.cpp index 3154813f..cfedb3b0 100644 --- a/ixwebsocket/IXSocket.cpp +++ b/ixwebsocket/IXSocket.cpp @@ -171,11 +171,16 @@ namespace ix fd_set rfds; struct timeval timeout; timeout.tv_sec = 0; - timeout.tv_usec = 1 * 1000; // 1ms + timeout.tv_usec = 1 * 1000; // 1ms timeout FD_ZERO(&rfds); FD_SET(_sockfd, &rfds); - select(_sockfd + 1, &rfds, nullptr, nullptr, &timeout); + + if (select(_sockfd + 1, &rfds, nullptr, nullptr, &timeout) < 0 && + (errno == EBADF || errno == EINVAL)) + { + return false; + } continue; } diff --git a/ixwebsocket/IXSocketConnect.cpp b/ixwebsocket/IXSocketConnect.cpp index 402f1909..8cf9aa22 100644 --- a/ixwebsocket/IXSocketConnect.cpp +++ b/ixwebsocket/IXSocketConnect.cpp @@ -43,9 +43,9 @@ namespace ix { errMsg = "no error"; - int fd = (int) socket(address->ai_family, - address->ai_socktype, - address->ai_protocol); + int fd = socket(address->ai_family, + address->ai_socktype, + address->ai_protocol); if (fd < 0) { errMsg = "Cannot create a socket"; @@ -56,33 +56,14 @@ namespace ix // block us for too long SocketConnect::configure(fd); - if (::connect(fd, address->ai_addr, address->ai_addrlen) == -1) + if (::connect(fd, address->ai_addr, address->ai_addrlen) == -1 + && errno != EINPROGRESS) { -#ifdef _WIN32 - if (WSAGetLastError() == WSAEWOULDBLOCK) errno = EINPROGRESS; -#endif - if (errno != EINPROGRESS) - { - closeSocket(fd); - errMsg = std::string("Connect error in ::connect:") + strerror(errno); - return -1; - } + closeSocket(fd); + errMsg = strerror(errno); + return -1; } - // Use select to see if the connect did succeed. - fd_set wfds; - FD_ZERO(&wfds); - FD_SET(fd, &wfds); - - fd_set efds; - FD_ZERO(&efds); - FD_SET(fd, &efds); - - // 50ms select timeout - struct timeval timeout; - timeout.tv_sec = 0; - timeout.tv_usec = 50 * 1000; - for (;;) { if (isCancellationRequested()) // Must handle timeout as well @@ -91,14 +72,31 @@ namespace ix errMsg = "Cancelled"; return -1; } + + // Use select to check the status of the new connection + struct timeval timeout; + timeout.tv_sec = 0; + timeout.tv_usec = 10 * 1000; // 10ms timeout + fd_set wfds; + fd_set efds; - select(fd + 1, nullptr, &wfds, &efds, &timeout); + FD_ZERO(&wfds); + FD_SET(fd, &wfds); + FD_ZERO(&efds); + FD_SET(fd, &efds); + + if (select(fd + 1, nullptr, &wfds, &efds, &timeout) < 0 && + (errno == EBADF || errno == EINVAL)) + { + closeSocket(fd); + errMsg = std::string("Connect error, select error: ") + strerror(errno); + return -1; + } // Nothing was written to the socket, wait again. if (!FD_ISSET(fd, &wfds)) continue; // Something was written to the socket. Check for errors. - int optval = -1; socklen_t optlen = sizeof(optval); @@ -113,7 +111,7 @@ namespace ix #endif { closeSocket(fd); - errMsg = std::string("Connect error in getsockopt:") + strerror(optval); + errMsg = strerror(optval); return -1; } else @@ -124,7 +122,7 @@ namespace ix } closeSocket(fd); - errMsg = "connect timed out"; + errMsg = "connect timed out after 60 seconds"; return -1; } diff --git a/ixwebsocket/IXSocketServer.cpp b/ixwebsocket/IXSocketServer.cpp index 317b3d8a..a6344e27 100644 --- a/ixwebsocket/IXSocketServer.cpp +++ b/ixwebsocket/IXSocketServer.cpp @@ -146,19 +146,28 @@ namespace ix // Return value of std::async, ignored std::future f; - // Select arguments - fd_set rfds; - struct timeval timeout; - timeout.tv_sec = 0; - timeout.tv_usec = 10 * 1000; // 10ms - for (;;) { if (_stop) return; + // Use select to check whether a new connection is in progress + fd_set rfds; + struct timeval timeout; + timeout.tv_sec = 0; + timeout.tv_usec = 10 * 1000; // 10ms timeout + FD_ZERO(&rfds); FD_SET(_serverFd, &rfds); - select(_serverFd + 1, &rfds, nullptr, nullptr, &timeout); + + if (select(_serverFd + 1, &rfds, nullptr, nullptr, &timeout) < 0 && + (errno == EBADF || errno == EINVAL)) + { + std::stringstream ss; + ss << "SocketServer::run() error in select: " + << strerror(Socket::getErrno()); + logError(ss.str()); + continue; + } if (!FD_ISSET(_serverFd, &rfds)) { diff --git a/ixwebsocket/IXWebSocketTransport.cpp b/ixwebsocket/IXWebSocketTransport.cpp index 3e183e2a..47310dc0 100644 --- a/ixwebsocket/IXWebSocketTransport.cpp +++ b/ixwebsocket/IXWebSocketTransport.cpp @@ -158,9 +158,8 @@ namespace ix { int N = (int) _rxbuf.size(); - int ret; _rxbuf.resize(N + 1500); - ret = _socket->recv((char*)&_rxbuf[0] + N, 1500); + ssize_t ret = _socket->recv((char*)&_rxbuf[0] + N, 1500); if (ret < 0 && (_socket->getErrno() == EWOULDBLOCK || _socket->getErrno() == EAGAIN)) { diff --git a/test/IXWebSocketTestConnectionDisconnection.cpp b/test/IXWebSocketTestConnectionDisconnection.cpp index 7bd5e09f..98005617 100644 --- a/test/IXWebSocketTestConnectionDisconnection.cpp +++ b/test/IXWebSocketTestConnectionDisconnection.cpp @@ -16,7 +16,8 @@ using namespace ix; namespace { - const std::string WEBSOCKET_DOT_ORG_URL("wss://echo.websocket.org"); + // const std::string WEBSOCKET_DOT_ORG_URL("wss://echo.websocket.org"); + const std::string WEBSOCKET_DOT_ORG_URL("wss://api.cobra.mobilewar-online.com/v2?appkey=ADdc208AB26B911F4cB05bFeedD8EbBA"); const std::string GOOGLE_URL("wss://google.com"); const std::string UNKNOWN_URL("wss://asdcasdcaasdcasdcasdcasdcasdcasdcasassdd.com");