From d30691559aa5d6042d8ff11767da92afdab0e344 Mon Sep 17 00:00:00 2001 From: ctrlaltca Date: Fri, 24 Apr 2020 22:26:59 +0200 Subject: [PATCH] Some improvements to Servatice network code (#3969) * Some improvements to Servatice network code 1. fix crash on fuzzy connection (tcp server only) 2. ensure websockets are parent()ed to avoid leaking them 3. quick catch disconnect()ed sockets instead of waiting for a socket error to happen 4. supporto mulltiple connection pools on the websocket server; they are still bound to the same thread due to a qt5 limitation. --- servatrice/servatrice.ini.example | 4 +-- servatrice/src/servatrice.cpp | 31 +++++++++++++++++------- servatrice/src/serversocketinterface.cpp | 10 +++++++- servatrice/src/serversocketinterface.h | 1 + 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index b0e72cee..9da88779 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -29,12 +29,12 @@ port=4747 ; Set to 0 to disable the tcp server. number_pools=1 -; Servatrice can listen for clients on websockets, too. Unfortunately it can't support more than one thread. +; Servatrice can listen for clients on websockets, too. Multiple connection pools are available but +; unfortunately, due to a Qt limitation, they must run in the same execution thread. ; Set to 0 to disable the websocket server. websocket_number_pools=1 ; The IP address servatrice will listen on for websockets clients; defaults to "any" - websocket_host=any ; The TCP port number servatrice will listen on for websockets clients; default is 4748 diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index ea464005..3264d140 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -111,21 +111,30 @@ Servatrice_ConnectionPool *Servatrice_GameServer::findLeastUsedConnectionPool() #define WEBSOCKET_POOL_NUMBER 999 Servatrice_WebsocketGameServer::Servatrice_WebsocketGameServer(Servatrice *_server, - int /* _numberPools */, + int _numberPools, const QSqlDatabase &_sqlDatabase, QObject *parent) : QWebSocketServer("Servatrice", QWebSocketServer::NonSecureMode, parent), server(_server) { - // Qt limitation: websockets can't be moved to another thread - auto newDatabaseInterface = new Servatrice_DatabaseInterface(WEBSOCKET_POOL_NUMBER, server); - auto newPool = new Servatrice_ConnectionPool(newDatabaseInterface); + for (int i = 0; i < _numberPools; ++i) { + int poolNumber = WEBSOCKET_POOL_NUMBER + i; + auto newDatabaseInterface = new Servatrice_DatabaseInterface(poolNumber, server); + auto newPool = new Servatrice_ConnectionPool(newDatabaseInterface); - server->addDatabaseInterface(thread(), newDatabaseInterface); - newDatabaseInterface->initDatabase(_sqlDatabase); + auto newThread = new QThread; + newThread->setObjectName("pool_" + QString::number(poolNumber)); + newPool->moveToThread(newThread); + newDatabaseInterface->moveToThread(newThread); + server->addDatabaseInterface(newThread, newDatabaseInterface); - connectionPools.append(newPool); + newThread->start(); + QMetaObject::invokeMethod(newDatabaseInterface, "initDatabase", Qt::BlockingQueuedConnection, + Q_ARG(QSqlDatabase, _sqlDatabase)); - connect(this, SIGNAL(newConnection()), this, SLOT(onNewConnection())); + connectionPools.append(newPool); + + connect(this, SIGNAL(newConnection()), this, SLOT(onNewConnection())); + } } Servatrice_WebsocketGameServer::~Servatrice_WebsocketGameServer() @@ -143,7 +152,11 @@ void Servatrice_WebsocketGameServer::onNewConnection() Servatrice_ConnectionPool *pool = findLeastUsedConnectionPool(); auto ssi = new WebsocketServerSocketInterface(server, pool->getDatabaseInterface()); - // ssi->moveToThread(pool->thread()); + /* + * Due to a Qt limitation, websockets can't be moved to another thread. + * This will hopefully change in Qt6 if QtWebSocket will be integrated in QtNetwork + */ + // ssi->moveToThread(pool->thread()); pool->addClient(); connect(ssi, SIGNAL(destroyed()), pool, SLOT(removeClient())); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 77e99d6a..ddf6c6ef 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -122,6 +122,11 @@ void AbstractServerSocketInterface::catchSocketError(QAbstractSocket::SocketErro prepareDestroy(); } +void AbstractServerSocketInterface::catchSocketDisconnected() +{ + prepareDestroy(); +} + void AbstractServerSocketInterface::transmitProtocolItem(const ServerMessage &item) { outputQueueMutex.lock(); @@ -1511,6 +1516,7 @@ TcpServerSocketInterface::TcpServerSocketInterface(Servatrice *_server, connect(socket, SIGNAL(readyRead()), this, SLOT(readClient())); connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(catchSocketError(QAbstractSocket::SocketError))); + connect(socket, SIGNAL(disconnected()), this, SLOT(catchSocketDisconnected())); } TcpServerSocketInterface::~TcpServerSocketInterface() @@ -1594,7 +1600,7 @@ void TcpServerSocketInterface::readClient() } else return; } - if (inputBuffer.size() < messageLength) + if (inputBuffer.size() < messageLength || messageLength < 0) return; CommandContainer newCommandContainer; @@ -1679,6 +1685,7 @@ WebsocketServerSocketInterface::~WebsocketServerSocketInterface() void WebsocketServerSocketInterface::initConnection(void *_socket) { socket = (QWebSocket *)_socket; + socket->setParent(this); address = socket->peerAddress(); QByteArray websocketIPHeader = settingsCache->value("server/web_socket_ip_header", "").toByteArray(); @@ -1699,6 +1706,7 @@ void WebsocketServerSocketInterface::initConnection(void *_socket) SLOT(binaryMessageReceived(const QByteArray &))); connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(catchSocketError(QAbstractSocket::SocketError))); + connect(socket, SIGNAL(disconnected()), this, SLOT(catchSocketDisconnected())); // Add this object to the server's list of connections before it can receive socket events. // Otherwise, in case a of a socket error, it could be removed from the list before it is added. diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index 71c59dac..722e1e0b 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -59,6 +59,7 @@ class AbstractServerSocketInterface : public Server_ProtocolHandler Q_OBJECT protected slots: void catchSocketError(QAbstractSocket::SocketError socketError); + void catchSocketDisconnected(); virtual void flushOutputQueue() = 0; signals: void outputQueueChanged();