fixed crash for non SSL websocket

fixed more race conditions
This commit is contained in:
Frederik Haselmeier 2018-03-27 05:47:44 +02:00
parent f56c34d714
commit 495db16ac8
4 changed files with 42 additions and 13 deletions

View File

@ -330,7 +330,8 @@ LUA_FUNCTION(webSocketThink)
}
//Pops the socket's table
LUA->Pop();
if (socket->state == STATE_DISCONNECTED)
//The size == 0 check here is required because a callback might trigger more messages in the callback
if (socket->state == STATE_DISCONNECTED && messages.size() == 0)
{
//This means the socket has been disconnected (possibly from the other side)
//We drop the reference to the table here so that the websocket can be gced

View File

@ -33,11 +33,11 @@ void GWSocket::onDisconnected(const boost::system::error_code & ec)
bool GWSocket::close()
{
auto expected = this->state.load();
if (expected == STATE_DISCONNECTED || expected == STATE_DISCONNECTING)
if (expected == STATE_DISCONNECTED || expected == STATE_DISCONNECTING || expected == SATE_DISCONNECT_REQUESTED)
{
return false;
}
if (!this->state.compare_exchange_strong(expected, STATE_DISCONNECTING))
if (!this->state.compare_exchange_strong(expected, SATE_DISCONNECT_REQUESTED))
{
return false;
}
@ -133,10 +133,19 @@ void GWSocket::handshakeCompleted(const boost::system::error_code &ec)
{
if (!ec)
{
this->state = STATE_CONNECTED;
this->messageQueue.put(GWSocketMessageIn(IN_CONNECTED, "Connected"));
this->asyncRead();
checkWriting();
auto expected = STATE_CONNECTING;
if (this->state.compare_exchange_strong(expected, STATE_CONNECTED))
{
this->messageQueue.put(GWSocketMessageIn(IN_CONNECTED, "Connected"));
this->asyncRead();
checkWriting();
}
else
{
//In this case the socket has been closed somewhere else, make sure that it is definitely closed
//and socket does not end up in undefined state
this->closeNow();
}
}
else
{
@ -213,7 +222,7 @@ void GWSocket::open()
void GWSocket::checkWriting()
{
std::lock_guard<std::mutex> guard(this->queueMutex);
if ((this->state == STATE_CONNECTED || this->state == STATE_DISCONNECTING) && !writing && !this->writeQueue.empty())
if ((this->state == STATE_CONNECTED || this->state == SATE_DISCONNECT_REQUESTED) && !writing && !this->writeQueue.empty())
{
this->writing = true;
GWSocketMessageOut message = this->writeQueue.front();
@ -224,8 +233,20 @@ void GWSocket::checkWriting()
this->asyncWrite(message.message);
break;
case OUT_DISCONNECT:
this->asyncCloseSocket();
{
auto expected = SATE_DISCONNECT_REQUESTED;
if (this->state.compare_exchange_weak(expected, STATE_DISCONNECTING))
{
this->asyncCloseSocket();
}
else
{
//If this case is reached it must be the case that the socket was closed elsewhere before.
//This just ensures that we will definitely end up in a disconnected state
this->closeNow();
}
break;
}
default:
break;
}

View File

@ -17,6 +17,11 @@
#include <functional>
#include "BlockingQueue.h"
//This entire implementations is written with the idea in mind that the IO worker might
//be ran in a different thread than the server's main thread. This is currently not the case, which
//is why some functions might seem a bit overprotective when it comes to avoiding race condition that can't
//even happen when only using a single thread.
using tcp = boost::asio::ip::tcp;
namespace websocket = boost::beast::websocket;
@ -44,10 +49,11 @@ public:
enum SocketState
{
STATE_CONNECTING,
STATE_CONNECTED,
STATE_DISCONNECTING,
STATE_DISCONNECTED,
STATE_CONNECTING, // Started connecting to the server, i.e. the TCP connection/handshakes are being done
STATE_CONNECTED, //The connection is open and communication is possible
SATE_DISCONNECT_REQUESTED, //The socket has been requested to close orderly using the close method
STATE_DISCONNECTING, //The socket is currently in the process of disconnecting
STATE_DISCONNECTED //The socket is fully disconnected, be it after disconnecting or the initial state
};
class GWSocket

View File

@ -6,6 +6,7 @@
void WebSocket::asyncConnect(tcp::resolver::iterator it)
{
this->resetWS();
boost::asio::async_connect(this->getWS()->next_layer(), it, boost::bind(&WebSocket::socketConnected, this, boost::placeholders::_1, boost::placeholders::_2));
}