From e9c53b1ea2af705d68152f9cd3a04c036f03d5bf Mon Sep 17 00:00:00 2001
From: sqwishy <somebody@froghat.ca>
Date: Sun, 6 Oct 2024 14:23:16 -0700
Subject: [PATCH] FIX(plugins): Unlink active positional plugin before
 unloading

If a plugin is providing positional audio data and it becomes unloaded,
such as by disabling the plugin in the settings GUI, the plugin manager
may still interact with the plugin through m_activePositionalDataPlugin.

This looks unintentional and, in debug mode, this fails an assertion in
the plugin fairly quickly: `assertPluginLoaded(this)`.

Additionally, in case Mumble shuts down while there exists an active
positional data plugin, Mumble will (now) try to log that the plugin has
lost link by calling the respective function on the global Log instance.
However, at the point where the plugin manager object is destroyed, the
log has already been deleted, making this produce a segmentation fault.
To prevent this from happening, an explicit check for whether the Log
instance still exists has been added to the logging logic.

Fixes #6549
---
 src/mumble/PluginManager.cpp | 21 +++++++++++++++++----
 src/mumble/PluginManager.h   |  6 +++---
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/mumble/PluginManager.cpp b/src/mumble/PluginManager.cpp
index 8ad2f17912b..d84286eec8d 100644
--- a/src/mumble/PluginManager.cpp
+++ b/src/mumble/PluginManager.cpp
@@ -176,7 +176,7 @@ bool PluginManager::eventFilter(QObject *target, QEvent *event) {
 	return QObject::eventFilter(target, event);
 }
 
-void PluginManager::unloadPlugins() const {
+void PluginManager::unloadPlugins() {
 	QReadLocker lock(&m_pluginCollectionLock);
 
 	auto it = m_pluginHashMap.begin();
@@ -538,7 +538,7 @@ bool PluginManager::loadPlugin(plugin_id_t pluginID) const {
 	return false;
 }
 
-void PluginManager::unloadPlugin(plugin_id_t pluginID) const {
+void PluginManager::unloadPlugin(plugin_id_t pluginID) {
 	plugin_ptr_t plugin;
 	{
 		QReadLocker lock(&m_pluginCollectionLock);
@@ -551,9 +551,20 @@ void PluginManager::unloadPlugin(plugin_id_t pluginID) const {
 	}
 }
 
-void PluginManager::unloadPlugin(Plugin &plugin) const {
+void PluginManager::unloadPlugin(Plugin &plugin) {
 	if (plugin.isLoaded()) {
 		// Only shut down loaded plugins
+
+		bool isActivePosDataPlugin = false;
+		{
+			QWriteLocker lock(&m_activePosDataPluginLock);
+			isActivePosDataPlugin = &plugin == m_activePositionalDataPlugin.get();
+		}
+
+		if (isActivePosDataPlugin) {
+			unlinkPositionalData();
+		}
+
 		plugin.shutdown();
 	}
 }
@@ -996,7 +1007,9 @@ void PluginManager::reportLostLink(mumble_plugin_id_t pluginID) {
 
 	const_plugin_ptr_t plugin = getPlugin(pluginID);
 
-	if (plugin) {
+	// Need to check for the presence of Global::get().l in case we are currently
+	// shutting down Mumble in which case the Log might already have been deleted.
+	if (plugin && Global::get().l) {
 		Global::get().l->log(Log::Information,
 							 PluginManager::tr("%1 lost link").arg(plugin->getName().toHtmlEscaped()));
 	}
diff --git a/src/mumble/PluginManager.h b/src/mumble/PluginManager.h
index b92fd45612f..c0cf191f33b 100644
--- a/src/mumble/PluginManager.h
+++ b/src/mumble/PluginManager.h
@@ -85,7 +85,7 @@ class PluginManager : public QObject {
 	bool eventFilter(QObject *target, QEvent *event) Q_DECL_OVERRIDE;
 
 	/// Unloads all plugins that are currently loaded.
-	void unloadPlugins() const;
+	void unloadPlugins();
 	/// Clears the current list of plugins
 	void clearPlugins();
 	/// Iterates over the plugins and tries to select a plugin that currently claims to be able to deliver positional
@@ -145,11 +145,11 @@ class PluginManager : public QObject {
 	/// Unloads the plugin with the given ID. Unloading means shutting the plugign down.
 	///
 	/// @param pluginID The ID of the plugin to unload
-	void unloadPlugin(plugin_id_t pluginID) const;
+	void unloadPlugin(plugin_id_t pluginID);
 	/// Unloads the given plugin. Unloading means shutting the plugign down.
 	///
 	/// @param plugin The plugin to unload
-	void unloadPlugin(Plugin &plugin) const;
+	void unloadPlugin(Plugin &plugin);
 	/// Clears the plugin from the list of known plugins
 	///
 	/// @param pluginID The ID of the plugin to forget about