From 2d8b12a5761f8d86d605ad7cd9cca3a5536f3a5b Mon Sep 17 00:00:00 2001 From: Max-Wilhelm Bruker Date: Sun, 17 Apr 2011 15:46:47 +0200 Subject: [PATCH] reduced game selector minimum height; server race conditions fixed --- cockatrice/src/localserverinterface.cpp | 2 +- cockatrice/src/tab_room.cpp | 2 +- common/server_game.cpp | 16 ++++++------ common/server_game.h | 1 + common/server_protocolhandler.cpp | 32 +++++++++++++----------- common/server_protocolhandler.h | 7 ++---- common/server_room.cpp | 15 ++++++----- common/server_room.h | 5 +--- servatrice/src/serversocketinterface.cpp | 5 +--- 9 files changed, 40 insertions(+), 45 deletions(-) diff --git a/cockatrice/src/localserverinterface.cpp b/cockatrice/src/localserverinterface.cpp index c4d41379..79734c40 100644 --- a/cockatrice/src/localserverinterface.cpp +++ b/cockatrice/src/localserverinterface.cpp @@ -9,7 +9,7 @@ LocalServerInterface::LocalServerInterface(LocalServer *_server) LocalServerInterface::~LocalServerInterface() { - server->removeClient(this); + prepareDestroy(); } void LocalServerInterface::sendProtocolItem(ProtocolItem *item, bool deleteItem) diff --git a/cockatrice/src/tab_room.cpp b/cockatrice/src/tab_room.cpp index 4e7770ae..efdc0e8a 100644 --- a/cockatrice/src/tab_room.cpp +++ b/cockatrice/src/tab_room.cpp @@ -49,7 +49,7 @@ GameSelector::GameSelector(AbstractClient *_client, TabRoom *_room, QWidget *par setLayout(mainLayout); setMinimumWidth((qreal) (gameListView->columnWidth(0) * gameListModel->columnCount()) / 1.5); - setMinimumHeight(400); + setMinimumHeight(200); connect(showFullGamesCheckBox, SIGNAL(stateChanged(int)), this, SLOT(showFullGamesChanged(int))); connect(createButton, SIGNAL(clicked()), this, SLOT(actCreate())); diff --git a/common/server_game.cpp b/common/server_game.cpp index 2c1e52b2..cf65553e 100644 --- a/common/server_game.cpp +++ b/common/server_game.cpp @@ -28,12 +28,12 @@ #include #include -Server_Game::Server_Game(Server_ProtocolHandler *_creator, int _gameId, const QString &_description, const QString &_password, int _maxPlayers, const QList &_gameTypes, bool _onlyBuddies, bool _onlyRegistered, bool _spectatorsAllowed, bool _spectatorsNeedPassword, bool _spectatorsCanTalk, bool _spectatorsSeeEverything, Server_Room *parent) - : QObject(parent), creatorInfo(new ServerInfo_User(_creator->getUserInfo())), gameStarted(false), gameId(_gameId), description(_description), password(_password), maxPlayers(_maxPlayers), gameTypes(_gameTypes), activePlayer(-1), activePhase(-1), onlyBuddies(_onlyBuddies), onlyRegistered(_onlyRegistered), spectatorsAllowed(_spectatorsAllowed), spectatorsNeedPassword(_spectatorsNeedPassword), spectatorsCanTalk(_spectatorsCanTalk), spectatorsSeeEverything(_spectatorsSeeEverything), inactivityCounter(0), secondsElapsed(0), gameMutex(QMutex::Recursive) +Server_Game::Server_Game(Server_ProtocolHandler *_creator, int _gameId, const QString &_description, const QString &_password, int _maxPlayers, const QList &_gameTypes, bool _onlyBuddies, bool _onlyRegistered, bool _spectatorsAllowed, bool _spectatorsNeedPassword, bool _spectatorsCanTalk, bool _spectatorsSeeEverything, Server_Room *_room) + : QObject(), room(_room), creatorInfo(new ServerInfo_User(_creator->getUserInfo())), gameStarted(false), gameId(_gameId), description(_description), password(_password), maxPlayers(_maxPlayers), gameTypes(_gameTypes), activePlayer(-1), activePhase(-1), onlyBuddies(_onlyBuddies), onlyRegistered(_onlyRegistered), spectatorsAllowed(_spectatorsAllowed), spectatorsNeedPassword(_spectatorsNeedPassword), spectatorsCanTalk(_spectatorsCanTalk), spectatorsSeeEverything(_spectatorsSeeEverything), inactivityCounter(0), secondsElapsed(0), gameMutex(QMutex::Recursive) { addPlayer(_creator, false, false); - if (parent->getServer()->getGameShouldPing()) { + if (room->getServer()->getGameShouldPing()) { pingClock = new QTimer(this); connect(pingClock, SIGNAL(timeout()), this, SLOT(pingClockTimeout())); pingClock->start(1000); @@ -79,7 +79,7 @@ void Server_Game::pingClockTimeout() } sendGameEvent(new Event_Ping(secondsElapsed, pingList)); - const int maxTime = static_cast(parent())->getServer()->getMaxGameInactivityTime(); + const int maxTime = room->getServer()->getMaxGameInactivityTime(); if (allPlayersInactive) { if (((++inactivityCounter >= maxTime) && (maxTime > 0)) || (playerCount < maxPlayers)) deleteLater(); @@ -193,9 +193,9 @@ ResponseCode Server_Game::checkJoin(ServerInfo_User *user, const QString &_passw if (!(user->getUserLevel() & ServerInfo_User::IsRegistered) && onlyRegistered) return RespUserLevelTooLow; if (onlyBuddies) - if (!static_cast(parent())->getServer()->getBuddyList(creatorInfo->getName()).contains(user->getName())) + if (!room->getServer()->getBuddyList(creatorInfo->getName()).contains(user->getName())) return RespOnlyBuddies; - if (static_cast(parent())->getServer()->getIgnoreList(creatorInfo->getName()).contains(user->getName())) + if (room->getServer()->getIgnoreList(creatorInfo->getName()).contains(user->getName())) return RespInIgnoreList; if (spectator) { if (!spectatorsAllowed) @@ -218,7 +218,7 @@ Server_Player *Server_Game::addPlayer(Server_ProtocolHandler *handler, bool spec players.insert(playerId, newPlayer); if (broadcastUpdate) - qobject_cast(parent())->broadcastGameListUpdate(this); + room->broadcastGameListUpdate(this); return newPlayer; } @@ -242,7 +242,7 @@ void Server_Game::removePlayer(Server_Player *player) if (gameStarted && playerActive) nextTurn(); } - qobject_cast(parent())->broadcastGameListUpdate(this); + room->broadcastGameListUpdate(this); } void Server_Game::removeArrowsToPlayer(Server_Player *player) diff --git a/common/server_game.h b/common/server_game.h index 2ce9b89e..15e85968 100644 --- a/common/server_game.h +++ b/common/server_game.h @@ -34,6 +34,7 @@ class ServerInfo_User; class Server_Game : public QObject { Q_OBJECT private: + Server_Room *room; ServerInfo_User *creatorInfo; QMap players; bool gameStarted; diff --git a/common/server_protocolhandler.cpp b/common/server_protocolhandler.cpp index 167bab6f..ab81a2c2 100644 --- a/common/server_protocolhandler.cpp +++ b/common/server_protocolhandler.cpp @@ -17,11 +17,20 @@ Server_ProtocolHandler::Server_ProtocolHandler(Server *_server, QObject *parent) : QObject(parent), server(_server), authState(PasswordWrong), acceptsUserListChanges(false), acceptsRoomListChanges(false), userInfo(0), timeRunning(0), lastDataReceived(0) { connect(server, SIGNAL(pingClockTimeout()), this, SLOT(pingClockTimeout())); - connect(this, SIGNAL(sigGameCreated(Server_Game *)), this, SLOT(processSigGameCreated(Server_Game *)), Qt::QueuedConnection); } Server_ProtocolHandler::~Server_ProtocolHandler() { +} + +// This is essentially the destructor, but it needs to be called from the +// child's destructor with the server mutex locked. Otherwise, the mutex +// could get unlocked while the object is not finished being destroyed, +// leading to calls to pure virtual functions. +void Server_ProtocolHandler::prepareDestroy() +{ + server->removeClient(this); + QMapIterator roomIterator(rooms); while (roomIterator.hasNext()) roomIterator.next().value()->removeClient(this); @@ -423,24 +432,17 @@ ResponseCode Server_ProtocolHandler::cmdCreateGame(Command_CreateGame *cmd, Comm for (int i = 0; i < gameTypeList.size(); ++i) gameTypes.append(gameTypeList[i]->getData()); - room->createGame(cmd->getDescription(), cmd->getPassword(), cmd->getMaxPlayers(), gameTypes, cmd->getOnlyBuddies(), cmd->getOnlyRegistered(), cmd->getSpectatorsAllowed(), cmd->getSpectatorsNeedPassword(), cmd->getSpectatorsCanTalk(), cmd->getSpectatorsSeeEverything(), this); - return RespOk; -} - -void Server_ProtocolHandler::gameCreated(Server_Game *game) -{ - emit sigGameCreated(game); -} - -void Server_ProtocolHandler::processSigGameCreated(Server_Game *game) -{ - QMutexLocker locker(&game->gameMutex); + Server_Game *game = room->createGame(cmd->getDescription(), cmd->getPassword(), cmd->getMaxPlayers(), gameTypes, cmd->getOnlyBuddies(), cmd->getOnlyRegistered(), cmd->getSpectatorsAllowed(), cmd->getSpectatorsNeedPassword(), cmd->getSpectatorsCanTalk(), cmd->getSpectatorsSeeEverything(), this); Server_Player *creator = game->getPlayers().values().first(); games.insert(game->getGameId(), QPair(game, creator)); sendProtocolItem(new Event_GameJoined(game->getGameId(), game->getDescription(), creator->getPlayerId(), false, game->getSpectatorsCanTalk(), game->getSpectatorsSeeEverything(), false)); sendProtocolItem(GameEventContainer::makeNew(new Event_GameStateChanged(game->getGameStarted(), game->getActivePlayer(), game->getActivePhase(), game->getGameState(creator)), game->getGameId())); + + game->gameMutex.unlock(); + + return RespOk; } ResponseCode Server_ProtocolHandler::cmdJoinGame(Command_JoinGame *cmd, CommandContainer * /*cont*/, Server_Room *room) @@ -454,7 +456,9 @@ ResponseCode Server_ProtocolHandler::cmdJoinGame(Command_JoinGame *cmd, CommandC Server_Game *g = room->getGames().value(cmd->getGameId()); if (!g) return RespNameNotFound; - + + QMutexLocker locker(&g->gameMutex); + ResponseCode result = g->checkJoin(userInfo, cmd->getPassword(), cmd->getSpectator()); if (result == RespOk) { Server_Player *player = g->addPlayer(this, cmd->getSpectator()); diff --git a/common/server_protocolhandler.h b/common/server_protocolhandler.h index 3234bdb3..8e671ce5 100644 --- a/common/server_protocolhandler.h +++ b/common/server_protocolhandler.h @@ -28,6 +28,8 @@ protected: bool acceptsRoomListChanges; ServerInfo_User *userInfo; QMap buddyList, ignoreList; + + void prepareDestroy(); private: QList itemQueue; QList messageSizeOverTime, messageCountOverTime; @@ -91,11 +93,7 @@ private: ResponseCode processCommandHelper(Command *command, CommandContainer *cont); private slots: void pingClockTimeout(); - void processSigGameCreated(Server_Game *game); -signals: - void sigGameCreated(Server_Game *game); public: - mutable QMutex protocolHandlerMutex; Server_ProtocolHandler(Server *_server, QObject *parent = 0); ~Server_ProtocolHandler(); void playerRemovedFromGame(Server_Game *game); @@ -107,7 +105,6 @@ public: const QMap &getBuddyList() const { return buddyList; } const QMap &getIgnoreList() const { return ignoreList; } - void gameCreated(Server_Game *game); int getLastCommandTime() const { return timeRunning - lastDataReceived; } void processCommandContainer(CommandContainer *cont); virtual void sendProtocolItem(ProtocolItem *item, bool deleteItem = true) = 0; diff --git a/common/server_room.cpp b/common/server_room.cpp index f9c9b5a0..32953427 100644 --- a/common/server_room.cpp +++ b/common/server_room.cpp @@ -6,7 +6,6 @@ Server_Room::Server_Room(int _id, const QString &_name, const QString &_description, bool _autoJoin, const QString &_joinMessage, const QStringList &_gameTypes, Server *parent) : QObject(parent), id(_id), name(_name), description(_description), autoJoin(_autoJoin), joinMessage(_joinMessage), gameTypes(_gameTypes), roomMutex(QMutex::Recursive) { - connect(this, SIGNAL(sigCreateGame(const QString &, const QString &, int, const QList &, bool, bool, bool, bool, bool, bool, Server_ProtocolHandler *)), this, SLOT(doCreateGame(const QString &, const QString &, int, const QList &, bool, bool, bool, bool, bool, bool, Server_ProtocolHandler *))); } Server *Server_Room::getServer() const @@ -79,24 +78,24 @@ void Server_Room::broadcastGameListUpdate(Server_Game *game) delete event; } -void Server_Room::doCreateGame(const QString &description, const QString &password, int maxPlayers, const QList &gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator) +Server_Game *Server_Room::createGame(const QString &description, const QString &password, int maxPlayers, const QList &gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator) { QMutexLocker locker(&roomMutex); Server_Game *newGame = new Server_Game(creator, static_cast(parent())->getNextGameId(), description, password, maxPlayers, gameTypes, onlyBuddies, onlyRegistered, spectatorsAllowed, spectatorsNeedPassword, spectatorsCanTalk, spectatorsSeeEverything, this); + newGame->moveToThread(thread()); + newGame->setParent(this); + // This mutex needs to be unlocked by the caller. + newGame->gameMutex.lock(); games.insert(newGame->getGameId(), newGame); connect(newGame, SIGNAL(gameClosing()), this, SLOT(removeGame())); broadcastGameListUpdate(newGame); - creator->gameCreated(newGame); emit gameCreated(newGame); emit roomInfoChanged(); -} - -void Server_Room::createGame(const QString &description, const QString &password, int maxPlayers, const QList &gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator) -{ - emit sigCreateGame(description, password, maxPlayers, gameTypes, onlyBuddies, onlyRegistered, spectatorsAllowed, spectatorsNeedPassword, spectatorsCanTalk, spectatorsSeeEverything, creator); + + return newGame; } void Server_Room::removeGame() diff --git a/common/server_room.h b/common/server_room.h index 44523401..10f6c255 100644 --- a/common/server_room.h +++ b/common/server_room.h @@ -20,8 +20,6 @@ signals: void roomInfoChanged(); void gameCreated(Server_Game *game); void gameClosing(int gameId); - - void sigCreateGame(const QString &description, const QString &password, int maxPlayers, const QList &_gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator); private: int id; QString name; @@ -31,7 +29,6 @@ private: QStringList gameTypes; QMap games; private slots: - void doCreateGame(const QString &description, const QString &password, int maxPlayers, const QList &_gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator); void removeGame(); public: mutable QMutex roomMutex; @@ -49,7 +46,7 @@ public: void removeClient(Server_ProtocolHandler *client); void say(Server_ProtocolHandler *client, const QString &s); void broadcastGameListUpdate(Server_Game *game); - void createGame(const QString &description, const QString &password, int maxPlayers, const QList &_gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator); + Server_Game *createGame(const QString &description, const QString &password, int maxPlayers, const QList &_gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator); void sendRoomEvent(RoomEvent *event); }; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 5aedb7db..835e9a11 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -69,8 +69,7 @@ ServerSocketInterface::~ServerSocketInterface() delete socket; socket = 0; - // This call has to stay here so that the mutex is not freed prematurely. - server->removeClient(this); + prepareDestroy(); } void ServerSocketInterface::processProtocolItem(ProtocolItem *item) @@ -94,8 +93,6 @@ void ServerSocketInterface::flushXmlBuffer() void ServerSocketInterface::readClient() { - QMutexLocker locker(&protocolHandlerMutex); - QByteArray data = socket->readAll(); logger->logMessage(QString(data)); xmlReader->addData(data);