Crashes when pushing through MIDI input while cloning a Vestige track

[BluuDreamz]

Step 1: Place a TCO (3rd party VST instrument through Vestige) within the Song Editor and link it to your MIDI Step 2: Send MIDI key input while cloning the TCO (Vestige linked VST instrument)

LMMS 1.2.0 RC8 Windows 10 MIDI: AKM320

Edit: Specifically occurs only with 3rd party VST instruments loaded through Vestige. Built-in LMMS instruments are not affected by this step process.

[DomClark]

When you say "TCO", I think you mean "track". "TCO" refers to a block of notes (or sample, etc.) in the song editor. I can't reproduce the issue when cloning the track, but I do get a crash when sending MIDI while deleting the track. I suspect the issues are related, so I'll take a look at this. In the meantime, if you have any other information that might be useful, please let us know.

[DomClark]

@BluuDreamz mentioned on Discord that this bug could also be triggered when loading an VST through the file dialog in VeSTige. I was able to reproduce using this method. The crash when sending MIDI while deleting a track is unrelated - I will open a separate issue for this.

[DomClark]

Here are the relevant parts of a backtrace from the hung thread:

#0  0x00007ffc55c1aa24 in ntdll!ZwWaitForSingleObject () from /c/WINDOWS/SYSTEM32/ntdll.dll
#1  0x00007ffc52359252 in WaitForSingleObjectEx () from /c/WINDOWS/System32/KERNELBASE.dll
#2  0x000000006675659d in QBasicMutex::lockInternal(int) () from /home/Dominic/LMMS/vagrant-build/2019-06-11-165416/Qt5Core.dll
#3  0x0000000069a02f34 in vestigeInstrument::handleMidiEvent (this=0x6ee0020, event=..., time=..., offset=0) at /home/vagrant/lmms/plugins/vestige/vestige.cpp:399
#4  0x00000000004fc006 in InstrumentTrack::processOutEvent (this=0x6e5a7a0, event=..., time=..., offset=0) at /home/vagrant/lmms/src/tracks/InstrumentTrack.cpp:421
#5  0x000000000043bda5 in NotePlayHandle::noteOff (this=0x6e30508, _s=0) at /home/vagrant/lmms/src/core/NotePlayHandle.cpp:376
#6  0x00000000004fb6a0 in InstrumentTrack::processInEvent (this=0x6e5a7a0, event=..., time=..., offset=0) at /home/vagrant/lmms/src/tracks/InstrumentTrack.cpp:285
#7  0x000000000048facc in PianoView::focusOutEvent (this=0x1023f600) at /home/vagrant/lmms/src/gui/PianoView.cpp:686
...
#28 0x0000000004c5b4a8 in QWidget::setVisible(bool) () from /home/Dominic/LMMS/vagrant-build/2019-06-11-165416/Qt5Widgets.dll
#29 0x0000000066c84537 in VstPlugin::toggleEditorVisibility (this=0x100bada0, visible=1) at /home/vagrant/lmms/plugins/vst_base/VstPlugin.cpp:621
#30 0x0000000066c84012 in VstPlugin::showUI (this=0x100bada0) at /home/vagrant/lmms/plugins/vst_base/VstPlugin.cpp:544
#31 0x0000000069a02d83 in vestigeInstrument::loadFile (this=0x6ee0020, _file=...) at /home/vagrant/lmms/plugins/vestige/vestige.cpp:358
#32 0x0000000069a0540d in VestigeInstrumentView::openPlugin (this=0x101ee260) at /home/vagrant/lmms/plugins/vestige/vestige.cpp:721
...

A lock is taken in VeSTige before loading the new plugin. When the plugin is loaded, the GUI is opened, which takes focus away from the instrument track window so LMMS turns off any held notes. In doing so, it tries to send a MIDI message to the plugin, which tries to take the same lock again and hangs.

I guess we ought to do one or both of the following:

• Release the lock before opening the GUI.
• Kill any existing notes for the plugin without sending a note-off event.

[JohannesLorenz]

When we make LMMS realtime safe, incoming MIDI events from the UI must be queued anyways. In the best case, with a realtime safe, lockless ringbuffer. The audio thread (vestigeInstrument::play()) would handle them later (e.g. before it does what it does right now). (I think that should work)

For now, a mutexed ringbuffer or even a mutexed vector might suffice. In the scenario of the issue, the plugin mutex would be locked by the loading thread, and then the ringbuffer would be locked to append the note off events. So we have 2 mutexes instead of one. After that, both will be freed.

