From 8ad47a315bd306cef1b3f6b6815742ee707b4485 Mon Sep 17 00:00:00 2001 From: Dimon4eg Date: Mon, 6 May 2019 22:22:57 +0300 Subject: [PATCH] Fix windows (#51) * More fixes for Windows * fix tests for windows * qf for linux * clean up --- ixwebsocket/IXSocket.cpp | 17 +++++------- ixwebsocket/IXSocket.h | 17 ++++++++++-- ixwebsocket/IXSocketConnect.cpp | 42 +++++++++++++---------------- ixwebsocket/IXSocketServer.cpp | 15 ++++++----- test/CMakeLists.txt | 8 +++--- test/IXTest.cpp | 6 ++--- test/IXWebSocketPingTest.cpp | 6 +++-- test/IXWebSocketPingTimeoutTest.cpp | 3 +-- test/IXWebSocketServerTest.cpp | 6 ++--- test/cmd_websocket_chat.cpp | 2 +- 10 files changed, 64 insertions(+), 58 deletions(-) diff --git a/ixwebsocket/IXSocket.cpp b/ixwebsocket/IXSocket.cpp index 0d8a4dfe..747ee9c6 100644 --- a/ixwebsocket/IXSocket.cpp +++ b/ixwebsocket/IXSocket.cpp @@ -189,29 +189,26 @@ namespace ix int Socket::getErrno() { + int err; + #ifdef _WIN32 - return WSAGetLastError(); + err = WSAGetLastError(); #else - return errno; + err = errno; #endif + + return err; } bool Socket::isWaitNeeded() { int err = getErrno(); - if (err == EWOULDBLOCK || err == EAGAIN) + if (err == EWOULDBLOCK || err == EAGAIN || err == EINPROGRESS) { return true; } -#ifdef _WIN32 - if (err == WSAEWOULDBLOCK || err == WSATRY_AGAIN) - { - return true; - } -#endif - return false; } diff --git a/ixwebsocket/IXSocket.h b/ixwebsocket/IXSocket.h index db047f52..d42b7111 100644 --- a/ixwebsocket/IXSocket.h +++ b/ixwebsocket/IXSocket.h @@ -16,6 +16,20 @@ #ifdef _WIN32 #include typedef SSIZE_T ssize_t; + +#undef EWOULDBLOCK +#undef EAGAIN +#undef EINPROGRESS +#undef EBADF +#undef EINVAL + +// map to WSA error codes +#define EWOULDBLOCK WSAEWOULDBLOCK +#define EAGAIN WSATRY_AGAIN +#define EINPROGRESS WSAEINPROGRESS +#define EBADF WSAEBADF +#define EINVAL WSAEINVAL + #endif #include "IXCancellationRequest.h" @@ -75,14 +89,13 @@ namespace ix static int getErrno(); static bool isWaitNeeded(); + static void closeSocket(int fd); // Used as special codes for pipe communication static const uint64_t kSendRequest; static const uint64_t kCloseRequest; protected: - void closeSocket(int fd); - std::atomic _sockfd; std::mutex _socketMutex; diff --git a/ixwebsocket/IXSocketConnect.cpp b/ixwebsocket/IXSocketConnect.cpp index 08011a33..2c19871b 100644 --- a/ixwebsocket/IXSocketConnect.cpp +++ b/ixwebsocket/IXSocketConnect.cpp @@ -7,6 +7,7 @@ #include "IXSocketConnect.h" #include "IXDNSLookup.h" #include "IXNetSystem.h" +#include "IXSocket.h" #include #include @@ -18,18 +19,6 @@ # include #endif -namespace -{ - void closeSocket(int fd) - { -#ifdef _WIN32 - closesocket(fd); -#else - ::close(fd); -#endif - } -} - namespace ix { // @@ -56,11 +45,12 @@ namespace ix // block us for too long SocketConnect::configure(fd); - if (::connect(fd, address->ai_addr, address->ai_addrlen) == -1 - && errno != EINPROGRESS && errno != 0) + int res = ::connect(fd, address->ai_addr, address->ai_addrlen); + + if (res == -1 && !Socket::isWaitNeeded()) { - errMsg = strerror(errno); - closeSocket(fd); + errMsg = strerror(Socket::getErrno()); + Socket::closeSocket(fd); return -1; } @@ -68,15 +58,17 @@ namespace ix { if (isCancellationRequested && isCancellationRequested()) // Must handle timeout as well { - closeSocket(fd); + Socket::closeSocket(fd); errMsg = "Cancelled"; return -1; } - // Use select to check the status of the new connection + // On Linux the timeout needs to be re-initialized everytime + // http://man7.org/linux/man-pages/man2/select.2.html struct timeval timeout; timeout.tv_sec = 0; timeout.tv_usec = 10 * 1000; // 10ms timeout + fd_set wfds; fd_set efds; @@ -85,11 +77,13 @@ namespace ix FD_ZERO(&efds); FD_SET(fd, &efds); - if (select(fd + 1, nullptr, &wfds, &efds, &timeout) < 0 && - (errno == EBADF || errno == EINVAL)) + // Use select to check the status of the new connection + res = select(fd + 1, nullptr, &wfds, &efds, &timeout); + + if (res < 0 && (Socket::getErrno() == EBADF || Socket::getErrno() == EINVAL)) { - closeSocket(fd); - errMsg = std::string("Connect error, select error: ") + strerror(errno); + Socket::closeSocket(fd); + errMsg = std::string("Connect error, select error: ") + strerror(Socket::getErrno()); return -1; } @@ -110,7 +104,7 @@ namespace ix optval != 0) #endif { - closeSocket(fd); + Socket::closeSocket(fd); errMsg = strerror(optval); return -1; } @@ -121,7 +115,7 @@ namespace ix } } - closeSocket(fd); + Socket::closeSocket(fd); errMsg = "connect timed out after 60 seconds"; return -1; } diff --git a/ixwebsocket/IXSocketServer.cpp b/ixwebsocket/IXSocketServer.cpp index 0a7966e9..79d8d3d3 100644 --- a/ixwebsocket/IXSocketServer.cpp +++ b/ixwebsocket/IXSocketServer.cpp @@ -77,7 +77,7 @@ namespace ix << "at address " << _host << ":" << _port << " : " << strerror(Socket::getErrno()); - ::close(_serverFd); + Socket::closeSocket(_serverFd); return std::make_pair(false, ss.str()); } @@ -101,7 +101,7 @@ namespace ix << "at address " << _host << ":" << _port << " : " << strerror(Socket::getErrno()); - ::close(_serverFd); + Socket::closeSocket(_serverFd); return std::make_pair(false, ss.str()); } @@ -115,7 +115,7 @@ namespace ix << "at address " << _host << ":" << _port << " : " << strerror(Socket::getErrno()); - ::close(_serverFd); + Socket::closeSocket(_serverFd); return std::make_pair(false, ss.str()); } @@ -159,7 +159,7 @@ namespace ix _stop = false; _conditionVariable.notify_one(); - ::close(_serverFd); + Socket::closeSocket(_serverFd); } void SocketServer::setConnectionStateFactory( @@ -242,7 +242,7 @@ namespace ix // Accept a connection. struct sockaddr_in client; // client address information int clientFd; // socket connected to client - socklen_t addressLen = sizeof(socklen_t); + socklen_t addressLen = sizeof(client); memset(&client, 0, sizeof(client)); if ((clientFd = accept(_serverFd, (struct sockaddr *)&client, &addressLen)) < 0) @@ -250,9 +250,10 @@ namespace ix if (!Socket::isWaitNeeded()) { // FIXME: that error should be propagated + int err = Socket::getErrno(); std::stringstream ss; ss << "SocketServer::run() error accepting connection: " - << strerror(Socket::getErrno()); + << err << ", " << strerror(err); logError(ss.str()); } continue; @@ -266,7 +267,7 @@ namespace ix << "Not accepting connection"; logError(ss.str()); - ::close(clientFd); + Socket::closeSocket(clientFd); continue; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f1038ae9..4f168ba5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -32,16 +32,16 @@ set (SOURCES IXDNSLookupTest.cpp IXSocketTest.cpp IXSocketConnectTest.cpp + IXWebSocketServerTest.cpp + IXWebSocketPingTest.cpp + IXWebSocketTestConnectionDisconnection.cpp ) # Some unittest don't work on windows yet if (NOT WIN32) - list(APPEND SOURCES - IXWebSocketServerTest.cpp - IXWebSocketPingTest.cpp + list(APPEND SOURCES IXWebSocketPingTimeoutTest.cpp cmd_websocket_chat.cpp - IXWebSocketTestConnectionDisconnection.cpp ) endif() diff --git a/test/IXTest.cpp b/test/IXTest.cpp index 9808994a..886df20f 100644 --- a/test/IXTest.cpp +++ b/test/IXTest.cpp @@ -108,7 +108,7 @@ namespace ix { log("Cannot compute a free port. bind error."); - ::close(sockfd); + Socket::closeSocket(sockfd); return getAnyFreePortRandom(); } @@ -118,12 +118,12 @@ namespace ix { log("Cannot compute a free port. getsockname error."); - ::close(sockfd); + Socket::closeSocket(sockfd); return getAnyFreePortRandom(); } int port = ntohs(sa.sin_port); - ::close(sockfd); + Socket::closeSocket(sockfd); return port; } diff --git a/test/IXWebSocketPingTest.cpp b/test/IXWebSocketPingTest.cpp index a9e6f53f..3e67b293 100644 --- a/test/IXWebSocketPingTest.cpp +++ b/test/IXWebSocketPingTest.cpp @@ -23,7 +23,6 @@ namespace public: WebSocketClient(int port, bool useHeartBeatMethod); - void subscribe(const std::string& channel); void start(); void stop(); bool isReady() const; @@ -57,7 +56,7 @@ namespace std::string url; { std::stringstream ss; - ss << "ws://localhost:" + ss << "ws://127.0.0.1:" << _port << "/"; @@ -347,6 +346,9 @@ TEST_CASE("Websocket_ping_data_sent_setPingInterval", "[setPingInterval]") webSocketClient.stop(); + // without this sleep test fails on Windows + ix::msleep(100); + // Here we test ping interval // client has sent data, but ping should have been sent no matter what // -> expected ping messages == 3 as 900+900+1300 = 3100 seconds, 1 ping sent every second diff --git a/test/IXWebSocketPingTimeoutTest.cpp b/test/IXWebSocketPingTimeoutTest.cpp index c2733898..45e839bb 100644 --- a/test/IXWebSocketPingTimeoutTest.cpp +++ b/test/IXWebSocketPingTimeoutTest.cpp @@ -23,7 +23,6 @@ namespace public: WebSocketClient(int port, int pingInterval, int pingTimeout); - void subscribe(const std::string& channel); void start(); void stop(); bool isReady() const; @@ -71,7 +70,7 @@ namespace std::string url; { std::stringstream ss; - ss << "ws://localhost:" + ss << "ws://127.0.0.1:" << _port << "/"; diff --git a/test/IXWebSocketServerTest.cpp b/test/IXWebSocketServerTest.cpp index d5cd437f..8a2c2adb 100644 --- a/test/IXWebSocketServerTest.cpp +++ b/test/IXWebSocketServerTest.cpp @@ -107,7 +107,7 @@ TEST_CASE("Websocket_server", "[websocket_server]") std::string errMsg; bool tls = false; std::shared_ptr socket = createSocket(tls, errMsg); - std::string host("localhost"); + std::string host("127.0.0.1"); auto isCancellationRequested = []() -> bool { return false; @@ -141,7 +141,7 @@ TEST_CASE("Websocket_server", "[websocket_server]") std::string errMsg; bool tls = false; std::shared_ptr socket = createSocket(tls, errMsg); - std::string host("localhost"); + std::string host("127.0.0.1"); auto isCancellationRequested = []() -> bool { return false; @@ -178,7 +178,7 @@ TEST_CASE("Websocket_server", "[websocket_server]") std::string errMsg; bool tls = false; std::shared_ptr socket = createSocket(tls, errMsg); - std::string host("localhost"); + std::string host("127.0.0.1"); auto isCancellationRequested = []() -> bool { return false; diff --git a/test/cmd_websocket_chat.cpp b/test/cmd_websocket_chat.cpp index 2907edc8..86890832 100644 --- a/test/cmd_websocket_chat.cpp +++ b/test/cmd_websocket_chat.cpp @@ -100,7 +100,7 @@ namespace std::string url; { std::stringstream ss; - ss << "ws://localhost:" + ss << "ws://127.0.0.1:" << _port << "/" << _user;