Segmentation fault when instruments are connected through MIDI inputs/outputs

[Gregaras]

OS: GNU/Linux
Distribution: Manjaro

Bug Summary

Crash (segfault) related to instruments that are connected through MIDI inputs/outputs.

Steps to reproduce

1. Put two instruments in Song Editor.
2. Enable MIDI output in the first instrument
3. Enable MIDI input in the second instrument
4. Connect second instrument to the first one (through MIDI input, 128:0 LMMS:first)
5. Put a clip in first instrument track and put single note in it
6. Press and hold Space in Song Editor to play/stop repeatedly

Expected behavior

No crash.

Actual behavior

Crash, segmentation fault.

Screenshot

Affected LMMS versions (versions in which crash happens)

LMMS 1.3.0-alpha.1.278+gfb529a19c (master) (logs are from this version)
LMMS 1.2.2 (appimage)
LMMS 1.3.0-alpha.1.102+g89fc6c9 (appimage)

Logs

Thread 18 "lmms::MidiAlsaS" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff7ffff6c0 (LWP 172705)]
0x00005555557d6e35 in lmms::NotePlayHandle::noteOff (this=0x0, _s=0) at /home/user/lmms/src/core/NotePlayHandle.cpp:371
371		if( m_released )

(gdb) backtrace
#0  0x00005555557d6e35 in lmms::NotePlayHandle::noteOff(int)
    (this=0x0, _s=0)
    at /home/user/lmms/src/core/NotePlayHandle.cpp:371
#1  0x000055555595a460 in lmms::InstrumentTrack::processInEvent(lmms::MidiEvent const&, lmms::TimePos const&, int)
     (this=0x7ffff27c9680, event=..., time=..., offset=0)
    at /home/user/lmms/src/tracks/InstrumentTrack.cpp:363
#2  0x000055555583818f in lmms::MidiPort::processInEvent(lmms::MidiEvent const&, lmms::TimePos const&)
    (this=0x7ffff27c9888, event=..., time=...)
    at /home/user/lmms/src/core/midi/MidiPort.cpp:151
#3  0x00005555558315bb in lmms::MidiAlsaSeq::run()
    (this=0x5555565829f0)
    at /home/user/lmms/src/core/midi/MidiAlsaSeq.cpp:543
#4  0x00007ffff64ff32a in  () at /usr/lib/libQt5Core.so.5
#5  0x00007ffff5f788fd in  () at /usr/lib/libc.so.6
#6  0x00007ffff5ffaa60 in  () at /usr/lib/libc.so.6

[messmerd]

@Gregaras Why did you delete this? This bug is still unfixed and in master currently.

[Gregaras]

Ok, I will restore it.

[michaelgregorius]

The problem occurs on this line and it might be a threading problem (at least it feels like one). It seems that two different threads access m_notes.

I have changed the code as follows:

case MidiNoteOff:
{
	auto key = event.key();
	auto * nph = m_notes[key];
	if( nph != nullptr )
	{
		// do actual note off and remove internal reference to NotePlayHandle (which itself will
		// be deleted later automatically)
		Engine::audioEngine()->requestChangeInModel();
		m_notes[event.key()]->noteOff( offset );

		// ... Snip ... //
	}
	eventHandled = true;
	break;
}

At the time of the crash nph is not nullptr. However when m_notes is accessed all its fields are nullptr. So before entering the if condition m_notes holds a value that is not nullptr but then it is removed.

[luzpaz]

@michaelgregorius is there a PR for this yet ?

[michaelgregorius]

No, @luzpaz. I'll leave this to someone with more knowledge about the internals of the engine.