Skip to content

Commit

Permalink
more protection against socket when closing
Browse files Browse the repository at this point in the history
  • Loading branch information
bsergean committed May 15, 2019
1 parent 5b2b2ea commit 9315eb5
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 40 deletions.
8 changes: 6 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
1 change: 1 addition & 0 deletions docker/Dockerfile.fedora
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
26 changes: 14 additions & 12 deletions docker/Dockerfile.ubuntu_xenial
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
FROM fedora:30 as build
# Build time
FROM ubuntu:xenial as build

RUN yum install -y gcc-g++
RUN yum install -y cmake
RUN yum install -y make
RUN yum install -y openssl-devel

RUN yum install -y wget
ENV DEBIAN_FRONTEND noninteractive
RUN apt-get update
RUN apt-get -y install 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"]
60 changes: 39 additions & 21 deletions ixwebsocket/IXWebSocketTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,16 @@ namespace ix
std::string("Could not parse URL ") + url);
}

bool tls = protocol == "wss";
std::string errorMsg;
_socket = createSocket(tls, errorMsg);

if (!_socket)
{
return WebSocketInitResult(false, 0, 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,
Expand All @@ -180,11 +183,14 @@ namespace ix
_useMask = false;

std::string errorMsg;
_socket = createSocket(fd, errorMsg);

if (!_socket)
{
return WebSocketInitResult(false, 0, errorMsg);
std::lock_guard<std::mutex> lock(_socketMutex);
_socket = createSocket(fd, errorMsg);

if (!_socket)
{
return WebSocketInitResult(false, 0, errorMsg);
}
}

WebSocketHandshake webSocketHandshake(_requestInitCancellation,
Expand Down Expand Up @@ -338,7 +344,7 @@ namespace ix

if (result == PollResultType::Error)
{
_socket->close();
closeSocket();
setReadyState(ReadyState::CLOSED);
break;
}
Expand All @@ -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;
}
Expand All @@ -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);
}

Expand Down Expand Up @@ -655,7 +661,6 @@ namespace ix
else
{
// Unexpected frame type

close(kProtocolErrorCode, kProtocolErrorMessage, _rxbuf.size());
}

Expand All @@ -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
Expand Down Expand Up @@ -949,22 +954,27 @@ 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())
{
break;
}
else if (ret <= 0)
{
_socket->close();

closeSocket();
setReadyState(ReadyState::CLOSED);
break;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions ixwebsocket/IXWebSocketTransport.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
7 changes: 5 additions & 2 deletions test/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,12 @@ 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 {} -j8'.format(buildDir))
runCommand('make -C {} -j 8'.format(buildDir))
else:
# build with cmake on recent
runCommand('cmake --build --parallel {}'.format(buildDir))
Expand Down

0 comments on commit 9315eb5

Please sign in to comment.