FIX(client,ui): Crash by pressing Esc in nickname dialog

When having the dialog open that allows to set a custom nickname to
another client (UserLocalNicknameDialog) pressing Esc would crash
Mumble. This is due to the Esc key triggering the dialog's reject()
function in which the dialog's smart-pointer object is removed from a
global list and therefore goes out of scope.

This results in the dialog getting deleted, but since this is done in a
member function of the dialog, there is still code accessing the now
deleted object (in Qt's internals) and this is causing the segmentation
fault.

The fix is to use a custom deleter function for the dialog pointer that
instead of using delete on it, calls deleteLater() on it. Thus the
deletion of the object is left to Qt which can then do it at a point at
which it is done with it.

As this is something that could get useful in other locations as well,
the custom deleter function was outsourced into its own file.

Fixes #4666

src/CMakeLists.txt

@@ -61,6 +61,7 @@ set(SHARED_SOURCES
 	"OSInfo.cpp"
 	"PasswordGenerator.cpp"
 	"PlatformCheck.cpp"
+	"QtUtils.cpp"
 	"SelfSignedCertificate.cpp"
 	"ServerAddress.cpp"
 	"ServerResolver.cpp"

src/QtUtils.cpp

@@ -0,0 +1,10 @@
+// Copyright 2021 The Mumble Developers. All rights reserved.
+// Use of this source code is governed by a BSD-style license
+// that can be found in the LICENSE file at the root of the
+// Mumble source tree or at <https://www.mumble.info/LICENSE>.
+
+#include <QObject>
+
+void deleteQObject(QObject *object) {
+	object->deleteLater();
+}

src/QtUtils.h

@@ -0,0 +1,20 @@
+// Copyright 2021 The Mumble Developers. All rights reserved.
+// Use of this source code is governed by a BSD-style license
+// that can be found in the LICENSE file at the root of the
+// Mumble source tree or at <https://www.mumble.info/LICENSE>.
+
+#ifndef MUMBLE_QTUTILS_H_
+#define MUMBLE_QTUTILS_H_
+
+class QObject;
+
+/**
+ * A deleter function to be used for QObjects that must not be deleted using
+ * delete directly but rather by calling deleteLater() on them (and thus letting
+ * Qt perform the actual deletion).
+ *
+ * This function is intended to be used in smart-pointers holding QObjects.
+ */
+void deleteQObject(QObject *object);
+
+#endif // MUMBLE_QTUTILS_H_

src/mumble/MainWindow.h

@@ -70,7 +70,7 @@ public:
 		qiIconMuteSuppressed;
 	QIcon qiTalkingOn, qiTalkingWhisper, qiTalkingShout, qiTalkingOff;
 	QMap< unsigned int, UserLocalVolumeDialog * > qmUserVolTracker;
-	std::unordered_map< unsigned int, std::unique_ptr< UserLocalNicknameDialog > > qmUserNicknameTracker;
+	std::unordered_map< unsigned int, NicknameDialogPtr > qmUserNicknameTracker;
 
 	/// "Action" for when there are no actions available
 	QAction *qaEmpty;

src/mumble/UserLocalNicknameDialog.cpp

@@ -17,7 +17,7 @@
 
 UserLocalNicknameDialog::UserLocalNicknameDialog(
 	unsigned int sessionId,
-	std::unordered_map< unsigned int, std::unique_ptr< UserLocalNicknameDialog > > &qmUserNicknameTracker)
+	std::unordered_map< unsigned int, NicknameDialogPtr > &qmUserNicknameTracker)
 	: QDialog(nullptr), m_clientSession(sessionId), m_qmUserNicknameTracker(qmUserNicknameTracker) {
 	setupUi(this);
 
@@ -49,13 +49,14 @@ void UserLocalNicknameDialog::closeEvent(QCloseEvent *event) {
 
 void UserLocalNicknameDialog::present(
 	unsigned int sessionId,
-	std::unordered_map< unsigned int, std::unique_ptr< UserLocalNicknameDialog > > &qmUserNicknameTracker) {
+	std::unordered_map< unsigned int, NicknameDialogPtr > &qmUserNicknameTracker) {
 	if (qmUserNicknameTracker.find(sessionId) != qmUserNicknameTracker.end()) {
 		qmUserNicknameTracker.at(sessionId)->show();
 		qmUserNicknameTracker.at(sessionId)->raise();
 	} else {
-		std::unique_ptr< UserLocalNicknameDialog > userNickname =
-			std::make_unique< UserLocalNicknameDialog >(sessionId, qmUserNicknameTracker);
+		// Make sure to use the custom deleter for QObjects that calls deleteLater() on them instead of using
+		// delete directly as the latter can lead to segmentation faults.
+		NicknameDialogPtr userNickname(new UserLocalNicknameDialog(sessionId, qmUserNicknameTracker), deleteQObject);
 		userNickname->show();
 		qmUserNicknameTracker.insert(std::make_pair(sessionId, std::move(userNickname)));
 	}

src/mumble/UserLocalNicknameDialog.h

@@ -6,12 +6,22 @@
 #ifndef MUMBLE_MUMBLE_USERNICKNAME_H_
 #define MUMBLE_MUMBLE_USERNICKNAME_H_
 
+#include "QtUtils.h"
+
 #include <memory>
 #include <unordered_map>
 
 #include "ClientUser.h"
 #include "ui_UserLocalNicknameDialog.h"
 
+class UserLocalNicknameDialog;
+
+/**
+ * A typedef for a unique pointer (std::unique_ptr) using the deleter-function dedicated for QObjects
+ * instead of using raw delete
+ */
+typedef std::unique_ptr< UserLocalNicknameDialog, decltype(&deleteQObject) > NicknameDialogPtr;
+
 class UserLocalNicknameDialog : public QDialog, private Ui::UserLocalNicknameDialog {
 	Q_OBJECT
 	Q_DISABLE_COPY(UserLocalNicknameDialog);
@@ -21,7 +31,7 @@ class UserLocalNicknameDialog : public QDialog, private Ui::UserLocalNicknameDia
 
 	/// The user's original nickname when entering the dialog.
 	QString m_originalNickname;
-	std::unordered_map< unsigned int, std::unique_ptr< UserLocalNicknameDialog > > &m_qmUserNicknameTracker;
+	std::unordered_map< unsigned int, NicknameDialogPtr > &m_qmUserNicknameTracker;
 
 public slots:
 	void closeEvent(QCloseEvent *event);
@@ -32,10 +42,10 @@ public slots:
 public:
 	static void
 		present(unsigned int sessionId,
-				std::unordered_map< unsigned int, std::unique_ptr< UserLocalNicknameDialog > > &qmUserNicknameTracker);
+				std::unordered_map< unsigned int, NicknameDialogPtr > &qmUserNicknameTracker);
 	UserLocalNicknameDialog(
 		unsigned int sessionId,
-		std::unordered_map< unsigned int, std::unique_ptr< UserLocalNicknameDialog > > &qmUserNicknameTracker);
+		std::unordered_map< unsigned int, NicknameDialogPtr > &qmUserNicknameTracker);
 };
 
 #endif