more protection against socket when closing

.travis.yml

@@ -5,12 +5,16 @@ matrix:
     # macOS
     - os: osx
       compiler: clang
-      script: make test
+      script:
+        - python test/run.py
+        - make
 
     # Linux
     - os: linux
       dist: xenial
-      script: python test/run.py
+      script: 
+        - python test/run.py
+        - make
       env:
         - CC=gcc
         - CXX=g++

CMakeLists.txt

@@ -155,6 +155,6 @@ install(TARGETS ixwebsocket
         PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_PREFIX}/include/ixwebsocket/
 )
 
-if (NOT WIN32)
+if (USE_WS)
     add_subdirectory(ws)
 endif()

docker/Dockerfile.fedora

@@ -16,6 +16,7 @@ ENV PATH="${CMAKE_BIN_PATH}:${PATH}"
 
 RUN yum install -y python
 RUN yum install -y libtsan
+RUN yum install -y zlib-devel
 
 COPY . .
 # RUN ["make", "test"]

docker/Dockerfile.ubuntu_xenial

@@ -1,22 +1,24 @@
-FROM fedora:30 as build
+# Build time
+FROM ubuntu:xenial as build
 
-RUN yum install -y gcc-g++
+ENV DEBIAN_FRONTEND noninteractive
-RUN yum install -y cmake
+RUN apt-get update 
-RUN yum install -y make
+RUN apt-get -y install wget 
-RUN yum install -y openssl-devel
-
-RUN yum install -y wget
 RUN mkdir -p /tmp/cmake
 WORKDIR /tmp/cmake
 RUN wget https://github.com/Kitware/CMake/releases/download/v3.14.0/cmake-3.14.0-Linux-x86_64.tar.gz 
 RUN tar zxf cmake-3.14.0-Linux-x86_64.tar.gz
 
+RUN apt-get -y install g++ 
+RUN apt-get -y install libssl-dev
+RUN apt-get -y install libz-dev
+RUN apt-get -y install make
+RUN apt-get -y install python
+
+COPY . .
+
 ARG CMAKE_BIN_PATH=/tmp/cmake/cmake-3.14.0-Linux-x86_64/bin
 ENV PATH="${CMAKE_BIN_PATH}:${PATH}"
 
-RUN yum install -y python
-RUN yum install -y libtsan
-
-COPY . .
-RUN ["make", "test"]
 # RUN ["make"]
+RUN ["make", "test"]

ixwebsocket/IXWebSocketTransport.cpp

@@ -149,14 +149,17 @@ namespace ix
                                        std::string("Could not parse URL ") + url);
         }
 
-        bool tls = protocol == "wss";
         std::string errorMsg;
+        {
+            bool tls = protocol == "wss";
+            std::lock_guard<std::mutex> lock(_socketMutex);
             _socket = createSocket(tls, errorMsg);
 
             if (!_socket)
             {
                 return WebSocketInitResult(false, 0, errorMsg);
             }
+        }
 
         WebSocketHandshake webSocketHandshake(_requestInitCancellation,
                                               _socket,
@@ -180,12 +183,15 @@ namespace ix
         _useMask = false;
 
         std::string errorMsg;
+        {
+            std::lock_guard<std::mutex> lock(_socketMutex);
             _socket = createSocket(fd, errorMsg);
 
             if (!_socket)
             {
                 return WebSocketInitResult(false, 0, errorMsg);
             }
+        }
 
         WebSocketHandshake webSocketHandshake(_requestInitCancellation,
                                               _socket,
@@ -338,7 +344,7 @@ namespace ix
 
                 if (result == PollResultType::Error)
                 {
-                    _socket->close();
+                    closeSocket();
                     setReadyState(ReadyState::CLOSED);
                     break;
                 }
@@ -363,7 +369,7 @@ namespace ix
                     // if there are received data pending to be processed, then delay the abnormal closure
                     // to after dispatch (other close code/reason could be read from the buffer)
                     
-                    _socket->close();
+                    closeSocket();
 
                     return PollResult::AbnormalClose;
                 }
@@ -377,18 +383,18 @@ namespace ix
         }
         else if (pollResult == PollResultType::Error)
         {
-            _socket->close();
+            closeSocket();
         }
         else if (pollResult == PollResultType::CloseRequest)
         {
-            _socket->close();
+            closeSocket();
         }
 
         if (_readyState == ReadyState::CLOSING && closingDelayExceeded())
         {
             _rxbuf.clear();
             // close code and reason were set when calling close()
-            _socket->close();
+            closeSocket();
             setReadyState(ReadyState::CLOSED);
         }
 
@@ -655,7 +661,6 @@ namespace ix
             else
             {
                 // Unexpected frame type
-
                 close(kProtocolErrorCode, kProtocolErrorMessage, _rxbuf.size());
             }
 
