From a9afad17ca9d906834dd7bffb223dae5bd17454f Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Thu, 31 Mar 2016 09:42:16 -0400 Subject: [PATCH] Plugin instances managed without naked new and delete --- src/PluginManager.cpp | 14 ++++++++++++-- src/PluginManager.h | 5 +++++ src/effects/LoadEffects.cpp | 11 ++++++----- src/effects/VST/VSTEffect.cpp | 12 ++++++------ src/effects/audiounits/AudioUnitEffect.cpp | 12 ++++++------ src/effects/ladspa/LadspaEffect.cpp | 12 ++++++------ src/effects/lv2/LoadLV2.cpp | 12 ++++++------ src/effects/nyquist/LoadNyquist.cpp | 16 +++++++--------- src/effects/vamp/LoadVamp.cpp | 12 ++++++------ 9 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/PluginManager.cpp b/src/PluginManager.cpp index f564e637c..71aa260e9 100644 --- a/src/PluginManager.cpp +++ b/src/PluginManager.cpp @@ -1053,13 +1053,17 @@ PluginDescriptor::PluginDescriptor() } PluginDescriptor::~PluginDescriptor() +{ + DeleteInstance(); +} + +void PluginDescriptor::DeleteInstance() { if (mInstance) { ModuleManager::Get().DeleteInstance(GetProviderID(), mInstance); + mInstance = nullptr; } - - return; } bool PluginDescriptor::IsInstantiated() const @@ -1086,6 +1090,12 @@ IdentInterface *PluginDescriptor::GetInstance() void PluginDescriptor::SetInstance(IdentInterface *instance) { + if (mInstance && mInstance != instance) + { + // Be sure not to leak resources!! + DeleteInstance(); + } + mInstance = instance; return; diff --git a/src/PluginManager.h b/src/PluginManager.h index db5ab92d3..5db8e911c 100644 --- a/src/PluginManager.h +++ b/src/PluginManager.h @@ -117,8 +117,13 @@ public: private: + void DeleteInstance(); + // Common + // Among other purposes, PluginDescriptor acts as the resouce handle, + // or smart pointer, to a resource created in a plugin library, and is responsible + // for a cleanup of this pointer. IdentInterface *mInstance; PluginType mPluginType; diff --git a/src/effects/LoadEffects.cpp b/src/effects/LoadEffects.cpp index 12302fe54..1e9961478 100644 --- a/src/effects/LoadEffects.cpp +++ b/src/effects/LoadEffects.cpp @@ -327,16 +327,17 @@ bool BuiltinEffectsModule::IsPluginValid(const wxString & path) IdentInterface *BuiltinEffectsModule::CreateInstance(const wxString & path) { + // Acquires a resource for the application. + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. return Instantiate(path); } void BuiltinEffectsModule::DeleteInstance(IdentInterface *instance) { - Effect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + // Releases the resource. + std::unique_ptr < Effect > { + dynamic_cast(instance) + }; } // ============================================================================ diff --git a/src/effects/VST/VSTEffect.cpp b/src/effects/VST/VSTEffect.cpp index 1324d325b..acb066a10 100644 --- a/src/effects/VST/VSTEffect.cpp +++ b/src/effects/VST/VSTEffect.cpp @@ -623,17 +623,17 @@ bool VSTEffectsModule::IsPluginValid(const wxString & path) IdentInterface *VSTEffectsModule::CreateInstance(const wxString & path) { + // Acquires a resource for the application. // For us, the ID is simply the path to the effect - return new VSTEffect(path); + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. + return safenew VSTEffect(path); } void VSTEffectsModule::DeleteInstance(IdentInterface *instance) { - VSTEffect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + std::unique_ptr < VSTEffect > { + dynamic_cast(instance) + }; } // ============================================================================ diff --git a/src/effects/audiounits/AudioUnitEffect.cpp b/src/effects/audiounits/AudioUnitEffect.cpp index 45c1af3cd..85f55bea2 100644 --- a/src/effects/audiounits/AudioUnitEffect.cpp +++ b/src/effects/audiounits/AudioUnitEffect.cpp @@ -176,6 +176,7 @@ bool AudioUnitEffectsModule::IsPluginValid(const wxString & path) IdentInterface *AudioUnitEffectsModule::CreateInstance(const wxString & path) { + // Acquires a resource for the application. wxString name; AudioComponent component = FindAudioUnit(path, name); if (component == NULL) @@ -183,16 +184,15 @@ IdentInterface *AudioUnitEffectsModule::CreateInstance(const wxString & path) return NULL; } - return new AudioUnitEffect(path, name, component); + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. + return safenew AudioUnitEffect(path, name, component); } void AudioUnitEffectsModule::DeleteInstance(IdentInterface *instance) { - AudioUnitEffect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + std::unique_ptr < AudioUnitEffect > { + dynamic_cast(instance) + }; } // ============================================================================ diff --git a/src/effects/ladspa/LadspaEffect.cpp b/src/effects/ladspa/LadspaEffect.cpp index 0b2b7a2c2..b916fd79b 100644 --- a/src/effects/ladspa/LadspaEffect.cpp +++ b/src/effects/ladspa/LadspaEffect.cpp @@ -261,6 +261,7 @@ bool LadspaEffectsModule::IsPluginValid(const wxString & path) IdentInterface *LadspaEffectsModule::CreateInstance(const wxString & path) { + // Acquires a resource for the application. // For us, the path is two words. // 1) The library's path // 2) The LADSPA descriptor index @@ -268,16 +269,15 @@ IdentInterface *LadspaEffectsModule::CreateInstance(const wxString & path) wxString realPath = path.BeforeFirst(wxT(';')); path.AfterFirst(wxT(';')).ToLong(&index); - return new LadspaEffect(realPath, (int) index); + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. + return safenew LadspaEffect(realPath, (int)index); } void LadspaEffectsModule::DeleteInstance(IdentInterface *instance) { - LadspaEffect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + std::unique_ptr < LadspaEffect > { + dynamic_cast(instance) + }; } wxArrayString LadspaEffectsModule::GetSearchPaths() diff --git a/src/effects/lv2/LoadLV2.cpp b/src/effects/lv2/LoadLV2.cpp index 50c9d619c..779ebc933 100644 --- a/src/effects/lv2/LoadLV2.cpp +++ b/src/effects/lv2/LoadLV2.cpp @@ -266,22 +266,22 @@ bool LV2EffectsModule::IsPluginValid(const wxString & path) IdentInterface *LV2EffectsModule::CreateInstance(const wxString & path) { + // Acquires a resource for the application. const LilvPlugin *plug = GetPlugin(path); if (!plug) { return NULL; } - return new LV2Effect(plug); + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. + return safenew LV2Effect(plug); } void LV2EffectsModule::DeleteInstance(IdentInterface *instance) { - LV2Effect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + std::unique_ptr < LV2Effect > { + dynamic_cast(instance) + }; } // ============================================================================ diff --git a/src/effects/nyquist/LoadNyquist.cpp b/src/effects/nyquist/LoadNyquist.cpp index e39838ee1..d891ed64b 100644 --- a/src/effects/nyquist/LoadNyquist.cpp +++ b/src/effects/nyquist/LoadNyquist.cpp @@ -226,24 +226,22 @@ bool NyquistEffectsModule::IsPluginValid(const wxString & path) IdentInterface *NyquistEffectsModule::CreateInstance(const wxString & path) { - NyquistEffect *effect = new NyquistEffect(path); + // Acquires a resource for the application. + auto effect = std::make_unique(path); if (effect->IsOk()) { - return effect; + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. + return effect.release(); } - delete effect; - return NULL; } void NyquistEffectsModule::DeleteInstance(IdentInterface *instance) { - NyquistEffect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + std::unique_ptr < NyquistEffect > { + dynamic_cast(instance) + }; } // ============================================================================ diff --git a/src/effects/vamp/LoadVamp.cpp b/src/effects/vamp/LoadVamp.cpp index 038a26f85..518e6fcc1 100644 --- a/src/effects/vamp/LoadVamp.cpp +++ b/src/effects/vamp/LoadVamp.cpp @@ -234,13 +234,15 @@ bool VampEffectsModule::IsPluginValid(const wxString & path) IdentInterface *VampEffectsModule::CreateInstance(const wxString & path) { + // Acquires a resource for the application. int output; bool hasParameters; Plugin *vp = FindPlugin(path, output, hasParameters); if (vp) { - return new VampEffect(vp, path, output, hasParameters); + // Safety of this depends on complementary calls to DeleteInstance on the module manager side. + return safenew VampEffect(vp, path, output, hasParameters); } return NULL; @@ -248,11 +250,9 @@ IdentInterface *VampEffectsModule::CreateInstance(const wxString & path) void VampEffectsModule::DeleteInstance(IdentInterface *instance) { - VampEffect *effect = dynamic_cast(instance); - if (effect) - { - delete effect; - } + std::unique_ptr < VampEffect > { + dynamic_cast(instance) + }; } // VampEffectsModule implementation