reduced game selector minimum height; server race conditions fixed

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)

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()));

common/server_game.cpp

@@ -28,12 +28,12 @@
 #include <QTimer>
 #include <QDebug>
 
-Server_Game::Server_Game(Server_ProtocolHandler *_creator, int _gameId, const QString &_description, const QString &_password, int _maxPlayers, const QList<int> &_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<int> &_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<Server_Room *>(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<Server_Room *>(parent())->getServer()->getBuddyList(creatorInfo->getName()).contains(user->getName()))
+		if (!room->getServer()->getBuddyList(creatorInfo->getName()).contains(user->getName()))
 			return RespOnlyBuddies;
-	if (static_cast<Server_Room *>(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<Server_Room *>(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<Server_Room *>(parent())->broadcastGameListUpdate(this);
+	room->broadcastGameListUpdate(this);
 }
 
 void Server_Game::removeArrowsToPlayer(Server_Player *player)

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<int, Server_Player *> players;
 	bool gameStarted;

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<int, Server_Room *> 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<Server_Game *, Server_Player *>(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());

common/server_protocolhandler.h

@@ -28,6 +28,8 @@ protected:
 	bool acceptsRoomListChanges;
 	ServerInfo_User *userInfo;
 	QMap<QString, ServerInfo_User *> buddyList, ignoreList;
+	
+	void prepareDestroy();
 private:
 	QList<ProtocolItem *> itemQueue;
 	QList<int> 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<QString, ServerInfo_User *> &getBuddyList() const { return buddyList; }
 	const QMap<QString, ServerInfo_User *> &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;

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<int> &, bool, bool, bool, bool, bool, bool, Server_ProtocolHandler *)), this, SLOT(doCreateGame(const QString &, const QString &, int, const QList<int> &, 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<int> &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<int> &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<Server *>(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<int> &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()

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<int> &_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<int, Server_Game *> games;
 private slots:
-	void doCreateGame(const QString &description, const QString &password, int maxPlayers, const QList<int> &_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<int> &_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<int> &_gameTypes, bool onlyBuddies, bool onlyRegistered, bool spectatorsAllowed, bool spectatorsNeedPassword, bool spectatorsCanTalk, bool spectatorsSeeEverything, Server_ProtocolHandler *creator);
 	
 	void sendRoomEvent(RoomEvent *event);
 };

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);