From 429eb2a1fd68de89714db224ac2af4f7ea3c04dc Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:13:21 +0100 Subject: [PATCH] [Servatrice] Use compile-time checked connections (#7631) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Servatrice] Use compile-time checked connections Replace the remaining SIGNAL/SLOT string connects in servatrice.cpp, smtpclient.cpp, serversocketinterface.cpp and isl_interface.cpp with pointer-to-member or lambda connects, and convert the string invokeMethod calls to the functor overload. QxtSmtp has overloaded signals, so those need qOverload. Slots inherited as protected members and the QWebSocket error connection owned by the ISL fix are left on lambdas/strings as appropriate; the vendored qxtsmtp.cpp is untouched. * Fix remaining review threads on the connection syntax - Connect the IslInterface signals straight to the inherited Server slots instead of forwarding lambdas. - Capture servatrice explicitly in the queued admin lambdas so they do not read this->servatrice after teardown. - Move the websocket newConnection connect out of the per-pool loop. - Terminate the logging category and restore pendingKey's indentation. --------- Co-authored-by: Lukas Brübach --- servatrice/src/isl_interface.cpp | 4 +- servatrice/src/servatrice.cpp | 79 ++++++++++++------------ servatrice/src/serversocketinterface.cpp | 31 +++++----- servatrice/src/smtpclient.cpp | 35 +++++------ 4 files changed, 76 insertions(+), 73 deletions(-) diff --git a/servatrice/src/isl_interface.cpp b/servatrice/src/isl_interface.cpp index 8635808ac..1898cd5fa 100644 --- a/servatrice/src/isl_interface.cpp +++ b/servatrice/src/isl_interface.cpp @@ -58,9 +58,9 @@ void IslInterface::sharedCtor(const QSslCertificate &cert, const QSslKey &privat socket->setLocalCertificate(cert); socket->setPrivateKey(privateKey); - connect(socket, SIGNAL(readyRead()), this, SLOT(readClient()), Qt::QueuedConnection); + connect(socket, &QSslSocket::readyRead, this, &IslInterface::readClient, Qt::QueuedConnection); connect(socket, &QAbstractSocket::errorOccurred, this, &IslInterface::catchSocketError); - connect(this, SIGNAL(outputBufferChanged()), this, SLOT(flushOutputBuffer()), Qt::QueuedConnection); + connect(this, &IslInterface::outputBufferChanged, this, &IslInterface::flushOutputBuffer, Qt::QueuedConnection); } IslInterface::IslInterface(int _socketDescriptor, diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index 49d8e1e4c..afe028f32 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -100,8 +100,10 @@ Servatrice_GameServer::Servatrice_GameServer(Servatrice *_server, newThread->start(); server->watchWorkerThread(newThread); - QMetaObject::invokeMethod(newDatabaseInterface, "initDatabase", Qt::BlockingQueuedConnection, - Q_ARG(QSqlDatabase, _sqlDatabase)); + QMetaObject::invokeMethod( + newDatabaseInterface, + [newDatabaseInterface, _sqlDatabase] { newDatabaseInterface->initDatabase(_sqlDatabase); }, + Qt::BlockingQueuedConnection); connectionPools.append(newPool); } @@ -125,9 +127,10 @@ void Servatrice_GameServer::incomingConnection(qintptr socketDescriptor) auto ssi = new TcpServerSocketInterface(server, pool->getDatabaseInterface()); ssi->moveToThread(pool->thread()); pool->addClient(); - connect(ssi, SIGNAL(destroyed()), pool, SLOT(removeClient())); + connect(ssi, &QObject::destroyed, pool, &Servatrice_ConnectionPool::removeClient); - QMetaObject::invokeMethod(ssi, "initConnection", Qt::QueuedConnection, Q_ARG(int, socketDescriptor)); + QMetaObject::invokeMethod( + ssi, [ssi, socketDescriptor] { ssi->initConnection(socketDescriptor); }, Qt::QueuedConnection); } Servatrice_ConnectionPool *Servatrice_GameServer::findLeastUsedConnectionPool() @@ -168,13 +171,15 @@ Servatrice_WebsocketGameServer::Servatrice_WebsocketGameServer(Servatrice *_serv newThread->start(); server->watchWorkerThread(newThread); - QMetaObject::invokeMethod(newDatabaseInterface, "initDatabase", Qt::BlockingQueuedConnection, - Q_ARG(QSqlDatabase, _sqlDatabase)); + QMetaObject::invokeMethod( + newDatabaseInterface, + [newDatabaseInterface, _sqlDatabase] { newDatabaseInterface->initDatabase(_sqlDatabase); }, + Qt::BlockingQueuedConnection); connectionPools.append(newPool); - - connect(this, SIGNAL(newConnection()), this, SLOT(onNewConnection())); } + + connect(this, &QWebSocketServer::newConnection, this, &Servatrice_WebsocketGameServer::onNewConnection); } Servatrice_WebsocketGameServer::~Servatrice_WebsocketGameServer() @@ -199,9 +204,10 @@ void Servatrice_WebsocketGameServer::onNewConnection() */ // ssi->moveToThread(pool->thread()); pool->addClient(); - connect(ssi, SIGNAL(destroyed()), pool, SLOT(removeClient())); + connect(ssi, &QObject::destroyed, pool, &Servatrice_ConnectionPool::removeClient); - QMetaObject::invokeMethod(ssi, "initConnection", Qt::QueuedConnection, Q_ARG(void *, nextPendingConnection())); + void *socket = nextPendingConnection(); + QMetaObject::invokeMethod(ssi, [ssi, socket] { ssi->initConnection(socket); }, Qt::QueuedConnection); } Servatrice_ConnectionPool *Servatrice_WebsocketGameServer::findLeastUsedConnectionPool() @@ -224,14 +230,14 @@ Servatrice_ConnectionPool *Servatrice_WebsocketGameServer::findLeastUsedConnecti void Servatrice_IslServer::incomingConnection(qintptr socketDescriptor) { auto thread = new QThread; - connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater())); + connect(thread, &QThread::finished, thread, &QObject::deleteLater); auto interface = new IslInterface(static_cast(socketDescriptor), cert, privateKey, server); interface->moveToThread(thread); - connect(interface, SIGNAL(destroyed()), thread, SLOT(quit())); + connect(interface, &QObject::destroyed, thread, &QThread::quit); thread->start(); - QMetaObject::invokeMethod(interface, "initServer", Qt::QueuedConnection); + QMetaObject::invokeMethod(interface, [interface] { interface->initServer(); }, Qt::QueuedConnection); } Servatrice::Servatrice(QObject *parent) @@ -585,15 +591,16 @@ bool Servatrice::initServer() auto *thread = new QThread; thread->setObjectName("isl_" + QString::number(prop.id)); - connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater())); + connect(thread, &QThread::finished, thread, &QObject::deleteLater); IslInterface *interface = new IslInterface(prop.id, prop.hostname, prop.address.toString(), prop.controlPort, prop.cert, cert, key, this); interface->moveToThread(thread); - connect(interface, SIGNAL(destroyed()), thread, SLOT(quit())); + connect(interface, &QObject::destroyed, thread, &QThread::quit); thread->start(); - QMetaObject::invokeMethod(interface, "initClient", Qt::BlockingQueuedConnection); + QMetaObject::invokeMethod( + interface, [interface] { interface->initClient(); }, Qt::BlockingQueuedConnection); } qDebug() << "Starting ISL server on port" << getISLNetworkPort(); @@ -610,18 +617,18 @@ bool Servatrice::initServer() } pingClock = new QTimer(this); - connect(pingClock, SIGNAL(timeout()), this, SIGNAL(pingClockTimeout())); + connect(pingClock, &QTimer::timeout, this, &Servatrice::pingClockTimeout); pingClock->start(getClientKeepAlive() * 1000); statusUpdateClock = new QTimer(this); - connect(statusUpdateClock, SIGNAL(timeout()), this, SLOT(statusUpdate())); + connect(statusUpdateClock, &QTimer::timeout, this, &Servatrice::statusUpdate); if (getServerStatusUpdateTime() != 0) { qDebug() << "Starting status update clock, interval" << getServerStatusUpdateTime() << "ms"; statusUpdateClock->start(getServerStatusUpdateTime()); } deckShareCleanupClock = new QTimer(this); - connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(cleanupExpiredDeckShares())); + connect(deckShareCleanupClock, &QTimer::timeout, this, &Servatrice::cleanupExpiredDeckShares); const int deckShareCleanupInterval = getDeckShareCleanupInterval(); if (deckShareCleanupInterval > 0) { qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms"; @@ -1038,7 +1045,7 @@ void Servatrice::scheduleShutdown(const QString &reason, int minutes) nextShutdownMessageMinutes = shutdownMinutes; if (minutes > 0) { shutdownTimer = new QTimer; - connect(shutdownTimer, SIGNAL(timeout()), this, SLOT(shutdownTimeout())); + connect(shutdownTimer, &QTimer::timeout, this, &Servatrice::shutdownTimeout); shutdownTimer->start(60000); } shutdownStateMutex.unlock(); @@ -1101,25 +1108,19 @@ void Servatrice::addIslInterface(int _serverId, IslInterface *interface) { // Only call with islLock locked for writing islInterfaces.insert(_serverId, interface); - connect(interface, SIGNAL(externalUserJoined(ServerInfo_User)), this, SLOT(externalUserJoined(ServerInfo_User))); - connect(interface, SIGNAL(externalUserLeft(QString)), this, SLOT(externalUserLeft(QString))); - connect(interface, SIGNAL(externalRoomUserJoined(int, ServerInfo_User)), this, - SLOT(externalRoomUserJoined(int, ServerInfo_User))); - connect(interface, SIGNAL(externalRoomUserLeft(int, QString)), this, SLOT(externalRoomUserLeft(int, QString))); - connect(interface, SIGNAL(externalRoomSay(int, QString, QString, QString)), this, - SLOT(externalRoomSay(int, QString, QString, QString))); - connect(interface, SIGNAL(externalRoomRemoveMessages(int, QString, int)), this, - SLOT(externalRoomRemoveMessages(int, QString, int))); - connect(interface, SIGNAL(externalRoomGameListChanged(int, ServerInfo_Game)), this, - SLOT(externalRoomGameListChanged(int, ServerInfo_Game))); - connect(interface, SIGNAL(joinGameCommandReceived(Command_JoinGame, int, int, int, qint64)), this, - SLOT(externalJoinGameCommandReceived(Command_JoinGame, int, int, int, qint64))); - connect(interface, SIGNAL(gameCommandContainerReceived(CommandContainer, int, int, qint64)), this, - SLOT(externalGameCommandContainerReceived(CommandContainer, int, int, qint64))); - connect(interface, SIGNAL(responseReceived(Response, qint64)), this, - SLOT(externalResponseReceived(Response, qint64))); - connect(interface, SIGNAL(gameEventContainerReceived(GameEventContainer, qint64)), this, - SLOT(externalGameEventContainerReceived(GameEventContainer, qint64))); + connect(interface, &IslInterface::externalUserJoined, this, &Servatrice::externalUserJoined); + connect(interface, &IslInterface::externalUserLeft, this, &Servatrice::externalUserLeft); + connect(interface, &IslInterface::externalRoomUserJoined, this, &Servatrice::externalRoomUserJoined); + connect(interface, &IslInterface::externalRoomUserLeft, this, &Servatrice::externalRoomUserLeft); + connect(interface, &IslInterface::externalRoomSay, this, &Servatrice::externalRoomSay); + connect(interface, &IslInterface::externalRoomRemoveMessages, this, &Servatrice::externalRoomRemoveMessages); + connect(interface, &IslInterface::externalRoomGameListChanged, this, &Servatrice::externalRoomGameListChanged); + connect(interface, &IslInterface::joinGameCommandReceived, this, &Servatrice::externalJoinGameCommandReceived); + connect(interface, &IslInterface::gameCommandContainerReceived, this, + &Servatrice::externalGameCommandContainerReceived); + connect(interface, &IslInterface::responseReceived, this, &Servatrice::externalResponseReceived); + connect(interface, &IslInterface::gameEventContainerReceived, this, + &Servatrice::externalGameEventContainerReceived); } void Servatrice::removeIslInterface(int _serverId) diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index f0c0c3536..67e807380 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -199,7 +199,8 @@ AbstractServerSocketInterface::AbstractServerSocketInterface(Servatrice *_server // Never call flushOutputQueue directly from outputQueueChanged. In case of a socket error, // it could lead to this object being destroyed while another function is still on the call stack. -> mutex // deadlocks etc. - connect(this, SIGNAL(outputQueueChanged()), this, SLOT(flushOutputQueue()), Qt::QueuedConnection); + connect(this, &AbstractServerSocketInterface::outputQueueChanged, this, + &AbstractServerSocketInterface::flushOutputQueue, Qt::QueuedConnection); } bool AbstractServerSocketInterface::initSession() @@ -1982,7 +1983,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com SessionEvent *se = userList[i]->prepareSessionEvent(event); userList[i]->sendProtocolItem(*se); delete se; - QMetaObject::invokeMethod(userList[i], "prepareDestroy", Qt::QueuedConnection); + QMetaObject::invokeMethod( + userList[i], [user = userList[i]] { user->prepareDestroy(); }, Qt::QueuedConnection); } } @@ -4191,15 +4193,15 @@ Response::ResponseCode AbstractServerSocketInterface::cmdUpdateServerMessage(const Command_UpdateServerMessage & /*cmd*/, ResponseContainer & /*rc*/) { - QMetaObject::invokeMethod(server, "updateLoginMessage"); + QMetaObject::invokeMethod(servatrice, [srv = servatrice] { srv->updateLoginMessage(); }); return Response::RespOk; } Response::ResponseCode AbstractServerSocketInterface::cmdShutdownServer(const Command_ShutdownServer &cmd, ResponseContainer & /*rc*/) { - QMetaObject::invokeMethod(server, "scheduleShutdown", Q_ARG(QString, textFromStdString(cmd.reason())), - Q_ARG(int, cmd.minutes())); + QMetaObject::invokeMethod(servatrice, [srv = servatrice, reason = textFromStdString(cmd.reason()), + minutes = cmd.minutes()] { srv->scheduleShutdown(reason, minutes); }); return Response::RespOk; } @@ -4208,7 +4210,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReloadConfig(const Comm { logDebugMessage("Received admin command: reloading configuration"); settingsCache->sync(); - QMetaObject::invokeMethod(server, "setRequiredFeatures", Q_ARG(QString, server->getRequiredFeatures())); + QMetaObject::invokeMethod(servatrice, [srv = servatrice, features = servatrice->getRequiredFeatures()] { + srv->setRequiredFeatures(features); + }); return Response::RespOk; } @@ -4260,7 +4264,7 @@ bool AbstractServerSocketInterface::removeAdminFlagFromUser(const QString &userN delete se; } - QMetaObject::invokeMethod(user, "prepareDestroy", Qt::QueuedConnection); + QMetaObject::invokeMethod(user, [user] { user->prepareDestroy(); }, Qt::QueuedConnection); return true; } @@ -4435,10 +4439,10 @@ TcpServerSocketInterface::TcpServerSocketInterface(Servatrice *_server, { socket = new QTcpSocket(this); socket->setSocketOption(QAbstractSocket::LowDelayOption, 1); - connect(socket, SIGNAL(readyRead()), this, SLOT(readClient())); - connect(socket, SIGNAL(disconnected()), this, SLOT(catchSocketDisconnected())); - connect(socket, SIGNAL(errorOccurred(QAbstractSocket::SocketError)), this, - SLOT(catchSocketError(QAbstractSocket::SocketError))); + connect(socket, &QTcpSocket::readyRead, this, [this] { readClient(); }); + connect(socket, &QTcpSocket::disconnected, this, [this] { catchSocketDisconnected(); }); + connect(socket, &QTcpSocket::errorOccurred, this, + [this](QAbstractSocket::SocketError error) { catchSocketError(error); }); } TcpServerSocketInterface::~TcpServerSocketInterface() @@ -4660,8 +4664,7 @@ void WebsocketServerSocketInterface::initConnection(void *_socket) } } - connect(socket, SIGNAL(binaryMessageReceived(const QByteArray &)), this, - SLOT(binaryMessageReceived(const QByteArray &))); + connect(socket, &QWebSocket::binaryMessageReceived, this, &WebsocketServerSocketInterface::binaryMessageReceived); // QWebSocket::errorOccurred was added in Qt 6.5 (error() is deprecated from 6.5 on), // so a typed connect cannot compile against the 6.4 floor. #if QT_VERSION >= QT_VERSION_CHECK(6, 5, 0) @@ -4671,7 +4674,7 @@ void WebsocketServerSocketInterface::initConnection(void *_socket) connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(catchSocketError(QAbstractSocket::SocketError))); #endif - connect(socket, SIGNAL(disconnected()), this, SLOT(catchSocketDisconnected())); + connect(socket, &QWebSocket::disconnected, this, [this] { catchSocketDisconnected(); }); // Add this object to the server's list of connections before it can receive socket events. // Otherwise, in case of a socket error, it could be removed from the list before it is added. diff --git a/servatrice/src/smtpclient.cpp b/servatrice/src/smtpclient.cpp index 82824e496..bfdfae905 100644 --- a/servatrice/src/smtpclient.cpp +++ b/servatrice/src/smtpclient.cpp @@ -25,7 +25,7 @@ static const qint64 IDLE_TIMEOUT_MS = 60000; // 60 seconds // MAX_RETRY_DELAY cap is applied, preventing unbounded intermediate values. static const int MAX_BACKOFF_SHIFT = 10; -inline Q_LOGGING_CATEGORY(SmtpClientLog, "smtp_client") +inline Q_LOGGING_CATEGORY(SmtpClientLog, "smtp_client"); static QString pendingKey(const QString &userName, EmailType type, const QString &token) { @@ -55,24 +55,23 @@ SmtpClient::SmtpClient(QObject *parent) : QObject(parent) idleTimer = new QTimer(this); idleTimer->setSingleShot(true); - connect(idleTimer, SIGNAL(timeout()), this, SLOT(idleTimeout())); + connect(idleTimer, &QTimer::timeout, this, &SmtpClient::idleTimeout); - connect(smtp, SIGNAL(authenticated()), this, SLOT(authenticated())); - connect(smtp, SIGNAL(authenticationFailed(const QByteArray &)), this, - SLOT(authenticationFailed(const QByteArray &))); - connect(smtp, SIGNAL(connected()), this, SLOT(connected())); - connect(smtp, SIGNAL(connectionFailed(const QByteArray &)), this, SLOT(connectionFailed(const QByteArray &))); - connect(smtp, SIGNAL(disconnected()), this, SLOT(disconnected())); - connect(smtp, SIGNAL(encrypted()), this, SLOT(encrypted())); - connect(smtp, SIGNAL(encryptionFailed(const QByteArray &)), this, SLOT(encryptionFailed(const QByteArray &))); - connect(smtp, SIGNAL(finished()), this, SLOT(finished())); - connect(smtp, SIGNAL(mailFailed(int, int, const QByteArray &)), this, - SLOT(mailFailed(int, int, const QByteArray &))); - connect(smtp, SIGNAL(mailSent(int)), this, SLOT(mailSent(int))); - connect(smtp, SIGNAL(recipientRejected(int, const QString &, const QByteArray &)), this, - SLOT(recipientRejected(int, const QString &, const QByteArray &))); - connect(smtp, SIGNAL(senderRejected(int, const QString &, const QByteArray &)), this, - SLOT(senderRejected(int, const QString &, const QByteArray &))); + connect(smtp, &QxtSmtp::authenticated, this, &SmtpClient::authenticated); + connect(smtp, qOverload(&QxtSmtp::authenticationFailed), this, + &SmtpClient::authenticationFailed); + connect(smtp, &QxtSmtp::connected, this, &SmtpClient::connected); + connect(smtp, qOverload(&QxtSmtp::connectionFailed), this, &SmtpClient::connectionFailed); + connect(smtp, &QxtSmtp::disconnected, this, &SmtpClient::disconnected); + connect(smtp, &QxtSmtp::encrypted, this, &SmtpClient::encrypted); + connect(smtp, qOverload(&QxtSmtp::encryptionFailed), this, &SmtpClient::encryptionFailed); + connect(smtp, &QxtSmtp::finished, this, &SmtpClient::finished); + connect(smtp, qOverload(&QxtSmtp::mailFailed), this, &SmtpClient::mailFailed); + connect(smtp, &QxtSmtp::mailSent, this, &SmtpClient::mailSent); + connect(smtp, qOverload(&QxtSmtp::recipientRejected), this, + &SmtpClient::recipientRejected); + connect(smtp, qOverload(&QxtSmtp::senderRejected), this, + &SmtpClient::senderRejected); } SmtpClient::~SmtpClient()