@@ -673,7 +678,7 @@ namespace ix
             // if we previously closed the connection (CLOSING state), then set state to CLOSED (code/reason were set before)
             if (_readyState == ReadyState::CLOSING)
             {
-                _socket->close();
+                closeSocket();
                 setReadyState(ReadyState::CLOSED);
             }
             // if we weren't closing, then close using abnormal close code and message 
@@ -949,13 +954,19 @@ namespace ix
                         _enablePerMessageDeflate, onProgressCallback);
     }
 
+    ssize_t WebSocketTransport::send()
+    {
+        std::lock_guard<std::mutex> lock(_socketMutex);
+        return _socket->send((char*)&_txbuf[0], _txbuf.size());
+    }
+
     void WebSocketTransport::sendOnSocket()
     {
         std::lock_guard<std::mutex> lock(_txbufMutex);
 
         while (_txbuf.size())
         {
-            ssize_t ret = _socket->send((char*)&_txbuf[0], _txbuf.size());
+            ssize_t ret = send();
 
             if (ret < 0 && Socket::isWaitNeeded())
             {
@@ -963,8 +974,7 @@ namespace ix
             }
             else if (ret <= 0)
             {
-                _socket->close();
+                closeSocket();
-
                 setReadyState(ReadyState::CLOSED);
                 break;
             }
@@ -998,9 +1008,16 @@ namespace ix
         }
     }
 
-    void WebSocketTransport::closeSocketAndSwitchToClosedState(uint16_t code, const std::string& reason, size_t closeWireSize, bool remote)
+    void WebSocketTransport::closeSocket()
     {
+        std::lock_guard<std::mutex> lock(_socketMutex);
         _socket->close();
+    }
+
+    void WebSocketTransport::closeSocketAndSwitchToClosedState(
+        uint16_t code, const std::string& reason, size_t closeWireSize, bool remote)
+    {
+        closeSocket();
         {
             std::lock_guard<std::mutex> lock(_closeDataMutex);
             _closeCode = code;
@@ -1011,7 +1028,8 @@ namespace ix
         setReadyState(ReadyState::CLOSED);
     }
 
-    void WebSocketTransport::close(uint16_t code, const std::string& reason, size_t closeWireSize, bool remote)
+    void WebSocketTransport::close(
+        uint16_t code, const std::string& reason, size_t closeWireSize, bool remote)
     {
         _requestInitCancellation = true;
 

ixwebsocket/IXWebSocketTransport.h

@@ -96,6 +96,9 @@ namespace ix
                    size_t closeWireSize = 0,
                    bool remote = false);
 
+        void closeSocket();
+        ssize_t send();
+
         ReadyState getReadyState() const;
         void setReadyState(ReadyState readyState);
         void setOnCloseCallback(const OnCloseCallback& onCloseCallback);
@@ -151,6 +154,7 @@ namespace ix
 
         // Underlying TCP socket
         std::shared_ptr<Socket> _socket;
+        std::mutex _socketMutex;
 
         // Hold the state of the connection (OPEN, CLOSED, etc...)
         std::atomic<ReadyState> _readyState;

makefile

@@ -9,7 +9,7 @@ install: brew
 # on osx it is good practice to make /usr/local user writable
 # sudo chown -R `whoami`/staff /usr/local
 brew:
-	mkdir -p build && (cd build ; cmake -DUSE_TLS=1 .. ; make -j install)
+	mkdir -p build && (cd build ; cmake -DUSE_TLS=1 -DUSE_WS=1 .. ; make -j install)
 
 uninstall:
 	xargs rm -fv < build/install_manifest.txt

test/CMakeLists.txt

@@ -37,12 +37,12 @@ set (SOURCES
   IXWebSocketTestConnectionDisconnection.cpp
   IXUrlParserTest.cpp
   IXWebSocketServerTest.cpp
-  IXWebSocketCloseTest.cpp
 )
 
 # Some unittest don't work on windows yet
 if (UNIX)
   list(APPEND SOURCES
+    IXWebSocketCloseTest.cpp
     IXDNSLookupTest.cpp
     cmd_websocket_chat.cpp
   )

test/run.py

@@ -352,7 +352,10 @@ def run(testName, buildDir, sanitizer, xmlOutput, testRunName, buildOnly, useLLD
     # gen build files with CMake
     runCMake(sanitizer, buildDir)
 
-    if platform.system() == 'Darwin':
+    if platform.system() == 'Linux':
+        # build with make -j
+        runCommand('make -C {} -j 2'.format(buildDir))
+    elif platform.system() == 'Darwin':
         # build with make
         runCommand('make -C {} -j 8'.format(buildDir))
     else: