Simplify memory management in PluginManager

include/audacity/ModuleInterface.h

@@ -43,6 +43,7 @@
 #define __AUDACITY_MODULEINTERFACE_H__
 
 #include <functional>
+#include <memory>
 #include "Identifier.h"
 #include "audacity/ComponentInterface.h"
 #include "audacity/PluginInterface.h"
@@ -129,10 +130,8 @@ public:
    virtual bool IsPluginValid(const PluginPath & path, bool bFast) = 0;
 
    // When appropriate, CreateInstance() will be called to instantiate the plugin.
-   virtual ComponentInterface *CreateInstance(const PluginPath & path) = 0;
-
-   // When appropriate, DeleteInstance() will be called to delete the plugin.
-   virtual void DeleteInstance(ComponentInterface *instance) = 0;
+   virtual std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) = 0;
 };
 
 // ----------------------------------------------------------------------------

modules/mod-nyq-bench/NyqBench.cpp

@@ -1327,9 +1327,11 @@ void NyqBench::OnLargeIcons(wxCommandEvent & e)
 
 void NyqBench::OnGo(wxCommandEvent & e)
 {
-   // No need to delete...EffectManager will do it
-   mEffect = new NyquistEffect(wxT("Nyquist Effect Workbench"));
-   const PluginID & ID = EffectManager::Get().RegisterEffect(mEffect);
+   auto pEffect =
+      std::make_unique<NyquistEffect>(L"Nyquist Effect Workbench");
+   mEffect = pEffect.get();
+   const PluginID & ID =
+      EffectManager::Get().RegisterEffect(std::move(pEffect));
 
    mEffect->SetCommand(mScript->GetValue());
    mEffect->RedirectOutput();

src/ModuleManager.cpp

@@ -530,26 +530,14 @@ ModuleInterface *ModuleManager::CreateProviderInstance(const PluginID & provider
    return nullptr;
 }
 
-ComponentInterface *ModuleManager::CreateInstance(const PluginID & providerID,
-                                              const PluginPath & path)
+std::unique_ptr<ComponentInterface> ModuleManager::CreateInstance(
+   const PluginID & providerID, const PluginPath & path)
 {
-   if (mDynModules.find(providerID) == mDynModules.end())
-   {
-      return NULL;
-   }
-
-   return mDynModules[providerID]->CreateInstance(path);
-}
-
-void ModuleManager::DeleteInstance(const PluginID & providerID,
-                                   ComponentInterface *instance)
-{
-   if (mDynModules.find(providerID) == mDynModules.end())
-   {
-      return;
-   }
-
-   mDynModules[providerID]->DeleteInstance(instance);
+   if (auto iter = mDynModules.find(providerID);
+       iter == mDynModules.end())
+      return nullptr;
+   else
+      return iter->second->CreateInstance(path);
 }
 
 bool ModuleManager::IsProviderValid(const PluginID & WXUNUSED(providerID),

src/ModuleManager.h

@@ -96,9 +96,10 @@ public:
    bool RegisterEffectPlugin(const PluginID & provider, const PluginPath & path,
                        TranslatableString &errMsg);
 
-   ModuleInterface *CreateProviderInstance(const PluginID & provider, const PluginPath & path);
-   ComponentInterface *CreateInstance(const PluginID & provider, const PluginPath & path);
-   void DeleteInstance(const PluginID & provider, ComponentInterface *instance);
+   ModuleInterface *CreateProviderInstance(
+      const PluginID & provider, const PluginPath & path);
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginID & provider, const PluginPath & path);
 
    bool IsProviderValid(const PluginID & provider, const PluginPath & path);
    bool IsPluginValid(const PluginID & provider, const PluginPath & path, bool bFast);

src/PluginManager.cpp

@@ -1088,7 +1088,7 @@ PluginDescriptor::PluginDescriptor()
    mPluginType = PluginTypeNone;
    mEnabled = false;
    mValid = false;
-   mInstance = NULL;
+   mInstance = nullptr;
 
    mEffectType = EffectTypeNone;
    mEffectInteractive = false;
@@ -1100,21 +1100,13 @@ PluginDescriptor::PluginDescriptor()
 
 PluginDescriptor::~PluginDescriptor()
 {
-   DeleteInstance();
 }
 
-void PluginDescriptor::DeleteInstance()
-{
-   if (mInstance)
-   {
-      ModuleManager::Get().DeleteInstance(GetProviderID(), mInstance);
-      mInstance = nullptr;
-   }
-}
+PluginDescriptor &PluginDescriptor::operator =(PluginDescriptor &&) = default;
 
 bool PluginDescriptor::IsInstantiated() const
 {
-   return mInstance != NULL;
+   return mInstance != nullptr;
 }
 
 ComponentInterface *PluginDescriptor::GetInstance()
@@ -1122,29 +1114,21 @@ ComponentInterface *PluginDescriptor::GetInstance()
    if (!mInstance)
    {
       if (GetPluginType() == PluginTypeModule)
-      {
          mInstance = ModuleManager::Get().CreateProviderInstance(GetID(), GetPath());
-      }
       else
       {
-         mInstance = ModuleManager::Get().CreateInstance(GetProviderID(), GetPath());
+         muInstance = ModuleManager::Get().CreateInstance(GetProviderID(), GetPath());
+         mInstance = muInstance.get();
       }
    }
 
    return mInstance;
 }
 