@DomClark does that make sense to you? If yes, I'd be happy to play around with a PR.

[DomClark]

That does make sense, and I agree that MIDI events from the UI should be passed to the processing thread for handling there, but I feel this is too big a change for stable-1.2. Unlocking the mutex earlier as I suggested above did indeed fix the hang, but the reason I haven't opened a PR with it yet is that I then hit a different crash later on in the code, which I also want to address.

[JohannesLorenz]

It sounds dangerous to me to release the mutex earlier. E.g., from the code, can you be sure that m_plugin->showUI() must not be mutexed?

Adding a simple ringbuffer doesn't seem that much to me, and who knows how many other issues that would fix?

[JohannesLorenz]

I can't reproduce the cloning bug with a9640c8 (stable-1.2). I connected a virtual keyboard from jack-MIDI that plays a note. In LMMS, this plays a note in a ZynAddSubFX VST.

Now, while the note is playing, I clone the track using the gear wheel. There's a short beep while the new instrument is loading, but no crash.

[JohannesLorenz]

@BluuDreamz Can you still reproduce this with the current stable-1.2 from git? I ask because I can't, maybe it has been solved meanwhile?

[tresf]

with the current stable-1.2 from git?

He's running Windows, is there an expectation that he's going to build the branch from scratch?

I can test on Windows 10 and stable-1.2, but the bug fails to mentions exactly which VST is being used.

Edit: The bug also fails to mention which architecture LMMS (32-bit versus 64-bit) is being tested as well as which architecture VST is being used (32-bit versus 64-bit).

[tresf]

@BluuDreamz Can you still reproduce this with the current stable-1.2 from git? I ask because I can't, maybe it has been solved meanwhile?

Reproduced with 1.2.1:

1. Install LMMS 1.2.1 64-bit for Windows
2. Add a VST instrument by dragging the DLL into the song editor. I used GTG DPC 3 drum pad, which is a 32-bit VST plugin.
3. Attach a MIDI device. In my case, I used an Alesis V25 miniature keyboard.
4. Route the MIDI through the VST instrument by clicking the :gear: icon, MIDI, Input, [MIDI device]
5. While playing notes (don't just hold down the key, but rather play multiple keys) clone the VST.
6. In my case I have to clone SEVERAL TIMES for the bug to occur. The first clone seems to survive most of the time. If possible, hold down many MIDI keys at a time.
7. LMMS will crash
8. Important, find RemoteVstPlugin32.exe using Task Manager and end the task, it'll keep running and prevent the MIDI device from being used on restart.

[JohannesLorenz]

@tresf Many thanks for trying.

If you have to clone several times, and you still have to press keys while cloning, this seems like a really seldom crash that will not annoy many users (I never even thought about playing music while cloning tracks...). I'm OK with re-milestoning it. On master, we'll have time to write a more elegant fix.

Re-milestoning in 7 days if no one disagrees.

[BluuDreamz]

@BluuDreamz Can you still reproduce this with the current stable-1.2 from git? I ask because I can't, maybe it has been solved meanwhile?

my apologies. i've obtained an SSD which called for a fresh install of windows as well as visual studio for use with cmake (doesn't work with 2019 and i can't be bothered to revert to 2017).

@tresf i can confirm i tested this bug on 64-bit LMMS as well as interchangeably attempted to produce it using 32 / 64-bit VSTs (both lead to crashing).

i can also confirm that using either the clone or remove function produces this bug, for what it's worth. the remove function seems to never let LMMS survive the predicted crash, as opposed to the clone function sometimes surviving the process (although i believe this is dependent on how fast you are spamming MIDI input).

@tresf Many thanks for trying.

If you have to clone several times, and you still have to press keys while cloning, this seems like a really seldom crash that will not annoy many users (I never even thought about playing music while cloning tracks...). I'm OK with re-milestoning it. On master, we'll have time to write a more elegant fix.

Re-milestoning in 7 days if no one disagrees.

i found this bug as i continuously spammed MIDI input while shuffling through VST instrument presets. i never stopped spamming MIDI input as i deleted/cloned the VST at times, which lead me to discover this in the first place. it might not be something the common user comes across, but it certainly does prove to be dangerous to anyone who accidentally sends MIDI input while performing any of the associated actions leading to the crash, essentially resulting in possible project loss.