Plugin manager: simplifies code, fixes a crash on exit

Author: moooorg

PluginManager.cpp

@@ -200,7 +200,6 @@ void PluginManager::AddPlugin(const filesystem::path& path)
                     entry.info          = info;
                     entry.plugin        = plugin;
                     entry.loader        = loader;
-                    entry.loaded        = false;
                     entry.path          = path_string;
                     entry.enabled       = enabled;
                     entry.widget        = nullptr;
@@ -209,7 +208,7 @@ void PluginManager::AddPlugin(const filesystem::path& path)
 
                     loader->unload();
 
-                    PluginManager::ActivePlugins.push_back(entry);
+                    ActivePlugins.push_back(entry);
 
                     if(entry.enabled)
                     {
@@ -236,7 +235,6 @@ void PluginManager::AddPlugin(const filesystem::path& path)
                     entry.info          = info;
                     entry.plugin        = plugin;
                     entry.loader        = loader;
-                    entry.loaded        = false;
                     entry.path          = path_string;
                     entry.enabled       = false;
                     entry.widget        = nullptr;
@@ -247,10 +245,24 @@ void PluginManager::AddPlugin(const filesystem::path& path)
 
                     PluginManager::ActivePlugins.push_back(entry);
 
-                    loader->unload();
+                    bool unloaded = loader->unload();
+
                     LOG_WARNING("[PluginManager] Plugin %s has an incompatible API version", path.c_str());
+
+                    if(!unloaded)
+                    {
+                        LOG_WARNING("[PluginManager] Plugin %s cannot be unloaded", path.c_str());
+                    }
                 }
             }
+            else
+            {
+                LOG_WARNING("[PluginManager] Plugin %s cannot be casted to OpenRGBPluginInterface", path.c_str());
+            }
+        }
+        else
+        {
+            LOG_WARNING("[PluginManager] Plugin %s cannot be instantiated.", path.c_str());
         }
     }
 }
@@ -284,7 +296,7 @@ void PluginManager::RemovePlugin(const filesystem::path& path)
     /*---------------------------------------------------------------------*\
     | If the selected plugin is in the list and loaded, unload it           |
     \*---------------------------------------------------------------------*/