-void PluginDescriptor::SetInstance(ComponentInterface *instance)
+void PluginDescriptor::SetInstance(std::unique_ptr<ComponentInterface> instance)
 {
-   if (mInstance && mInstance != instance)
-   {
-      // Be sure not to leak resources!!
-      DeleteInstance();
-   }
-
-   mInstance = instance;
-
-   return;
+   muInstance = std::move(instance);
+   mInstance = muInstance.get();
 }
 
 PluginType PluginDescriptor::GetPluginType() const
@@ -2273,7 +2257,7 @@ void PluginManager::LoadGroup(FileConfig *pRegistry, PluginType type)
       }
 
       // Everything checked out...accept the plugin
-      mPlugins[groupName] = plug;
+      mPlugins[groupName] = std::move(plug);
    }
 
    return;
@@ -2497,9 +2481,11 @@ bool PluginManager::ShowManager(wxWindow *parent, EffectType type)
 
 // Here solely for the purpose of Nyquist Workbench until
 // a better solution is devised.
-const PluginID & PluginManager::RegisterPlugin(EffectDefinitionInterface *effect, PluginType type)
+const PluginID & PluginManager::RegisterPlugin(
+   std::unique_ptr<EffectDefinitionInterface> effect, PluginType type)
 {
-   PluginDescriptor & plug = CreatePlugin(GetID(effect), effect, type);
+   PluginDescriptor & plug =
+      CreatePlugin(GetID(effect.get()), effect.get(), type);
 
    plug.SetEffectType(effect->GetType());
    plug.SetEffectFamily(effect->GetFamily().Internal());
@@ -2508,7 +2494,7 @@ const PluginID & PluginManager::RegisterPlugin(EffectDefinitionInterface *effect
    plug.SetEffectRealtime(effect->SupportsRealtime());
    plug.SetEffectAutomatable(effect->SupportsAutomation());
 
-   plug.SetInstance(effect);
+   plug.SetInstance(std::move(effect));
    plug.SetEffectLegacy(true);
    plug.SetEnabled(true);
    plug.SetValid(true);

src/PluginManager.h

@@ -46,11 +46,12 @@ class AUDACITY_DLL_API PluginDescriptor
 {
 public:
    PluginDescriptor();
+   PluginDescriptor &operator =(PluginDescriptor &&);
    virtual ~PluginDescriptor();
 
    bool IsInstantiated() const;
    ComponentInterface *GetInstance();
-   void SetInstance(ComponentInterface *instance);
+   void SetInstance(std::unique_ptr<ComponentInterface> instance);
 
    PluginType GetPluginType() const;
    void SetPluginType(PluginType type);
@@ -121,13 +122,12 @@ public:
 
 private:
 
-   void DeleteInstance();
-
    // Common
 
    // Among other purposes, PluginDescriptor acts as the resource handle,
    // or smart pointer, to a resource created in a plugin library, and is responsible
    // for a cleanup of this pointer.
+   std::unique_ptr<ComponentInterface> muInstance; // may be null for a module
    ComponentInterface *mInstance;
 
    PluginType mPluginType;
@@ -264,7 +264,10 @@ public:
 
    bool ShowManager(wxWindow *parent, EffectType type = EffectTypeNone);
 
-   const PluginID & RegisterPlugin(EffectDefinitionInterface *effect, PluginType type );
+   //! Used only by Nyquist Workbench module
+   const PluginID & RegisterPlugin(
+      std::unique_ptr<EffectDefinitionInterface> effect, PluginType type );
+   //! Used only by Nyquist Workbench module
    void UnregisterPlugin(const PluginID & ID);
 
 private:

src/commands/LoadCommands.cpp

@@ -194,19 +194,11 @@ bool BuiltinCommandsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return mCommands.find( path ) != mCommands.end();
 }
 
-ComponentInterface *BuiltinCommandsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+BuiltinCommandsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
-   // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-   return Instantiate(path).release();
-}
-
-void BuiltinCommandsModule::DeleteInstance(ComponentInterface *instance)
-{
-   // Releases the resource.
-   std::unique_ptr < AudacityCommand > {
-      dynamic_cast<AudacityCommand *>(instance)
-   };
+   return Instantiate(path);
 }
 
 // ============================================================================

src/commands/LoadCommands.h

@@ -69,8 +69,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
 private:
    // BuiltinEffectModule implementation

src/effects/EffectManager.cpp

@@ -56,12 +56,12 @@ EffectManager::~EffectManager()
 
 // Here solely for the purpose of Nyquist Workbench until
 // a better solution is devised.
-const PluginID & EffectManager::RegisterEffect(Effect *f)
+const PluginID & EffectManager::RegisterEffect(std::unique_ptr<Effect> uEffect)
 {
-   const PluginID & ID = PluginManager::Get().RegisterPlugin(f, PluginTypeEffect);
-
-   mEffects[ID] = f;
-
+   auto pEffect = uEffect.get();
+   const PluginID & ID =
+      PluginManager::Get().RegisterPlugin(std::move(uEffect), PluginTypeEffect);
+   mEffects[ID] = pEffect;
    return ID;
 }
 

src/effects/EffectManager.h

@@ -75,10 +75,10 @@ public:
    EffectManager();
    virtual ~EffectManager();
 
-   /** (Un)Register an effect so it can be executed. */
-   // Here solely for the purpose of Nyquist Workbench until
-   // a better solution is devised.
-   const PluginID & RegisterEffect(Effect *f);
+   //! Here solely for the purpose of Nyquist Workbench until a better solution is devised.
+   /** Register an effect so it can be executed. */
+   const PluginID & RegisterEffect(std::unique_ptr<Effect> uEffect);
+   //! Used only by Nyquist Workbench module
    void UnregisterEffect(const PluginID & ID);
 
    TranslatableString GetEffectFamilyName(const PluginID & ID);

src/effects/LoadEffects.cpp

@@ -191,19 +191,11 @@ bool BuiltinEffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return mEffects.find( path ) != mEffects.end();
 }
 
