mutex fixes, server shutdown works again

cockatrice/src/localserverinterface.cpp

@@ -9,7 +9,6 @@ LocalServerInterface::LocalServerInterface(LocalServer *_server)
 
 LocalServerInterface::~LocalServerInterface()
 {
-	prepareDestroy();
 }
 
 void LocalServerInterface::transmitProtocolItem(const ServerMessage &item)

common/server.cpp

@@ -32,7 +32,7 @@
 #include <QDebug>
 
 Server::Server(QObject *parent)
-	: QObject(parent), nextGameId(0), nextReplayId(0)
+	: QObject(parent), nextGameId(0), nextReplayId(0), clientsLock(QReadWriteLock::Recursive)
 {
 	qRegisterMetaType<ServerInfo_Game>("ServerInfo_Game");
 	qRegisterMetaType<ServerInfo_Room>("ServerInfo_Room");
@@ -45,17 +45,17 @@ Server::~Server()
 
 void Server::prepareDestroy()
 {
+	clientsLock.lockForWrite();
+	while (!clients.isEmpty())
+		clients.first()->prepareDestroy();
+	clientsLock.unlock();
+	
 	roomsLock.lockForWrite();
 	QMapIterator<int, Server_Room *> roomIterator(rooms);
 	while (roomIterator.hasNext())
 		delete roomIterator.next().value();
 	rooms.clear();
 	roomsLock.unlock();
-	
-	clientsLock.lockForWrite();
-	while (!clients.isEmpty())
-		delete clients.takeFirst();
-	clientsLock.unlock();
 }
 
 AuthenticationResult Server::loginUser(Server_ProtocolHandler *session, QString &name, const QString &password, QString &reasonStr)

common/server_game.cpp

@@ -307,6 +307,12 @@ void Server_Game::stopGameIfFinished()
 
 Response::ResponseCode Server_Game::checkJoin(ServerInfo_User *user, const QString &_password, bool spectator, bool overrideRestrictions)
 {
+	{
+		QMapIterator<int, Server_Player *> playerIterator(players);
+		while (playerIterator.hasNext())
+			if (playerIterator.next().value()->getUserInfo()->name() == user->name())
+				return Response::RespContextError;
+	}
 	if (!(overrideRestrictions && (user->user_level() & ServerInfo_User::IsModerator))) {
 		if ((_password != password) && !(spectator && !spectatorsNeedPassword))
 			return Response::RespWrongPassword;

common/server_protocolhandler.cpp

@@ -98,9 +98,6 @@ Server_ProtocolHandler::~Server_ProtocolHandler()
 {
 }
 
-// This is essentially the destructor, but it needs to be called from the
-// child's destructor so that the server mutex does not get unlocked during
-// finalization.
 void Server_ProtocolHandler::prepareDestroy()
 {
 	qDebug("Server_ProtocolHandler::prepareDestroy");
@@ -146,6 +143,8 @@ void Server_ProtocolHandler::prepareDestroy()
 		r->gamesMutex.unlock();
 	}
 	server->roomsLock.unlock();
+	
+	deleteLater();
 }
 
 void Server_ProtocolHandler::playerRemovedFromGame(Server_Game *game)
@@ -437,7 +436,7 @@ void Server_ProtocolHandler::pingClockTimeout()
 	}
 	
 	if (timeRunning - lastDataReceived > server->getMaxPlayerInactivityTime())
-		deleteLater();
+		prepareDestroy();
 	++timeRunning;
 }
 
@@ -486,7 +485,6 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
 	server->roomsLock.lockForRead();
 	
 	QMapIterator<int, Server_Room *> roomIterator(server->getRooms());
-	gameListMutex.lock();
 	while (roomIterator.hasNext()) {
 		Server_Room *room = roomIterator.next().value();
 		room->gamesMutex.lock();
@@ -498,7 +496,10 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
 			for (int j = 0; j < gamePlayers.size(); ++j)
 				if (gamePlayers[j]->getUserInfo()->name() == userInfo->name()) {
 					gamePlayers[j]->setProtocolHandler(this);
+					
+					gameListMutex.lock();
 					games.insert(game->getGameId(), QPair<int, int>(room->getId(), gamePlayers[j]->getPlayerId()));
+					gameListMutex.unlock();
 					
 					Event_GameJoined event1;
 					event1.set_game_id(game->getGameId());
@@ -526,7 +527,6 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd
 		}
 		room->gamesMutex.unlock();
 	}
-	gameListMutex.unlock();
 	server->roomsLock.unlock();
 	
 	rc.setResponseExtension(re);
