Module interfaces managed by smart pointers

src/ModuleManager.cpp

@@ -213,15 +213,7 @@ ModuleManager::~ModuleManager()
    }
    mModules.Clear();
 
-   ModuleMap::iterator iter = mDynModules.begin();
+   mDynModules.clear();
-   while (iter != mDynModules.end())
-   {
-      UnloadModule(iter->second);
-
-      mDynModules.erase(iter->first);
-
-      iter = mDynModules.begin();
-   }
 
    if (pBuiltinModuleList != NULL)
    {
@@ -432,18 +424,25 @@ void ModuleManager::InitializeBuiltins()
 
    for (size_t i = 0, cnt = pBuiltinModuleList->GetCount(); i < cnt; i++)
    {
-      ModuleInterface *module = ((ModuleMain) (*pBuiltinModuleList)[i])(this, NULL);
+      ModuleInterfaceHandle module {
+         ((ModuleMain)(*pBuiltinModuleList)[i])(this, NULL), ModuleInterfaceDeleter{}
+      };
 
       if (module->Initialize())
       {
          // Register the provider
-         const PluginID & id = pm.RegisterPlugin(module);
+         ModuleInterface *pInterface = module.get();
+         const PluginID & id = pm.RegisterPlugin(pInterface);
 
          // Need to remember it 
-         mDynModules[id] = module;
+         mDynModules[id] = std::move(module);
 
          // Allow the module to auto-register children
-         module->AutoRegisterPlugins(pm);
+         pInterface->AutoRegisterPlugins(pm);
+      }
+      else
+      {
+         // Don't leak!  Destructor of module does that.
       }
    }
 }
@@ -459,19 +458,20 @@ ModuleInterface *ModuleManager::LoadModule(const wxString & path)
                                                             &success);
       if (success && audacityMain)
       {
-         ModuleInterface *module = audacityMain(this, &path);
+         ModuleInterfaceHandle handle {
-         if (module)
+            audacityMain(this, &path), ModuleInterfaceDeleter{}
+         };
+         if (handle)
          {
-            if (module->Initialize())
+            if (handle->Initialize())
             {
 
-               mDynModules[PluginManager::GetID(module)] = module;
+               auto module = handle.get();
+               mDynModules[PluginManager::GetID(module)] = std::move(handle);
                mLibs[module] = lib;
 
                return module;
             }
-            module->Terminate();
-            delete module;
          }
       }
 
@@ -483,35 +483,42 @@ ModuleInterface *ModuleManager::LoadModule(const wxString & path)
    return NULL;
 }
 
-void ModuleManager::UnloadModule(ModuleInterface *module)
+void ModuleInterfaceDeleter::operator() (ModuleInterface *pInterface) const
 {
-   if (module)
+   if (pInterface)
    {
-      module->Terminate();
+      pInterface->Terminate();
 
-      if (mLibs.find(module) != mLibs.end())
+      auto &libs = ModuleManager::Get().mLibs;
+      if (libs.find(pInterface) != libs.end())
       {
-         mLibs[module]->Unload();
+         libs[pInterface]->Unload();
-         mLibs.erase(module);
+         libs.erase(pInterface);
       }
 
-      delete module; //After terminating and unloading, we can safely DELETE the module
+      delete pInterface;
    }
 }
 
-void ModuleManager::RegisterModule(ModuleInterface *module)
+void ModuleManager::RegisterModule(ModuleInterface *inModule)
 {
-   PluginID id = PluginManager::GetID(module);
+   std::unique_ptr<ModuleInterface> module{ inModule };
+
+   PluginID id = PluginManager::GetID(module.get());
 
    if (mDynModules.find(id) != mDynModules.end())
    {
       // TODO:  Should we complain about a duplicate registeration????
+      // PRL:  Don't leak resources!
+      module->Terminate();
       return;
    }
 
-   mDynModules[id] = module;
+   mDynModules[id] = ModuleInterfaceHandle {
+      module.release(), ModuleInterfaceDeleter{}
+   };
 
-   PluginManager::Get().RegisterPlugin(module);
+   PluginManager::Get().RegisterPlugin(inModule);
 }
 
 void ModuleManager::FindAllPlugins(PluginIDList & providers, wxArrayString & paths)
