VST Preview Crash

[musikBear]

0.9.90 p.r. v32bit
Platform winXP
Old bug
The 'pre-hear' feature is very nice for all factory-presets, but VSTs will launch a vsts-GUI without a lmms-handle. It can be closed through the window-context-menu, but it still exists in memory (eg leak). (with one especially large VST (Alionoctis), lmms crashed, but that can definately be my memory, and not related to lmms)

[diizy]

IMHO, we could just disable the preview function for VST's altogether, at least for now.

[tobydox]

It's disabled for DLL files already but I'd not disable it for regular LMMS presets. Additionally it'd be unneccessary extra logic to distinguish between good and bad presets.

[diizy]

On 03/18/2014 09:54 PM, Tobias Doerffel wrote:

It's disabled for DLL files already but I'd not disable it for regular
LMMS presets. Additionally it'd be unneccessary extra logic to
distinguish between good and bad presets.

Couldn't we just disable it for presets that are in the "vestige"
directory for now? It's an ugly fix, sure, but it'd let us get 1.0.0
released and maybe we could release a more complete fix in a point
release later on.

[tresf]

I just reproduced this crash with a very small VST (DSK Musicbox)

It seems it starts stacking the processes. It seemed to crash at 4 on my PC but after further investigation it seems it never unloads the RemoteVstPlugin32.exe (on windows at least) when the crash occurs.

To add insult to injury, when dragging a VeSTige preset in, it loads once on click, and then again on drag, so a drag and drop loads two instances right away.

These XPFs store the path to the DLLs in them... could we blacklist DLLs from previewing? I know @tobydox didn't want unnecessary extra logic in the preview process but as @musikBear stated, large VSTs probably shouldn't preview anyway. That should cover 99.9% and would be a bit more scalable than a hard coded folder name.

Personally, I really like the idea of a VST preview, this is a fantastic feature and I didn't even know it had existed, but the current process is reproducibly crash prone and probably better to be turned off using some form of file filter until we can determine how this could ever be done without duplication/triplication of child processes.

An idea is to perhaps put a file size limitation on autopreview that's configurable by the user to prevent the unnecessary memory footprint on single click (this may not even help with larger VSTs if they use multiple DLLs) but said size limitation wouldn't fix this particular bug until we can do our best at making a singleton drag-and-drop/preview process, auto-close preview changed process, and for safety perhaps a manually-closable-child process listing.

-Tres

[diizy]

Bumping milestone to 1.1 since we don't have time to fix this for 1.0.

[musikBear]

Looking fwd -closing for now

[tresf]

I tried creating a patch to suppress VSTs from previewing:

PresetPreviewHandle.cpp#L131

if ( dataFile.content().elementsByTagName( "vestige" ) == NULL )
{
   // load instrument
}

But it won't compile. Complains about accessing pointers that have private-only access.

/usr/include/qt4/QtXml/qdom.h:275:5: error: ‘QDomNodeList::QDomNodeList(QDomNodeListPrivate*)’ is private
~/lmms/src/core/PresetPreviewPlayHandle.cpp:152:61: error: within this context

I'm not sure this would be better anyways because this could potentially break all VST preset from previewing, right?

-Tres

[musikBear]

what result with an 'if !' statement, and loose compare to NULL
mmm so..
if ! ( dataFile.content().elementsByTagName( "vestige" )) { //load}
(rusty in code ..very :)

[diizy]

So do we have any idea for a fix for this, or shall we bump this up to 1.2?

[tresf]

I was hoping to put a quick XML check on the dataFile at or around PresetPreviewHandle.cpp#L131 but I had some struggles, mostly lack of knowledge with CPP.

I would really like to see this fixed for 1.1 because it is a relatively small check and fixes a reproducible hard crash, which I find to be the most intrusive to users (and in this case popular enough to be reported twice on our tracker).

[curlymorphic]

Im confused as to whats needed?

are we trying to stop previewing vst instruments, or any instrument with a vst effect on?

[tresf]

are we trying to stop previewing vst instruments

Yes.

or any instrument with a vst effect on?

Not sure about effects. Probably good to disable them too but for now, disabling the VST instrument presets would be a big win (or we focus on fixing the underlying crash).

-Tres

[curlymorphic]

or we focus on fixing the underlying crash

I cant reproduce a crash, but i do end up with orphaned vst effects.

Can someone test to see if this crash is still reproducible with master please?

[tresf]

I cant reproduce a crash, but i do end up with orphaned vst effects.

If you are on Linux, Wine may be handling this better. In this case, we may have to make a Windows installer to reproduce the actual crash.

[curlymorphic]

Some progress

@tresf your if statement was nearly there

if(dataFile.content().elementsByTagName( "vestige" ).length() == 0 )

is what we needed. But this leads onto another issue, what to do when i know its a vestige?

if i simply dont loadTrackSpecficSettings , then a preview of the last selected previewed track will play.
If there was no last preview then lmms segfaults.

I think the cleanest solution maybe to have a dummy preset, (any native synth with vol = 0 ) loaded instead. This dummy preset can simply be an xml string hard coded into the source, to save users ever finding the file.

Is this the way to proceed?

[tresf]

Is this the way to proceed?

Hmm... We have the concept of dummy instruments. A dummy preview/preset may be a good option.

[tresf]

Closed via #1587