FIX(client): Racecondition deleting shortcuts

Server-specific shortcuts are read into memory in
MainWindow::msgServerSync and are saved into the DB again in
MainWindow::serverDisconnected.
If however the disconnect function runs before the server
synchronization has finished, the shortcut list in memory is empty and
by writing that into the DB all existing server-specific shortcuts are
lost (overwritten).

This commit adds checks before saving the shortcuts (and
ChannelListeners) so that saving only occurs if the synchronization has
happened before and thus the shortcuts have been loaded in the first
place.

src/mumble/MainWindow.cpp

@@ -3093,9 +3093,12 @@ void MainWindow::serverConnected() {
 }
 
 void MainWindow::serverDisconnected(QAbstractSocket::SocketError err, QString reason) {
-	// Note that the saving of the ChannelListeners has to be done, before resetting g.uiSession
-	// Save ChannelListeners
-	ChannelListener::saveToDB();
+	if (g.sh->hasSynchronized()) {
+		// Note that the saving of the ChannelListeners has to be done, before resetting g.uiSession
+		// Save ChannelListeners
+		ChannelListener::saveToDB();
+	}
+
 	// clear ChannelListener
 	ChannelListener::clear();
 
@@ -3117,8 +3120,15 @@ void MainWindow::serverDisconnected(QAbstractSocket::SocketError err, QString re
 	QString uname, pw, host;
 	unsigned short port;
 	g.sh->getConnectionInfo(host, port, uname, pw);
-	if (g.db->setShortcuts(g.sh->qbaDigest, g.s.qlShortcuts))
-		GlobalShortcutEngine::engine->bNeedRemap = true;
+
+	if (g.sh->hasSynchronized()) {
+		// Only save server-specific shortcuts if the client and server have been synchronized before as only then
+		// did the client actually load them from the DB. If we store them without having loaded them, we will effectively
+		// clear the server-specific shortcuts for this server.
+		if (g.db->setShortcuts(g.sh->qbaDigest, g.s.qlShortcuts)) {
+			GlobalShortcutEngine::engine->bNeedRemap = true;
+		}
+	}
 
 	if (aclEdit) {
 		aclEdit->reject();

src/mumble/Messages.cpp

@@ -200,6 +200,8 @@ void MainWindow::msgServerSync(const MumbleProto::ServerSync &msg) {
 		}
 	});
 
+	g.sh->setServerSynchronized(true);
+
 	emit serverSynchronized();
 }
 

src/mumble/ServerHandler.cpp

@@ -309,6 +309,20 @@ void ServerHandler::sendProtoMessage(const ::google::protobuf::Message &msg, uns
 	}
 }
 
+bool ServerHandler::isConnected() const {
+	// If the digest isn't empty, then we are currently connected to a server (the digest being a hash
+	// of the server's certificate)
+	return !qbaDigest.isEmpty();
+}
+
+bool ServerHandler::hasSynchronized() const {
+	return serverSynchronized;
+}
+
+void ServerHandler::setServerSynchronized(bool synchronized) {
+	serverSynchronized = synchronized;
+}
+
 void ServerHandler::hostnameResolved() {
 	ServerResolver *sr = qobject_cast<ServerResolver *>(QObject::sender());
 	QList<ServerResolverRecord> records = sr->records();
@@ -374,6 +388,11 @@ void ServerHandler::run() {
 			ConnectionPtr connection(new Connection(this, qtsSock));
 			cConnection = connection;
 
+			// Technically it isn't necessary to reset this flag here since a ServerHandler will not be used
+			// for multiple connections in a row but just in case that at some point it will, we'll reset the
+			// flag here.
+			serverSynchronized = false;
+
 			qlErrors.clear();
 			qscCert.clear();
 
@@ -657,6 +676,7 @@ void ServerHandler::serverConnectionClosed(QAbstractSocket::SocketError err, con
 	}
 
 	emit disconnected(err, reason);
+
 	exit(0);
 }
 

src/mumble/ServerHandler.h

@@ -70,6 +70,10 @@ class ServerHandler : public QThread {
 		bool bUdp;
 		bool bStrong;
 
+		/// Flag indicating whether the server we are currently connected to has
+		/// finished synchronizing already.
+		bool serverSynchronized = false;
+
 #ifdef Q_OS_WIN
 		HANDLE hQoS;
 		DWORD dwFlowUDP;
@@ -112,6 +116,16 @@ class ServerHandler : public QThread {
 		void sendProtoMessage(const ::google::protobuf::Message &msg, unsigned int msgType);
 		void sendMessage(const char *data, int len, bool force = false);
 
+		/// @returns Whether this handler is currently connected to a server.
+		bool isConnected() const;
+
+		/// @returns Whether the server this handler is currently connected to, has finished
+		/// 	synchronizing yet.
+		bool hasSynchronized() const;
+
+		/// @param synchronized Whether the server has finished synchronization
+		void setServerSynchronized(bool synchronized);
+
 #define MUMBLE_MH_MSG(x) void sendMessage(const MumbleProto:: x &msg) { sendProtoMessage(msg, MessageHandler:: x); }
 		MUMBLE_MH_ALL
 #undef MUMBLE_MH_MSG