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
This commit is contained in:
sqwishy 2024-10-06 14:23:16 -07:00 committed by Robert Adam
parent 2aeb6cbbd3
commit e9c53b1ea2
2 changed files with 20 additions and 7 deletions

View File

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

View File

@ -85,7 +85,7 @@ protected:
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 @@ public:
/// 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