@@ -575,7 +582,7 @@ IdentInterface *ModuleManager::CreateProviderInstance(const PluginID & providerI
 {
    if (path.IsEmpty() && mDynModules.find(providerID) != mDynModules.end())
    {
-      return mDynModules[providerID];
+      return mDynModules[providerID].get();
    }
 
    return LoadModule(path);

src/ModuleManager.h

@@ -14,6 +14,7 @@
 
 #include <wx/dynlib.h>
 
+#include "MemoryX.h"
 #include <map>
 #include <vector>
 
@@ -61,16 +62,21 @@ private:
    fnModuleDispatch mDispatch;
 };
 
+struct ModuleInterfaceDeleter {
+   void operator ()(ModuleInterface *pInterface) const;
+};
+
+using ModuleInterfaceHandle = movable_ptr_with_deleter<
+   ModuleInterface, ModuleInterfaceDeleter
+>;
+
 typedef std::map<wxString, ModuleMain *> ModuleMainMap;
-typedef std::map<wxString, ModuleInterface *> ModuleMap;
+typedef std::map<wxString, ModuleInterfaceHandle> ModuleMap;
 typedef std::map<ModuleInterface *, wxDynamicLibrary *> LibraryMap;
 
 class ModuleManager final : public ModuleManagerInterface
 {
 public:
-   ModuleManager();
-   virtual ~ModuleManager();
-
    // -------------------------------------------------------------------------
    // ModuleManagerInterface implementation
    // -------------------------------------------------------------------------
@@ -102,11 +108,15 @@ public:
    bool IsPluginValid(const PluginID & provider, const wxString & path);
 
 private:
+   // I'm a singleton class
+   ModuleManager();
+   virtual ~ModuleManager();
+
    void InitializeBuiltins();
    ModuleInterface *LoadModule(const wxString & path);
-   void UnloadModule(ModuleInterface *module);
 
 private:
+   friend ModuleInterfaceDeleter;
    static ModuleManager *mInstance;
 
    ModuleMainMap mModuleMains;

src/effects/LoadEffects.cpp

@@ -198,7 +198,8 @@ static const wxChar *kExcludedNames[] =
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create and register the importer
-   return new BuiltinEffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew BuiltinEffectsModule(moduleManager, path);
 }
 
 // ============================================================================

src/effects/VST/VSTEffect.cpp

@@ -112,7 +112,8 @@
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create our effects module and register
-   return new VSTEffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew VSTEffectsModule(moduleManager, path);
 }
 
 // ============================================================================

src/effects/audiounits/AudioUnitEffect.cpp

@@ -49,7 +49,8 @@
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create and register the importer
-   return new AudioUnitEffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew AudioUnitEffectsModule(moduleManager, path);
 }
 
 // ============================================================================

src/effects/ladspa/LadspaEffect.cpp

@@ -73,7 +73,8 @@ const static wxChar *kShippedEffects[] =
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create and register the importer
-   return new LadspaEffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew LadspaEffectsModule(moduleManager, path);
 }
 
 // ============================================================================

src/effects/lv2/LoadLV2.cpp

@@ -56,7 +56,8 @@ Functions that find and load all LV2 plugins on the system.
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create and register the importer
-   return new LV2EffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew LV2EffectsModule(moduleManager, path);
 }
 
 // ============================================================================

src/effects/nyquist/LoadNyquist.cpp

@@ -60,7 +60,8 @@ const static wxChar *kShippedEffects[] =
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create and register the importer
-   return new NyquistEffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew NyquistEffectsModule(moduleManager, path);
 }
 
 // ============================================================================

src/effects/vamp/LoadVamp.cpp

@@ -37,7 +37,8 @@ using namespace Vamp::HostExt;
 DECLARE_MODULE_ENTRY(AudacityModule)
 {
    // Create and register the importer
-   return new VampEffectsModule(moduleManager, path);
+   // Trust the module manager not to leak this
+   return safenew VampEffectsModule(moduleManager, path);
 }
 
 // ============================================================================