-    if(ActivePlugins[plugin_idx].loaded)
+    if(ActivePlugins[plugin_idx].loader->isLoaded())
     {
         LOG_TRACE("[PluginManager] Plugin %s is active, unloading", path.c_str());
         UnloadPlugin(path);
@@ -330,10 +342,9 @@ void PluginManager::LoadPlugin(const filesystem::path& path)
     /*---------------------------------------------------------------------*\
     | If the selected plugin is in the list but not loaded, load it         |
     \*---------------------------------------------------------------------*/
-    if(!ActivePlugins[plugin_idx].loaded)
+    if(!ActivePlugins[plugin_idx].loader->isLoaded())
     {
         ActivePlugins[plugin_idx].loader->load();
-        ActivePlugins[plugin_idx].loaded = true;
 
         QObject* instance                = ActivePlugins[plugin_idx].loader->instance();
 
@@ -390,7 +401,7 @@ void PluginManager::UnloadPlugin(const filesystem::path& path)
     /*---------------------------------------------------------------------*\
     | If the selected plugin is in the list and loaded, unload it           |
     \*---------------------------------------------------------------------*/
-    if(ActivePlugins[plugin_idx].loaded)
+    if(ActivePlugins[plugin_idx].loader->isLoaded())
     {
         /*-------------------------------------------------*\
         | Call plugin's Unload function before GUI removal  |
@@ -405,15 +416,30 @@ void PluginManager::UnloadPlugin(const filesystem::path& path)
             RemovePluginCallbackVal(RemovePluginCallbackArg, &ActivePlugins[plugin_idx]);
         }
 
-        ActivePlugins[plugin_idx].loader->unload();
-        ActivePlugins[plugin_idx].loaded = false;
+        bool unloaded = ActivePlugins[plugin_idx].loader->unload();
+
+        if(!unloaded)
+        {
+            LOG_WARNING("[PluginManager] Plugin %s cannot be unloaded", path.c_str());
+        }
+        else
+        {
+            LOG_TRACE("[PluginManager] Plugin %s successfully unloaded", path.c_str());
+        }
+    }
+    else
+    {
+        LOG_TRACE("[PluginManager] Plugin %s was already unloaded", path.c_str());
     }
 }
 
 void PluginManager::UnloadPlugins()
 {
     for(const OpenRGBPluginEntry& plugin_entry: ActivePlugins)
     {
-        plugin_entry.plugin->Unload();
+        if(plugin_entry.loader->isLoaded())
+        {
+            plugin_entry.plugin->Unload();
+        }
     }
 }

PluginManager.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "OpenRGBPluginInterface.h"
-#include "ResourceManager.h"
 
 #include <QPluginLoader>
 #include <QLabel>
@@ -16,7 +15,6 @@ typedef struct
     OpenRGBPluginInfo           info;
     OpenRGBPluginInterface*     plugin;
     QPluginLoader*              loader;
-    bool                        loaded;
     QWidget*                    widget;
     QMenu*                      traymenu;
     std::string                 path;

qt/OpenRGBDialog2/OpenRGBDialog2.cpp

@@ -1001,78 +1001,61 @@ void OpenRGBDialog2::AddPlugin(OpenRGBPluginEntry* plugin)
 void OpenRGBDialog2::RemovePlugin(OpenRGBPluginEntry* plugin)
 {
     /*-----------------------------------------------------*\
-    | Place plugin as its own top level tab                 |
+    | Remove plugin's tray menu                             |
     \*-----------------------------------------------------*/
-    if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_TOP)
-    {
-        for(int tab_idx = 0; tab_idx < ui->MainTabBar->count(); tab_idx++)
-        {
-            if(dynamic_cast<OpenRGBPluginContainer*>(ui->MainTabBar->widget(tab_idx)) != nullptr)
-            {
-                if(dynamic_cast<OpenRGBPluginContainer*>(ui->MainTabBar->widget(tab_idx))->plugin_widget == plugin->widget)
-                {
-                    ui->MainTabBar->removeTab(tab_idx);
-                    delete plugin->widget;
-                }
-            }
-        }
-    }
-    /*-----------------------------------------------------*\
-    | Place plugin in the Devices tab                       |
-    \*-----------------------------------------------------*/
-    else if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_DEVICES)
+    if(plugin->traymenu)
     {
-        for(int tab_idx = 0; tab_idx < ui->DevicesTabBar->count(); tab_idx++)
+        QWidget* plugin_tray_entry = trayIconMenu->find(plugin->traymenu->winId());
+
+        if(plugin_tray_entry)
         {
-            if(dynamic_cast<OpenRGBPluginContainer*>(ui->DevicesTabBar->widget(tab_idx)) != nullptr)
-            {
-                if(dynamic_cast<OpenRGBPluginContainer*>(ui->DevicesTabBar->widget(tab_idx))->plugin_widget == plugin->widget)
-                {
-                    ui->DevicesTabBar->removeTab(tab_idx);
-                    delete plugin->widget;
-                }
-            }
+            trayIconMenu->removeAction(plugin->traymenu->menuAction());
         }
+
+        //delete plugin->traymenu;
     }
+
     /*-----------------------------------------------------*\
-    | Place plugin in the Information tab                   |
+    | Find plugin's container                               |
     \*-----------------------------------------------------*/
-    else if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_INFORMATION)
+    QTabWidget *plugin_parent_widget = nullptr;
+
+    switch(plugin->info.Location)
     {
-        for(int tab_idx = 0; tab_idx < ui->InformationTabBar->count(); tab_idx++)
-        {
-            if(dynamic_cast<OpenRGBPluginContainer*>(ui->InformationTabBar->widget(tab_idx)) != nullptr)
-            {
-                if(dynamic_cast<OpenRGBPluginContainer*>(ui->InformationTabBar->widget(tab_idx))->plugin_widget == plugin->widget)
-                {
-                    ui->InformationTabBar->removeTab(tab_idx);
-                    delete plugin->widget;
-                }
-            }
-        }
+        case OPENRGB_PLUGIN_LOCATION_TOP:
+            plugin_parent_widget = ui->MainTabBar;
+            break;
+        case OPENRGB_PLUGIN_LOCATION_DEVICES:
+            plugin_parent_widget = ui->DevicesTabBar;
+            break;
+        case OPENRGB_PLUGIN_LOCATION_INFORMATION:
+            plugin_parent_widget = ui->InformationTabBar;
+            break;
+        case OPENRGB_PLUGIN_LOCATION_SETTINGS:
+            plugin_parent_widget = ui->InformationTabBar;
+            break;
+        default:
+            break;
     }
+
     /*-----------------------------------------------------*\
-    | Place plugin in the Settings tab                      |
+    | Remove plugin from its container                      |
     \*-----------------------------------------------------*/
-    else if(plugin->info.Location == OPENRGB_PLUGIN_LOCATION_SETTINGS)
+    if(plugin_parent_widget != nullptr)
     {
-        for(int tab_idx = 0; tab_idx < ui->SettingsTabBar->count(); tab_idx++)
+        for(int tab_idx = 0; tab_idx < plugin_parent_widget->count(); tab_idx++)
         {
-            if(dynamic_cast<OpenRGBPluginContainer*>(ui->SettingsTabBar->widget(tab_idx)) != nullptr)
+            if(dynamic_cast<OpenRGBPluginContainer*>(plugin_parent_widget->widget(tab_idx)) != nullptr)
             {
-                if(dynamic_cast<OpenRGBPluginContainer*>(ui->SettingsTabBar->widget(tab_idx))->plugin_widget == plugin->widget)
+                if(dynamic_cast<OpenRGBPluginContainer*>(plugin_parent_widget->widget(tab_idx))->plugin_widget == plugin->widget)
                 {
-                    ui->SettingsTabBar->removeTab(tab_idx);
-                    delete plugin->widget;
+                    plugin_parent_widget->removeTab(tab_idx);
+                    //delete plugin->widget;
+                    break;
                 }
             }
         }
     }
