Incorrect destruction order for VST3 modules

[robbert-vdh]

Hi,

I'm currently working on adding support for VST3 plugins to yabridge. When testing this under Qtractor, I noticed that loading any plugin would segfault. This happens because Qtractor will unload the module through ModuleExit() before destroying the objects created by the module's factory (this happens here). This order could result in a use-after-free for any plugin that stores some form of shared information or state. Since Qtractor performs this destroying sequence at the start of qtractorVst3PluginType::open, this would happen every time you try to open a plugin.

Please let me know if you need any more information here.

[rncbc]

is that loading the same plugin a second time or is it about loading any pVST3 plugin, anytime? on what circumstances in particular, may I ask?

oh, is this only about loading WinVST3 ?

[robbert-vdh]

This happens when loading any VST3 plugin. For instance if I open Qtractor with GDB attached and set a breakpoint for qtractorVst3PluginType::Impl::close, then that breakpoint will be hit when I insert a VST3 plugin (for instance, the native VST3 version of Presswerk). And within that function the module gets unloaded before the processor and the controller objects are being destroyed.

EDIT: But the actual issue here is that ModuleExit() gets called before m_controller and m_component are being cleaned up.

[rncbc]

This happens when loading any VST3 plugin...

really? that is not my experience for quite this whole year-of-doom--that is, since VST3 support was actually introduced in qtractor ;)

actual issue here is that ModuleExit() gets called before m_controller and m_component are being cleaned up.

m_controller and m_component should be nullptr at the time of Impl::open(). that should be the case whenever loading any VST3 plugin instance...

again i ask: what circumstances is this happening? is that one plugin type in particular (might be) or ALL plugins in your case (strange as it is not the experience here, from my POV on native Linux at least).

[robbert-vdh]

The call to qtractorVst3PluginType::Impl::close happens for every VST3 plugin. It gets called here and here at the start of qtractorVst3PluginType::Impl::open and qtractorVst3PluginType::open respectively. But the thing I wanted to draw attention to here is not that the plugin is being closed immediately on open (although that seems a bit weird, since plugins now have to initialize twice before they can be used), but to this part I linked above:

    ...

    if (m_component) {
        m_component->terminate();
        typedef bool (PLUGIN_API *VST3_ModuleExit)();
        const VST3_ModuleExit module_exit
            = reinterpret_cast<VST3_ModuleExit> (m_pFile->resolve("ModuleExit"));
        if (module_exit)
            module_exit();   // (1)
    }

    m_controller = nullptr;  // (2)
    m_component = nullptr;   // (3)
}

Here at (1) the module gets unloaded, causing the VST3 plugin clean up all of its resources. Then at (2) and (3) the IPtr<T> get set to nullptr, which causes m_{component,controller}->release() to be called, which in turn does a delete this;. The issue here is that library code from the VST3 module is being called after the module has been unloaded. In my case that causes issues because I need to do some housekeeping in every object's destructor, and this would also cause issues for other plugins that do something similar.

The solution here would be to move the call to module_exit() to after (3).

[rncbc]

yes, i see your point, which seems to be a dang valid one...

onto my defense :) notwithstanding your sure valid point, here comes my u-he's presswerk screenshot of evidence:

a probable fix to all this mess will come shortly on the develop branch... thanks anyway

[robbert-vdh]

Yes Presswerk works absolutely fine! Sorry if that caused confusion. I just wanted to say that that qtractorVst3PluginType::Impl::close function was being called when opening any VST3 plugin, and the bug I was talking about happened within in that function :sweat_smile:.

VST3 is pretty wild, and while working on yabridge's VST3 support I've already seen way too much weird plugin and host behaviour (which is totally understandable, because the SDK is massive and the workflow diagrams are both incomplete and kind of overwhelming). But luckily with FOSS we can just try to try to fix these things instead of working around them :). Looking forward to the next Qtractor version!

[rncbc]

fix maybe? https://github.com/rncbc/qtractor/commit/fa47198

can you test && tell?

[robbert-vdh]

Sorry, it took a bit longer because I forgot to compile with --with-vst3 (and me being lazy I just used the AUR package, so I had to rebuild from scratch). I quickly tested the feature/vst3 branch of yabridge a few Melda plugins (which don't separate their processor and controller parts), iZotope VocalSynth 2 (which does some nastiness by exchanging pointers between its processor and controller), Spitfire LABS and Melodyne 5. Some of those plugins tend to do some weird things, which is why I just tested those first. Everything seems to work great now except for VocalSynth 2 opening with the wrong editor size and LABS segfaulting on IPlugView::attached. Those things work fine in other hosts, but I'll first have to check if those are issues on yabridge's side or if those things should be fixed within Qtractor (since some plugins are very strict on having to follow the exact order of events specified in the SDK). Thanks for the quick fix!

[robbert-vdh]

This has been fixed in the January release, so I'll close this to clean up your issue tracker a bit!