socket server / used wrong mutex to protect _connectionsThreads

* More fixes for Windows

* fix tests for windows

* qf for linux

* clean up

README.md

@@ -10,6 +10,7 @@
 * iOS
 * Linux
 * Android
+* Windows (no TLS)
 
 The code was made to compile once on Windows but support is currently broken on this platform.
 

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;
     }
 

ixwebsocket/IXSocket.h

@@ -16,6 +16,20 @@
 #ifdef _WIN32
 #include <BaseTsd.h>
 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<int> _sockfd;
         std::mutex _socketMutex;
 

ixwebsocket/IXSocketConnect.cpp

@@ -7,6 +7,7 @@
 #include "IXSocketConnect.h"
 #include "IXDNSLookup.h"
 #include "IXNetSystem.h"
+#include "IXSocket.h"
 
 #include <string.h>
 #include <fcntl.h>
@@ -18,18 +19,6 @@
 # include <linux/tcp.h>
 #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
+        int res = ::connect(fd, address->ai_addr, address->ai_addrlen);
-            && errno != EINPROGRESS && errno != 0)
+
+        if (res == -1 && !Socket::isWaitNeeded())
         {
-            errMsg = strerror(errno);
+            errMsg = strerror(Socket::getErrno());
-            closeSocket(fd);
+            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 &&
+            // Use select to check the status of the new connection
-                (errno == EBADF || errno == EINVAL))
+            res = select(fd + 1, nullptr, &wfds, &efds, &timeout);
+
+            if (res < 0 && (Socket::getErrno() == EBADF || Socket::getErrno() == EINVAL))
             {
-                closeSocket(fd);
+                Socket::closeSocket(fd);
-                errMsg = std::string("Connect error, select error: ") + strerror(errno);
+                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;
     }

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;
             }
@@ -280,7 +281,7 @@ namespace ix
             if (_stop) return;
 
             // Launch the handleConnection work asynchronously in its own thread.
-            std::lock_guard<std::mutex> lock(_conditionVariableMutex);
+            std::lock_guard<std::mutex> lock(_connectionsThreadsMutex);
             _connectionsThreads.push_back(std::make_pair(
                     connectionState,
                     std::thread(&SocketServer::handleConnection,

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 
+  list(APPEND SOURCES
-    IXWebSocketServerTest.cpp
-    IXWebSocketPingTest.cpp
     IXWebSocketPingTimeoutTest.cpp
     cmd_websocket_chat.cpp
-    IXWebSocketTestConnectionDisconnection.cpp
   )
 endif()
 

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;
     }

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

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
                << "/";
 

test/IXWebSocketServerTest.cpp

@@ -107,7 +107,7 @@ TEST_CASE("Websocket_server", "[websocket_server]")
         std::string errMsg;
         bool tls = false;
         std::shared_ptr<Socket> 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> 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> socket = createSocket(tls, errMsg);
-        std::string host("localhost");
+        std::string host("127.0.0.1");
         auto isCancellationRequested = []() -> bool
         {
             return false;

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;