Skip to content

Commit

Permalink
FIX(plugins): Unlink active positional plugin before unloading
Browse files Browse the repository at this point in the history
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 mumble-voip#6549

(cherry picked from commit e9c53b1)
  • Loading branch information
sqwishy committed Jan 11, 2025
1 parent c9b1a3b commit 421eca9
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 7 deletions.
21 changes: 17 additions & 4 deletions src/mumble/PluginManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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();
}
}
Expand Down Expand Up @@ -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()));
}
Expand Down
6 changes: 3 additions & 3 deletions src/mumble/PluginManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 421eca9

Please sign in to comment.