-
-    if(plugin->traymenu)
-    {
-        trayIconMenu->removeAction(plugin->traymenu->menuAction());
-    }
 }
 
 void OpenRGBDialog2::AddI2CToolsPage()

qt/OpenRGBPluginsPage/OpenRGBPluginsPage.cpp

@@ -44,25 +44,25 @@ void Ui::OpenRGBPluginsPage::RefreshList()
     ui->PluginsList->clear();
     entries.clear();
 
-    for(unsigned int plugin_idx = 0; plugin_idx < plugin_manager->ActivePlugins.size(); plugin_idx++)
+    for(const OpenRGBPluginEntry& plugin: plugin_manager->ActivePlugins)
     {
         OpenRGBPluginsEntry* entry = new OpenRGBPluginsEntry();
 
         /*---------------------------------------------------------*\
         | Fill in plugin information fields                         |
         \*---------------------------------------------------------*/
-        entry->ui->NameValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Name));
-        entry->ui->DescriptionValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Description));
-        entry->ui->VersionValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Version));
-        entry->ui->CommitValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.Commit));
-        entry->ui->URLValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].info.URL));
-        entry->ui->APIVersionValue->setText(QString::number(plugin_manager->ActivePlugins[plugin_idx].api_version));
+        entry->ui->NameValue->setText(QString::fromStdString(plugin.info.Name));
+        entry->ui->DescriptionValue->setText(QString::fromStdString(plugin.info.Description));
+        entry->ui->VersionValue->setText(QString::fromStdString(plugin.info.Version));
+        entry->ui->CommitValue->setText(QString::fromStdString(plugin.info.Commit));
+        entry->ui->URLValue->setText(QString::fromStdString(plugin.info.URL));
+        entry->ui->APIVersionValue->setText(QString::number(plugin.api_version));
 
         /*---------------------------------------------------------*\
         | If the plugin is incompatible, highlight the API version  |
         | in red and disable the enable checkbox                    |
         \*---------------------------------------------------------*/
-        if(plugin_manager->ActivePlugins[plugin_idx].incompatible)
+        if(plugin.incompatible)
         {
             entry->ui->APIVersionValue->setStyleSheet("QLabel { color : red; }");
             entry->ui->EnabledCheckBox->setEnabled(false);
@@ -71,20 +71,20 @@ void Ui::OpenRGBPluginsPage::RefreshList()
         /*---------------------------------------------------------*\
         | Fill in plugin icon                                       |
         \*---------------------------------------------------------*/
-        QPixmap pixmap(QPixmap::fromImage(plugin_manager->ActivePlugins[plugin_idx].info.Icon));
+        QPixmap pixmap(QPixmap::fromImage(plugin.info.Icon));
 
         entry->ui->IconView->setPixmap(pixmap);
         entry->ui->IconView->setScaledContents(true);
 
         /*---------------------------------------------------------*\
         | Fill in plugin path                                       |
         \*---------------------------------------------------------*/
-        entry->ui->PathValue->setText(QString::fromStdString(plugin_manager->ActivePlugins[plugin_idx].path));
+        entry->ui->PathValue->setText(QString::fromStdString(plugin.path));
 
         /*---------------------------------------------------------*\
         | Fill in plugin enabled status                             |
         \*---------------------------------------------------------*/
-        entry->ui->EnabledCheckBox->setChecked((plugin_manager->ActivePlugins[plugin_idx].enabled));
+        entry->ui->EnabledCheckBox->setChecked((plugin.enabled));
 
         entry->RegisterEnableClickCallback(EnableClickCallbackFunction, this);
 
@@ -261,7 +261,7 @@ void Ui::OpenRGBPluginsPage::on_EnableButton_clicked(OpenRGBPluginsEntry* entry)
     \*-----------------------------------------------------*/
     std::string     name        = "";
     std::string     description = "";
-    bool            enabled     = true;
+    bool            enabled     = entry->ui->EnabledCheckBox->isChecked();
     bool            found       = false;
     unsigned int    plugin_ct   = 0;
     unsigned int    plugin_idx  = 0;
@@ -270,15 +270,6 @@ void Ui::OpenRGBPluginsPage::on_EnableButton_clicked(OpenRGBPluginsEntry* entry)
     std::string     entry_desc  = entry->ui->DescriptionValue->text().toStdString();
     std::string     entry_path  = entry->ui->PathValue->text().toStdString();
 
-    if(entry->ui->EnabledCheckBox->isChecked())
-    {
-        enabled                 = true;
-    }
-    else
-    {
-        enabled                 = false;
-    }
-
     if(plugin_settings.contains("plugins"))
     {
         plugin_ct = plugin_settings["plugins"].size();