Rare crash on replacing instruments while playing notes

PhysSong commented 1 year ago

Summary

LMMS may crash on replacing instruments while playing notes. This also happens when loading preset to an instrument track while it's playing.

Steps to reproduce

Open and the provided project file(alternatively, you can open any instrument and put a lot of notes there)
Try replacing instruments until it crash

a_lot_of_notes.mmp.zip

Since it's very hard to reproduce, you may want to modify the code like this:

diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp
index 0e75c990a..942081917 100644
--- a/src/tracks/InstrumentTrack.cpp
+++ b/src/tracks/InstrumentTrack.cpp
@@ -1027,9 +1027,11 @@ Instrument * InstrumentTrack::loadInstrument(const QString & _plugin_name,
        Q_ASSERT(!key);

    silenceAllNotes( true );
+   QThread::msleep(100);

    lock();
    delete m_instrument;
+   QThread::msleep(100);
    m_instrument = Instrument::instantiate(_plugin_name, this,
                    key, keyFromDnd);
    unlock();

Technical details

In InstrumentTrack::loadInstrument, m_instrument may be used while being deleted. Also, lock()ing after calling silenceAllNotes() doesn't prevent new notes to be added in between those two calls. When a NotePlayHandle is added between two calls, it may try to use its m_instrumentTrack->instrument() during their deletion(or after deletion) since the returned value itself is not null. In my cases, the crash always happened in InstrumentSoundShaping::releaseFrames , at the line calling m_instrumentTrack->instrument()->desiredReleaseFrames().

The same thing may happen in InstrumentTrack::loadTrackSpecificSettings, too.

anytizer commented 1 year ago

With your a_lot_of_notes.mmp.zip,

Progress bar remains at 0% while opening the project, not advancing.
It opens the project all of a sudden after half minutes of please-wait window.
Clicking anywhere would cause "Not responding" in the title (OS's behavior) but did not crash at all.
notes play well (green markers active in piano roll while playing)
Piano roll behaves almost frozen, while editing a note.
changing the instrument did not crash.
Some instrument presets crashed.

Few unique messages printed while the crash was met:

noteEnd() triggered without a corresponding activate()!
QIODevice::read (QFile, "-\lmms\samples\samples\empty.wav"): device not open
LocklessAllocator: No free space
Error loading icon pixmap "sid/filter": File not found
!!!Not enough samples

Following messages were not necessarily associated with the crash, but it would be good to address them. Seen while changing the instrument:

Freeboy Instrument:

noteEnd() triggered without a corresponding activate()!

Kicker Presets: eg. there is a difference in loading "Clap dry.xpf" and "Clap.xpf":

noteEnd() triggered without a corresponding activate()!

Kicker > SnareMarch.xpf:

JO-ID 6315252 already in use by automatablemodel:Decay time!
JO-ID 2498139 already in use by automatablemodel:High Frq Damp!
JO-ID 5231681 already in use by automatablemodel:Bass Cut!
JO-ID 1579810 already in use by automatablemodel:Treble Cut!
JO-ID 8103466 already in use by automatablemodel:Lowpass!
JO-ID 4889780 already in use by automatablemodel:Highpass!
JO-ID 6261168 already in use by automatablemodel:Lowpass!
JO-ID 4037431 already in use by automatablemodel:Highpass!
JO-ID 6293968 already in use by automatablemodel:Tone!
JO-ID 2846145 already in use by automatablemodel:Gradient!
JO-ID 7709628 already in use by automatablemodel:Lowpass!
JO-ID 7480205 already in use by automatablemodel:Highpass!
JO-ID 4036038 already in use by automatablemodel:Lowpass!
JO-ID 1042561 already in use by automatablemodel:Highpass!
JO-ID 92193 already in use by automatablemodel:Tone!
JO-ID 7520542 already in use by automatablemodel:Gradient!

SID Instrument:

Error loading icon pixmap "sid/filter": File not found
!!!Not enough samples

ZynAddSubFx Instrument: Too many messages printed continuously:

QtXmlWrapper::loadXMLfile(): empty data                  -- rarely
NOTES TOO MANY (> POLIPHONY) - (Part.cpp::NoteOn(..))   -- frequently
NOTES TOO MANY (> POLIPHONY) -  (Part.cpp::NoteOn(..))  -- less frequently

(Also, it looks like there are two different messages, later with an extra space after "-" and before "(Part.cpp".)

Here is how it surely crashes: Open the attached .mmp file. Choose TripleOscillator > Xylophon.xpf Try to import this in the instrument track and play.

Message seen:

QIODevice::read (QFile, "-\samples\samples\empty.wav"): device not open
LocklessAllocator: No free space

Common attributes are those presets trying to access -/samples/empty.wav will crash.

AmazingBubbles.xpf ... Xylophon.xpf

There are 21 files that access samples/empty.wav - which does not exist.

anytizer commented 1 year ago

Another crash condition:

Open the piano roll from song editor.
Optionally keep playing in a loop.
File > Open > [same file again].

It meets the following just before the crash.

https://github.com/LMMS/lmms/blob/507fa239ddffd2ed1f186d0a51016c85d9c2b5a9/src/gui/editors/PianoRoll.cpp#L847

Just Few notes are enough to crash. Basically you have to be piano roll, not in song editor, and be in debug mode.

sakertooth commented 1 year ago

TSan is a life saver for stuff like this. This is the report I got. Will try to fix soon.

WARNING: ThreadSanitizer: data race (pid=585581)
  Write of size 8 at 0x7f317dfa5748 by main thread:
    #0 lmms::InstrumentTrack::loadInstrument(QString const&, lmms::Plugin::Descriptor::SubPluginFeatures::Key const*, bool) /home/saker/Git/lmms/src/tracks/InstrumentTrack.cpp:1035:15 (lmms+0x72c6e0) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #1 lmms::gui::InstrumentTrackWindow::dropEvent(QDropEvent*) /home/saker/Git/lmms/src/gui/instrument/InstrumentTrackWindow.cpp:572:12 (lmms+0x68d59d) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #2 lmms::gui::InstrumentTrackView::dropEvent(QDropEvent*) /home/saker/Git/lmms/src/gui/tracks/InstrumentTrackView.cpp:289:30 (lmms+0x6d4013) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #3 lmms::gui::TrackLabelButton::dropEvent(QDropEvent*) /home/saker/Git/lmms/src/gui/tracks/TrackLabelButton.cpp:143:15 (lmms+0x6e2426) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #4 QWidget::event(QEvent*) <null> (libQt5Widgets.so.5+0x1af1f4) (BuildId: 39d7eaf37f0b5964cd014c95c22f073645994f3e)
    #5 lmms::gui::PluginDescWidget::mousePressEvent(QMouseEvent*) /home/saker/Git/lmms/src/gui/PluginBrowser.cpp:287:7 (lmms+0x5ae0f4) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #6 QWidget::event(QEvent*) <null> (libQt5Widgets.so.5+0x1af528) (BuildId: 39d7eaf37f0b5964cd014c95c22f073645994f3e)
    #7 <null> <null> (libc.so.6+0x27ccf) (BuildId: 316d0d3666387f0e8fb98773f51aa1801027c5ab)

  Previous read of size 8 at 0x7f317dfa5748 by thread T29:
    #0 lmms::InstrumentTrack::play(lmms::TimePos const&, short, int, int) /home/saker/Git/lmms/src/tracks/InstrumentTrack.cpp:691:8 (lmms+0x728b9d) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #1 lmms::Song::processNextBuffer() /home/saker/Git/lmms/src/core/Song.cpp:344:12 (lmms+0x4883f8) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #2 lmms::AudioEngine::renderStageNoteSetup() /home/saker/Git/lmms/src/core/AudioEngine.cpp:377:21 (lmms+0x345760) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #3 lmms::AudioEngine::renderNextBuffer() /home/saker/Git/lmms/src/core/AudioEngine.cpp:462:2 (lmms+0x346685) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #4 lmms::AudioEngine::fifoWriter::run() /home/saker/Git/lmms/src/core/AudioEngine.cpp:1292:50 (lmms+0x3480b3) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #5 <null> <null> (libQt5Core.so.5+0xf35d9) (BuildId: 3a99c60eec9dd07808b3cbddbd5a7f909931516a)

  Thread T29 'AudioEngine::fi' (tid=585614, running) created by main thread at:
    #0 pthread_create <null> (lmms+0x26bc16) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #1 QThread::start(QThread::Priority) <null> (libQt5Core.so.5+0xf1b84) (BuildId: 3a99c60eec9dd07808b3cbddbd5a7f909931516a)
    #2 lmms::Engine::init(bool) /home/saker/Git/lmms/src/core/Engine.cpp:89:17 (lmms+0x3d5fdc) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #3 lmms::gui::GuiApplication::GuiApplication() /home/saker/Git/lmms/src/gui/GuiApplication.cpp:135:2 (lmms+0x55c28e) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)
    #4 main /home/saker/Git/lmms/src/core/main.cpp:861:7 (lmms+0x74f804) (BuildId: a09c2067b77a6d5c056d5baf7b13b86188c736f2)

SUMMARY: ThreadSanitizer: data race /home/saker/Git/lmms/src/tracks/InstrumentTrack.cpp:1035:15 in lmms::InstrumentTrack::loadInstrument(QString const&, lmms::Plugin::Descriptor::SubPluginFeatures::Key const*, bool)

sakertooth commented 1 year ago

It seems calling requestChangesInGuard is not that effective from preventing the master audio thread from running (for reasons I do not know), so I probably need to rework that first. I have experienced this synchronization mechanism being very weak in the past, so I already started working on this, but I just need to finish it.

Edit: It also data races with itself, which is quite ironic.

Rossmaxx commented 4 months ago

@sakertooth update?

sakertooth commented 3 months ago

@sakertooth update?

Sorry, just seeing this. I'll try and see if this issue is still here.

sakertooth commented 3 months ago

Because of the nature of this bug, its hard to say if this is truly fixed, but I don't think I am getting the report I mentioned previously. There are still a lot of data races to resolve (I forgot how running TSan with LMMS just makes it spit out maybe 10 reports every minute about a data race), so its hard to tell just by sifting through the output.

LMMS / lmms