-ComponentInterface *BuiltinEffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+BuiltinEffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
-   // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-   return Instantiate(path).release();
-}
-
-void BuiltinEffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   // Releases the resource.
-   std::unique_ptr < Effect > {
-      dynamic_cast<Effect *>(instance)
-   };
+   return Instantiate(path);
 }
 
 // ============================================================================

src/effects/LoadEffects.h

@@ -69,8 +69,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
 private:
    // BuiltinEffectModule implementation

src/effects/VST/VSTEffect.cpp

@@ -684,19 +684,12 @@ bool VSTEffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return wxFileName::FileExists(realPath) || wxFileName::DirExists(realPath);
 }
 
-ComponentInterface *VSTEffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+VSTEffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
    // For us, the ID is simply the path to the effect
-   // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-   return safenew VSTEffect(path);
-}
-
-void VSTEffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   std::unique_ptr < VSTEffect > {
-      dynamic_cast<VSTEffect *>(instance)
-   };
+   return std::make_unique<VSTEffect>(path);
 }
 
 // ============================================================================

src/effects/VST/VSTEffect.h

@@ -432,8 +432,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
    // VSTEffectModule implementation
 

src/effects/audiounits/AudioUnitEffect.cpp

@@ -377,25 +377,13 @@ bool AudioUnitEffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return FindAudioUnit(path, name) != NULL;
 }
 
-ComponentInterface *AudioUnitEffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+AudioUnitEffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
-   wxString name;
-   AudioComponent component = FindAudioUnit(path, name);
-   if (component == NULL)
-   {
-      return NULL;
-   }
-
-   // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-   return safenew AudioUnitEffect(path, name, component);
-}
-
-void AudioUnitEffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   std::unique_ptr < AudioUnitEffect > {
-      dynamic_cast<AudioUnitEffect *>(instance)
-   };
+   if (wxString name; auto component = FindAudioUnit(path, name))
+      return std::make_unique<AudioUnitEffect>(path, name, component);
+   return nullptr;
 }
 
 // ============================================================================