@@ -777,11 +777,6 @@ Response::ResponseCode Server_ProtocolHandler::cmdJoinGame(const Command_JoinGam
 	if (authState == NotLoggedIn)
 		return Response::RespLoginNeeded;
 	
-	QMutexLocker locker(&gameListMutex);
-	
-	if (games.contains(cmd.game_id()))
-		return Response::RespContextError;
-	
 	room->gamesMutex.lock();
 	Server_Game *g = room->getGames().value(cmd.game_id());
 	if (!g) {
@@ -795,7 +790,9 @@ Response::ResponseCode Server_ProtocolHandler::cmdJoinGame(const Command_JoinGam
 	if (result == Response::RespOk) {
 		Server_Player *player = g->addPlayer(this, cmd.spectator());
 		
+		gameListMutex.lock();
 		games.insert(cmd.game_id(), QPair<int, int>(room->getId(), player->getPlayerId()));
+		gameListMutex.unlock();
 		
 		Event_GameJoined event1;
 		event1.set_game_id(g->getGameId());

common/server_protocolhandler.h

@@ -95,7 +95,6 @@ protected:
 	bool acceptsUserListChanges;
 	bool acceptsRoomListChanges;
 	
-	void prepareDestroy();
 	int sessionId;
 private:
 	QMutex gameListMutex;
@@ -174,6 +173,8 @@ private slots:
 	void pingClockTimeout();
 signals:
 	void logDebugMessage(const QString &message, Server_ProtocolHandler *session);
+public slots:
+	void prepareDestroy();
 public:
 	Server_ProtocolHandler(Server *_server, QObject *parent = 0);
 	~Server_ProtocolHandler();

servatrice/src/main.cpp

@@ -128,7 +128,6 @@ int main(int argc, char *argv[])
 	loggerThread = new QThread;
 	logger = new ServerLogger;
 	logger->moveToThread(loggerThread);
-	QObject::connect(logger, SIGNAL(destroyed()), loggerThread, SLOT(quit()));
 	
 	loggerThread->start();
 	QMetaObject::invokeMethod(logger, "startLog", Qt::BlockingQueuedConnection, Q_ARG(QString, settings->value("server/logfile").toString()));

servatrice/src/servatrice.h

@@ -82,6 +82,8 @@ class Servatrice : public Server
 private slots:
 	void statusUpdate();
 	void shutdownTimeout();
+public slots:
+	void scheduleShutdown(const QString &reason, int minutes);
 public:
 	mutable QMutex dbMutex;
 	Servatrice(QSettings *_settings, QObject *parent = 0);
@@ -110,7 +112,6 @@ public:
 	QMap<QString, ServerInfo_User> getIgnoreList(const QString &name);
 	bool isInBuddyList(const QString &whoseList, const QString &who);
 	bool isInIgnoreList(const QString &whoseList, const QString &who);
-	void scheduleShutdown(const QString &reason, int minutes);
 	void incTxBytes(quint64 num);
 	void incRxBytes(quint64 num);
 	int getUserIdInDB(const QString &name);

servatrice/src/server_logger.cpp

@@ -16,6 +16,8 @@ ServerLogger::ServerLogger(QObject *parent)
 ServerLogger::~ServerLogger()
 {
 	flushBuffer();
+	// This does not work with the destroyed() signal as this destructor is called after the main event loop is done.
+	thread()->quit();
 }
 
 void ServerLogger::startLog(const QString &logFileName)

servatrice/src/serversocketinterface.cpp

@@ -60,8 +60,9 @@ static const int protocolVersion = 13;
 ServerSocketInterface::ServerSocketInterface(Servatrice *_server, QTcpSocket *_socket, QObject *parent)
 	: Server_ProtocolHandler(_server, parent), servatrice(_server), socket(_socket), messageInProgress(false)
 {
+	bool success = true;
+	
 	connect(socket, SIGNAL(readyRead()), this, SLOT(readClient()));
-	connect(socket, SIGNAL(disconnected()), this, SLOT(deleteLater()));
 	connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(catchSocketError(QAbstractSocket::SocketError)));
 	connect(this, SIGNAL(outputBufferChanged()), this, SLOT(flushOutputBuffer()), Qt::QueuedConnection);
 	
@@ -81,18 +82,19 @@ ServerSocketInterface::ServerSocketInterface(Servatrice *_server, QTcpSocket *_s
 		sendProtocolItem(*se);
 		delete se;
 		
-		deleteLater();
+		success = false;
 	}
 	
 	server->addClient(this);
+	
+	if (!success)
+		prepareDestroy();
 }
 
 ServerSocketInterface::~ServerSocketInterface()
 {
 	logger->logMessage("ServerSocketInterface destructor", this);
 	
-	prepareDestroy();
-	
 	flushOutputBuffer();
 	delete socket;
 	socket = 0;
@@ -143,7 +145,7 @@ void ServerSocketInterface::catchSocketError(QAbstractSocket::SocketError socket
 {
 	qDebug() << "Socket error:" << socketError;
 	
-	deleteLater();
+	prepareDestroy();
 }
 
 void ServerSocketInterface::transmitProtocolItem(const ServerMessage &item)
@@ -623,7 +625,7 @@ Response::ResponseCode ServerSocketInterface::cmdBanFromServer(const Command_Ban
 		for (int i = 0; i < userList.size(); ++i) {
 			SessionEvent *se = userList[i]->prepareSessionEvent(event);
 			userList[i]->sendProtocolItem(*se);
-			userList[i]->deleteLater();
+			userList[i]->prepareDestroy();
 			delete se;
 		}
 	}