From 8d3a47a873a21a8450049c0d40cca242dcc00280 Mon Sep 17 00:00:00 2001 From: Dimon4eg Date: Sat, 11 May 2019 19:51:26 +0300 Subject: [PATCH] Fix crash during closing on Windows (#64) * fix crash on close * Improve calculateRetryWaitMilliseconds (#63) * improve calculateRetryWaitMilliseconds * update comment * cout -> spdlog * fix crash on close * uncomment test * Revert "uncomment test" This reverts commit 27df86ee8f9eb1e0e14652d0707faab1aff88001. --- ixwebsocket/IXWebSocket.cpp | 114 ++++++++++++++--------------- ixwebsocket/IXWebSocket.h | 6 +- ixwebsocket/IXWebSocketErrorInfo.h | 8 +- 3 files changed, 60 insertions(+), 68 deletions(-) diff --git a/ixwebsocket/IXWebSocket.cpp b/ixwebsocket/IXWebSocket.cpp index 1821185e..7d568561 100644 --- a/ixwebsocket/IXWebSocket.cpp +++ b/ixwebsocket/IXWebSocket.cpp @@ -151,16 +151,13 @@ namespace ix close(); - if (!_thread.joinable()) + if (_thread.joinable()) { - _automaticReconnection = automaticReconnection; - return; + _stop = true; + _thread.join(); + _stop = false; } - _stop = true; - _thread.join(); - _stop = false; - _automaticReconnection = automaticReconnection; } @@ -220,72 +217,63 @@ namespace ix return getReadyState() == WebSocket_ReadyState_Closing; } - bool WebSocket::isConnectedOrClosing() const - { - return isConnected() || isClosing(); - } - void WebSocket::close() { _ws.close(); } - void WebSocket::reconnectPerpetuallyIfDisconnected() + void WebSocket::checkConnection(bool initial) { - uint32_t retries = 0; - WebSocketErrorInfo connectErr; - ix::WebSocketInitResult status; using millis = std::chrono::duration; - millis duration; - // Try to connect only once when we don't have automaticReconnection setup - if (!isConnectedOrClosing() && !_stop && !_automaticReconnection) + uint32_t retries = 0; + millis duration; + ix::WebSocketInitResult status; + + // we will try to connect perpertually + while (true) { + if (isConnected() || isClosing() || _stop) + { + break; + } + + if (!initial && !_automaticReconnection) + { + // don't attempt to reconnect + break; + } + + initial = false; + + // Only sleep if we are retrying + if (duration.count() > 0) + { + // to do: make conditional sleeping + std::this_thread::sleep_for(duration); + } + + // try to connect (sync connect) status = connect(_handshakeTimeoutSecs); if (!status.success) { - duration = millis(calculateRetryWaitMilliseconds(retries++)); + WebSocketErrorInfo connectErr; - connectErr.retries = retries; - connectErr.wait_time = duration.count(); - connectErr.reason = status.errorStr; - connectErr.http_status = status.http_status; - _onMessageCallback(WebSocket_MessageType_Error, "", 0, - connectErr, WebSocketOpenInfo(), - WebSocketCloseInfo()); - } - } - else - { - // Otherwise try to reconnect perpertually - while (true) - { - if (isConnectedOrClosing() || _stop || !_automaticReconnection) - { - break; - } - - status = connect(_handshakeTimeoutSecs); - - if (!status.success) + if (_automaticReconnection) { duration = millis(calculateRetryWaitMilliseconds(retries++)); - connectErr.retries = retries; connectErr.wait_time = duration.count(); - connectErr.reason = status.errorStr; - connectErr.http_status = status.http_status; - _onMessageCallback(WebSocket_MessageType_Error, "", 0, - connectErr, WebSocketOpenInfo(), - WebSocketCloseInfo()); - - // Only sleep if we aren't in the middle of stopping - if (!_stop) - { - std::this_thread::sleep_for(duration); - } + connectErr.retries = retries; } + + connectErr.reason = status.errorStr; + connectErr.http_status = status.http_status; + + _onMessageCallback(WebSocket_MessageType_Error, "", 0, + connectErr, WebSocketOpenInfo(), + WebSocketCloseInfo()); } } } @@ -294,12 +282,20 @@ namespace ix { setThreadName(getUrl()); + bool initial = true; + while (true) { - if (_stop && !isClosing()) return; - // 1. Make sure we are always connected - reconnectPerpetuallyIfDisconnected(); + checkConnection(initial); + + initial = false; + + // if here we are closed then checkConnection was not able to connect + if (getReadyState() == WebSocket_ReadyState_Closed) + { + break; + } // 2. Poll to see if there's any new data available WebSocketTransport::PollPostTreatment pollPostTreatment = _ws.poll(); @@ -315,6 +311,7 @@ namespace ix WebSocketMessageType webSocketMessageType; switch (messageKind) { + default: case WebSocketTransport::MSG: { webSocketMessageType = WebSocket_MessageType_Message; @@ -345,9 +342,6 @@ namespace ix WebSocket::invokeTrafficTrackerCallback(msg.size(), true); }); - - // If we aren't trying to reconnect automatically, exit if we aren't connected - if (!isConnectedOrClosing() && !_automaticReconnection) return; } } diff --git a/ixwebsocket/IXWebSocket.h b/ixwebsocket/IXWebSocket.h index 07c00a4a..1520645c 100644 --- a/ixwebsocket/IXWebSocket.h +++ b/ixwebsocket/IXWebSocket.h @@ -91,7 +91,6 @@ namespace ix void setUrl(const std::string& url); void setPerMessageDeflateOptions(const WebSocketPerMessageDeflateOptions& perMessageDeflateOptions); - void setHandshakeTimeout(int handshakeTimeoutSecs); void setHeartBeatPeriod(int heartBeatPeriodSecs); void setPingInterval(int pingIntervalSecs); // alias of setHeartBeatPeriod void setPingTimeout(int pingTimeoutSecs); @@ -100,6 +99,7 @@ namespace ix // Run asynchronously, by calling start and stop. void start(); + // stop is synchronous void stop(); // Run in blocking mode, by connecting first manually, and then calling run. @@ -136,13 +136,11 @@ namespace ix bool isConnected() const; bool isClosing() const; - bool isConnectedOrClosing() const; - void reconnectPerpetuallyIfDisconnected(); + void checkConnection(bool initial); std::string readyStateToString(ReadyState readyState); static void invokeTrafficTrackerCallback(size_t size, bool incoming); // Server - void setSocketFileDescriptor(int fd); WebSocketInitResult connectToSocket(int fd, int timeoutSecs); WebSocketTransport _ws; diff --git a/ixwebsocket/IXWebSocketErrorInfo.h b/ixwebsocket/IXWebSocketErrorInfo.h index 8b515fac..e8f16c62 100644 --- a/ixwebsocket/IXWebSocketErrorInfo.h +++ b/ixwebsocket/IXWebSocketErrorInfo.h @@ -12,10 +12,10 @@ namespace ix { struct WebSocketErrorInfo { - uint32_t retries; - double wait_time; - int http_status; + uint32_t retries = 0; + double wait_time = 0; + int http_status = 0; std::string reason; - bool decompressionError; + bool decompressionError = false; }; }