src/effects/audiounits/AudioUnitEffect.h

@@ -261,8 +261,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
    // AudioUnitEffectModule implementation
 

src/effects/ladspa/LadspaEffect.cpp

@@ -330,7 +330,8 @@ bool LadspaEffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return wxFileName::FileExists(realPath);
 }
 
-ComponentInterface *LadspaEffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+LadspaEffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
    // For us, the path is two words.
@@ -339,16 +340,7 @@ ComponentInterface *LadspaEffectsModule::CreateInstance(const PluginPath & path)
    long index;
    wxString realPath = path.BeforeFirst(wxT(';'));
    path.AfterFirst(wxT(';')).ToLong(&index);
-
-   // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-   return safenew LadspaEffect(realPath, (int)index);
-}
-
-void LadspaEffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   std::unique_ptr < LadspaEffect > {
-      dynamic_cast<LadspaEffect *>(instance)
-   };
+   return std::make_unique<LadspaEffect>(realPath, (int)index);
 }
 
 FilePaths LadspaEffectsModule::GetSearchPaths()

src/effects/ladspa/LadspaEffect.h

@@ -237,8 +237,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
    // LadspaEffectModule implementation
 

src/effects/lv2/LoadLV2.cpp

@@ -302,24 +302,13 @@ bool LV2EffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return GetPlugin(path) != NULL;
 }
 
-ComponentInterface *LV2EffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+LV2EffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
-   const LilvPlugin *plug = GetPlugin(path);
-   if (!plug)
-   {
-      return NULL;
-   }
-
-   // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-   return safenew LV2Effect(plug);
-}
-
-void LV2EffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   std::unique_ptr < LV2Effect > {
-      dynamic_cast<LV2Effect *>(instance)
-   };
+   if (auto plug = GetPlugin(path))
+      return std::make_unique<LV2Effect>(plug);
+   return nullptr;
 }
 
 // ============================================================================

src/effects/lv2/LoadLV2.h

@@ -192,8 +192,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
    // LV2EffectModule implementation
 

src/effects/nyquist/LoadNyquist.cpp

@@ -264,24 +264,14 @@ bool NyquistEffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return wxFileName::FileExists(path);
 }
 
-ComponentInterface *NyquistEffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+NyquistEffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
    auto effect = std::make_unique<NyquistEffect>(path);
    if (effect->IsOk())
-   {
-      // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-      return effect.release();
-   }
-
-   return NULL;
-}
-
-void NyquistEffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   std::unique_ptr < NyquistEffect > {
-      dynamic_cast<NyquistEffect *>(instance)
-   };
+      return effect;
+   return nullptr;
 }
 
 // ============================================================================

src/effects/nyquist/LoadNyquist.h

@@ -51,6 +51,6 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 };

src/effects/vamp/LoadVamp.cpp

@@ -236,27 +236,16 @@ bool VampEffectsModule::IsPluginValid(const PluginPath & path, bool bFast)
    return bool(vp);
 }
 
-ComponentInterface *VampEffectsModule::CreateInstance(const PluginPath & path)
+std::unique_ptr<ComponentInterface>
+VampEffectsModule::CreateInstance(const PluginPath & path)
 {
    // Acquires a resource for the application.
    int output;
    bool hasParameters;
 
-   auto vp = FindPlugin(path, output, hasParameters);
-   if (vp)
-   {
-      // Safety of this depends on complementary calls to DeleteInstance on the module manager side.
-      return safenew VampEffect(std::move(vp), path, output, hasParameters);
-   }
-
-   return NULL;
-}
-
-void VampEffectsModule::DeleteInstance(ComponentInterface *instance)
-{
-   std::unique_ptr < VampEffect > {
-      dynamic_cast<VampEffect *>(instance)
-   };
+   if (auto vp = FindPlugin(path, output, hasParameters))
+      return std::make_unique<VampEffect>(std::move(vp), path, output, hasParameters);
+   return nullptr;
 }
 
 // VampEffectsModule implementation

src/effects/vamp/LoadVamp.h

@@ -58,8 +58,8 @@ public:
 
    bool IsPluginValid(const PluginPath & path, bool bFast) override;
 
-   ComponentInterface *CreateInstance(const PluginPath & path) override;
-   void DeleteInstance(ComponentInterface *instance) override;
+   std::unique_ptr<ComponentInterface>
+      CreateInstance(const PluginPath & path) override;
 
 private:
    // VampEffectModule implementation