Hydrogen: H2 assert : patterns size failed

[thijz]

Hydrogen version * : 1.0.0-'24f984a'
Operating system + version : ubuntu 16.04.2
Audio driver + version : jackdmp

---

just had a couple of crashes and using gdb i was able to get a hint of what is going wrong :

/src/core/src/basics/pattern_list.cpp:96: H2Core::Pattern* H2Core::PatternList::get(int): Assertion `idx >= 0 && idx < __patterns.size()' failed.

The only thing i saw with this song is that 1 of the patterns was 31 steps long while all other were 32. setting all patterns to 32 fixed it (so it seems)

Any idea what could cause this unpredictable behavior ?

BTW: i know this is an old H2 version, but it's the one i use on my live setup and it's been (relatively :-) stable.. Have a gig in 2 weeks time so i dont want to rush in to upgrading right now

[mauser]

Hi,

just for reference: this situation happened to me with today's current H2 version, everytime in pretty basic situations during testing of other features. But currently i'm not able to reproduce it..

[thijz]

i'll do my best to reproduce this, but it doesnt seem very easy ...
i created a song that combines all sorts of patterns of different length, but no matter what combination i make i cant reproduce it. yet.

Assuming i _do_ reproduce it at some point, what would be the most efficient way to get debug info about the bug ?

[thijz]

more info from a new crash :

hydrogen: /home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/src/core/src/basics/pattern_list.cpp:96: H2Core::Pattern* H2Core::PatternList::get(int): Assertion `idx >= 0 && idx < __patterns.size()' failed.

Thread 1 "hydrogen" received signal SIGABRT, Aborted.
0x00007ffff5afb428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
54  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.

and the backtrace

(gdb) bt
#0  0x00007ffff5afb428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff5afd02a in __GI_abort () at abort.c:89
#2  0x00007ffff5af3bd7 in __assert_fail_base (fmt=<optimized out>, 
    assertion=assertion@entry=0x7ffff7b85f50 "idx >= 0 && idx < __patterns.size()", 
    file=file@entry=0x7ffff7b85eb0 "/home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/src/core/src/basics/pattern_list.cpp", line=line@entry=96, 
    function=function@entry=0x7ffff7b86120 <H2Core::PatternList::get(int)::__PRETTY_FUNCTION__> "H2Core::Pattern* H2Core::PatternList::get(int)")
    at assert.c:92
#3  0x00007ffff5af3c82 in __GI___assert_fail (
    assertion=0x7ffff7b85f50 "idx >= 0 && idx < __patterns.size()", 
    file=0x7ffff7b85eb0 "/home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/src/core/src/basics/pattern_list.cpp", line=96, 
    function=0x7ffff7b86120 <H2Core::PatternList::get(int)::__PRETTY_FUNCTION__> "H2Core::Pattern* H2Core::PatternList::get(int)") at assert.c:101
#4  0x00007ffff7b37b23 in H2Core::PatternList::get (this=0xb62b40, idx=1)
    at /home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/src/core/src/basics/pattern_list.cpp:96
#5  0x0000000000603d82 in PatternEditorRuler::updateEditor (this=0x3ee2200, 
    bRedrawAll=false)
    at /home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/src/gui/src/Pa---Type <return> to continue, or q <return> to quit---

---Type <return> to continue, or q <return> to quit---
tternEditor/PatternEditorRuler.cpp:131
#6  0x00000000006208ae in PatternEditorRuler::qt_static_metacall (_o=0x3ee2200, _c=QMetaObject::InvokeMetaMethod, _id=1, _a=0x7fffffffd430)
    at /home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/build/src/gui/src/PatternEditor/moc_PatternEditorRuler.cpp:74
#7  0x00007ffff69e4d2a in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#8  0x00007ffff69f15c8 in QTimer::timerEvent(QTimerEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007ffff69e5bb3 in QObject::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#10 0x00007ffff74c605c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#11 0x00007ffff74cb516 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007ffff69b638b in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#13 0x00007ffff6a0b5ed in QTimerInfoList::activateTimers() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#14 0x00007ffff6a0baf1 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#15 0x00007ffff43b9197 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007ffff43b93f0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#17 0x00007ffff43b949c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#18 0x00007ffff6a0c7eb in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00007ffff69b3b4a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007ffff69bbbec in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x00000000005740b5 in main (argc=4, argv=0x7fffffffe3b8) at /home/thijs/Music/Hydrogen_Code/hydrogen_thijz_mod_midifix/src/gui/src/main.cpp:391

[thijz]

just had this crash again on todays code 51cddb057bd59b5dbe5cbb809b8edd0a59241d9f

hydrogen: /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp:96: H2Core::Pattern* H2Core::PatternList::get(int): Assertion `idx >= 0 && idx < __patterns.size()' failed.

Thread 1 "hydrogen" received signal SIGABRT, Aborted.
0x00007ffff5ae2428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
54  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.

backtrace :

(gdb) bt
#0  0x00007ffff5ae2428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff5ae402a in __GI_abort () at abort.c:89
#2  0x00007ffff5adabd7 in __assert_fail_base (fmt=<optimized out>, 
    assertion=assertion@entry=0x7ffff7b814e8 "idx >= 0 && idx < __patterns.size()", 
    file=file@entry=0x7ffff7b81450 "/home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp", line=line@entry=96, 
    function=function@entry=0x7ffff7b816c0 <H2Core::PatternList::get(int)::__PRETTY_FUNCTION__> "H2Core::Pattern* H2Core::PatternList::get(int)")
    at assert.c:92
#3  0x00007ffff5adac82 in __GI___assert_fail (
    assertion=0x7ffff7b814e8 "idx >= 0 && idx < __patterns.size()", 
    file=0x7ffff7b81450 "/home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp", line=96, 
    function=0x7ffff7b816c0 <H2Core::PatternList::get(int)::__PRETTY_FUNCTION__> "H2Core::Pattern* H2Core::PatternList::get(int)") at assert.c:101
#4  0x00007ffff7b2754f in H2Core::PatternList::get (this=0xa77130, idx=0)
    at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp:96
#5  0x000000000054012c in SongEditorPositionRuler::paintEvent (this=0xd295d0, 
    ev=0x7fffffffcf20)
    at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/gui/src/SongEdi---Type <return> to continue, or q <return> to quit---
tor/SongEditor.cpp:2078
#6  0x00007ffff74eff88 in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#7  0x00007ffff74ad05c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#8  0x00007ffff74b2516 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#9  0x00007ffff699d38b in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#10 0x00007ffff74e8a79 in QWidgetPrivate::sendPaintEvent(QRegion const&) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#11 0x00007ffff74e90c1 in QWidgetPrivate::drawWidget(QPaintDevice*, QRegion const&, QPoint const&, int, QPainter*, QWidgetBackingStore*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007ffff74ba856 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#13 0x00007ffff74baa8c in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007ffff74d8c1f in QWidgetPrivate::syncBackingStore() () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00007ffff74efd88 in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#16 0x00007ffff7605d7b in QMainWindow::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007ffff74ad05c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007ffff74b2516 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#19 0x00007ffff699d38b in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007ffff699f786 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x00007ffff69f33c3 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#22 0x00007ffff4169197 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#23 0x00007ffff41693f0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#24 0x00007ffff416949c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#25 0x00007ffff69f37cf in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007ffff699ab4a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#27 0x00007ffff69a2bec in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#28 0x0000000000586977 in main (argc=4, argv=0x7fffffffe3b8) at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/gui/src/main.cpp:406
(gdb) 

[thijz]

aarggg !! this is the most irritating bug ever! h2 ran just fine for over 3 hours while editing several songs in a playlist, skipping to the next, stop, play, etc ...
Next day during rehearsal : i'm 3 min in the first song (with out doing anything but pressing play) : crash !
i just cant figure out what triggers this...

[theGreatWhiteShark]

Do you have a feeling whether this is a more general problem with respect to the mismatching pattern length or closely related to some workflow/songs of yours?

I will give it a shot during the weekend.

[thijz]

Hi @theGreatWhiteShark
I have triple-checked that all patterns are of the same size (per song). Almost all songs are 32steps, so no exotic stuff.
Also this happens only ocasionally at random moments... I will continue to test and try to find a way to reproduce this.
Is there anything else that i can do if rhis happens again ? For the moment i'm always running H2 in gdb, so maybe there is something extra i can check when this crash happens again?

Happy to help !
Thijs

[theGreatWhiteShark]

It's quite a nuisance but I would add some log messages or just prints to stdout, start H2 with the --verbose=Debug argument, and pipe all the output in a log file.

For now gdb did an awesome job in pointing us to the very line in the very function that causes the issue. I would add some logs for the input argument idx, some important members of the __patterns object, and a small but unique note just before the PatternList::get() function is called in the code (for all calls). It's tedious but hopefully it just need one more crash and we now where and why it is failing.

[thijz]

hi @theGreatWhiteShark
i'm actuall running H2 with the "-VDebug" argument already. this gives me extra debug info, but there is no debug info at all just before the crash

I just had the crash again (now also pasting some of the debug output from just before the crash) :

(I) JackMidiDriver::handleMidiMessage [end of handleMidiMessage]
(I) JackMidiDriver::handleMidiMessage [start of handleMidiMessage]
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] channel: 8
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] val1: 21
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] val2: 33
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] CONTROL_CHANGE Parameter: 21, Value: 33
(I) JackMidiDriver::handleMidiMessage [end of handleMidiMessage]
(I) JackMidiDriver::handleMidiMessage [start of handleMidiMessage]
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] channel: 8
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] val1: 21
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] val2: 34
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] CONTROL_CHANGE Parameter: 21, Value: 34
(I) JackMidiDriver::handleMidiMessage [end of handleMidiMessage]
(I) JackMidiDriver::handleMidiMessage [start of handleMidiMessage]
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] channel: 8
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] val1: 21
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] val2: 35
(I) JackMidiDriver::handleMidiMessage [handleMidiMessage] CONTROL_CHANGE Parameter: 21, Value: 35
(I) JackMidiDriver::handleMidiMessage [end of handleMidiMessage]
(I) SongWriter::writeSong Saving song /home/thijs2/Music/hydrogen songs/_BHBM/hoist_that_rag_V3_LnV.autosave.h2song
(I) SongWriter::writeSong Save was successful.
(I) SongWriter::writeSong Saving song /home/thijs2/Music/hydrogen songs/_BHBM/hoist_that_rag_V3_LnV.autosave.h2song
(I) SongWriter::writeSong Save was successful.
hydrogen: /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp:96: H2Core::Pattern* H2Core::PatternList::get(int): Assertion `idx >= 0 && idx < __patterns.size()' failed.

Thread 1 "hydrogen" received signal SIGABRT, Aborted.
0x00007ffff5ae2428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
54  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.

and the gdb traceback :

(gdb) bt
#0  0x00007ffff5ae2428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff5ae402a in __GI_abort () at abort.c:89
#2  0x00007ffff5adabd7 in __assert_fail_base (fmt=<optimized out>, assertion=assertion@entry=0x7ffff7b814e8 "idx >= 0 && idx < __patterns.size()", 
    file=file@entry=0x7ffff7b81450 "/home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp", line=line@entry=96, 
    function=function@entry=0x7ffff7b816c0 <H2Core::PatternList::get(int)::__PRETTY_FUNCTION__> "H2Core::Pattern* H2Core::PatternList::get(int)")
    at assert.c:92
#3  0x00007ffff5adac82 in __GI___assert_fail (assertion=0x7ffff7b814e8 "idx >= 0 && idx < __patterns.size()", 
    file=0x7ffff7b81450 "/home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp", line=96, 
    function=0x7ffff7b816c0 <H2Core::PatternList::get(int)::__PRETTY_FUNCTION__> "H2Core::Pattern* H2Core::PatternList::get(int)") at assert.c:101
#4  0x00007ffff7b2754f in H2Core::PatternList::get (this=0xaae480, idx=0)
    at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/core/src/basics/pattern_list.cpp:96
#5  0x0000000000619d3e in PatternEditorRuler::updateEditor (this=0x294cea0, bRedrawAll=false)
    at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/gui/src/PatternEditor/PatternEditorRuler.cpp:131
#6  0x000000000062cb38 in PatternEditorRuler::qt_static_metacall (_o=0x294cea0, _c=QMetaObject::InvokeMetaMethod, _id=1, _a=0x7fffffffd480)
    at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/build/src/gui/moc_PatternEditorRuler.cpp:74
#7  0x00007ffff69cbd2a in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#8  0x00007ffff69d85c8 in QTimer::timerEvent(QTimerEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007ffff69ccbb3 in QObject::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#10 0x00007ffff74ad05c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#11 0x00007ffff74b2516 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007ffff699d38b in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#13 0x00007ffff69f25ed in QTimerInfoList::activateTimers() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#14 0x00007ffff69f2af1 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#15 0x00007ffff4169197 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007ffff41693f0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#17 0x00007ffff416949c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#18 0x00007ffff69f37eb in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00007ffff699ab4a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007ffff69a2bec in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x0000000000586977 in main (argc=4, argv=0x7fffffffe3e8) at /home/thijs2/Music/Hydrogen_Code/hydrogen_11april2019/src/gui/src/main.cpp:406
(gdb) 

[thijz]

this one messed up our live set last friday :-(
@theGreatWhiteShark : if you want to add extra debug output i'm more than happy to test until i hit this bug again (maybe on a dedicated branch ?)
Dont hesitate to let me know how i can help

[theGreatWhiteShark]

Oh no. :( What a drag.

Thing is, since I neither use the playlist nor any MIDI functionality of H2 but just a deterministic setting with all songs wrapped in NSM sessions, I never encountered this bug myself.

But sure, I prepare you a branch with loads of log messages, which hopefully help us to track down the culprit.

[theGreatWhiteShark]

Here you go https://gitlab.com/theGreatWhiteShark/hydrogen/tree/debug714

It produces tons of messages so be sure to rotate the log or your disk might run full half the way. You can do so with rotatelogs from the apach2-utils package.

hydrogen --verbose=Debug | rotatelogs -n 2 h2.log 1M

[theGreatWhiteShark]

Does H2 crash right after switching to the next Song in the playlist?

I just got it to freeze two times in a row when button jamming at different Songs in a Playlist. The error occured both times in SongEditorPatternList::createBackground() in the middle of looping through the pattern list. In addition, I got a QXcbConnection error. I will look at this issue more during the weekend and try to boil it down to something deterministic.

P.S. your hint on enabling JACK outputs per instrument, which are create right before the above function is called, might have done the trick.

[thijz]

Does H2 crash right after switching to the next Song in the playlist?

no, H2 crashes at random moments

building your debug branch while i'm typing this (thanks !!) and will start testing right away
will post back as soon as i have more info

[thijz]

It produces tons of messages

no kidding ! :-)
had to increase the number of files to 15 and filesize to 2M to have at least 20sec of logging data !

and the bad news is : i cant get h2 to crash ...
maybe the extra logging introduces enough delay to 'fix' the race condition ?
will keep trying

[theGreatWhiteShark]

You could remove the log messages in PatternList::get() to reduce the amount of log messages by half. I only included them to ensure we do not miss a single instance they are called.

Could you elaborate more on your setup? Maybe it's some minor option causing all of this. In the end the error descrivbed above I do only get when enabling JACK outputs per instrument. And I use a darn slow i3 laptop and was compiling a bunch of stuff meanwhile to simulate heavy load.

[theGreatWhiteShark]

Also, since both segmentation faults you reported above did occur in the PatternEditorRuler::updateEditor() function, I can prepare (maybe tomorrow) a debug version tailored to just display information about this function and the state it is in.

[theGreatWhiteShark]

This version only prints messages in the PatternEditorRuler::updateEditor() function. Maybe this will do the job. https://gitlab.com/theGreatWhiteShark/hydrogen/tree/debug714-2

[thijz]

thaks @theGreatWhiteShark !!
just compiled your '-2' version and running tests right now. fingers crossed !

[thijz]

i think i finally managed to capture the bug (log is in att > you can search for 'assertion')
i wasnt able to reproduce the issue using your 714-2 branch so i switched back to the original 714 branch.
i dont think this is what caused the bug to be triggered, just luck ...

i really hope this gives us some more hints on what goes wrong
patternsize_crash.txt

[thijz]

and a second report
crash2.txt.tar.gz

[theGreatWhiteShark]

Thanks. With a little luck I might find some time to look into it already during the next days

[thijz]

notes :

• both crashes occurred after more than 1 hour of continuous looping the same song (different songs for the 2 crash reports)
• the gdb backtrace info is also at the bottom of the log
• the 'patterns.size' value that is being logged is not the size of the pattern, right ? all of my patterns are 32 steps (except for the second crash where i deliberately added 2 extra patterns of 30 and 31 steps). i see all kinds of values but never 32. So what exactly is it ?

[theGreatWhiteShark]

both crashes occurred after more than 1 hour of continuous looping the same song (different songs for the 2 crash reports)

Uh. Smells like an overflow 😁

the 'patterns.size' [cut] So what exactly is it ?

Actually, the design of the basic types of H2 is quite bloated IMHO and I still not fully understand a number of details. That member seems to be one those. It's on my todo list to delve into them, add some documentation, and to write another section in the developer documentation (see #737) describing all the fundamental types.

In addition, there are bugs lurking in the custom read/write functions and their initialization (as we have seen recently with the Preferences class). It crossed my mind several times to rebase them on ~proper~ established serialization packages. This would not only allow to handle them way more cleanly but also to seemlessly read and save them from different programming languages (ever tried to change the drumkit and switching instruments on a couple of song in an automated way? I use some Elisp functions to handle this but there might be an actual need and definitely a better solution). But there are much more 'urgent' things to do before that might happen.

[thijz]

some more info
i was using jack since i suspected jack to have something to do with triggering the issue, especially since i run jack transport with H2 as jack transport master
however, running H2 using ALSA directly iso jack i was also able to reproduce it
i have the feeling that it has to do with system load and (i guess) with the number of x-runs this can cause
maybe the crash is just a side effect of something else that is triggered by xruns

i see things like this :

(I) ::int H2Core::findPatternInTick(int, bool, int*) ping 4
(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) ::int H2Core::findPatternInTick(int, bool, int*) ping 4
(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) SongEditorPositionRuler::paintEvent ping
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) i: 0
(I) PatternList::get get: idx: 0; __patterns.size(): 0
(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) i: 1
(E) PatternList::get idx 0 out of [0;0]
(I) PatternList::get get: idx: 1; __patterns.size(): 3
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) i: 2
(I) PatternList::get get: idx: 2; __patterns.size(): 3
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) nPat: 0
(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) nPat: 1
(I) PatternList::get get: idx: 1; __patterns.size(): 3
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) nPat: 2

and this :

(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) PatternEditorRuler::updateEditor i: 1
(I) PatternList::get get: idx: 1; __patterns.size(): 3
(I) PatternEditorRuler::updateEditor i: 2
(I) PatternList::get get: idx: 2; __patterns.size(): 3
(E) ::void* H2Core::alsaAudioDriver_processCaller(void*) XRUN
(I) PatternEditorRuler::updateEditor nSelectedPatternNumber: 0
(I) PatternList::get get: idx: 0; __patterns.size(): 10
(I) PatternEditorRuler::updateEditor i: 0
(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) PatternEditorRuler::updateEditor i: 1
(I) PatternList::get get: idx: 1; __patterns.size(): 3

[theGreatWhiteShark]

Ok, it's an ugly race condition.

(I) SongEditorPositionRuler::paintEvent ping
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) i: 0
(I) PatternList::get get: idx: 0; __patterns.size(): 0
(I) PatternList::get get: idx: 0; __patterns.size(): 3
(I) ::int H2Core::audioEngine_updateNoteQueue(unsigned int) i: 1
(E) PatternList::get idx 0 out of [0;0]

SongEditorPositionRuler::paintEvent() obtains a pointer to the m_pPlayingPatterns object and performs a for loop over all its entries to check which of its patterns is currently playing. This happens in the thread of the Qt-based GUI. During the same time the audioEngine_updateNoteQueue() function updates the very same m_pPlayingPatterns object. In rare occasions this can happen at the same time and the assertion in PatternList::get() will be triggered (which under normal circumstances should be impossible to set off). Same thing for PatternEditorRuler::updateEditor() in the other bug report.

To deal with this kind of problems, all mutable objects should be copied in the context of the GUI instead being just referenced by pointers. But this is major endeavour. Not only because of the vast number of classes in the GUI, but also because from the perspective of the code it's not clear at all which objects handled by the core are actually mutable. Intuitively, I would say we need only to tackle objects altered by the audio engine. But it wouldn't surprise me if those end up to be more than 50% of all objects defined in the core part.

[thijz]

OK, great to see that your were able to find the root cause of this issue !!
Not so great that this is not a trivial fix :-(

The reason i have been trying hard to reproduce this issue is because this is the bug that has f*cked up 2 of my live gigs. not a great feeling.

How about this for ugly fix: i build 2 versions of H2

• 1 'regular' version that i use to create my songs
• 1 'tweaked' version where i somehow disable the 2 Ruler update routines and that i can use for playing live (for playing live i think i can do without the moving playhead)
  Or are the 2 ruler update routines that you mention only 2 of many things that can go wrong because of the way the GUI currently works ? For example : i also use the director ...

Thanks for your work @theGreatWhiteShark !! i really appreciate it !
Thijs

[theGreatWhiteShark]

The reason i have been trying hard to reproduce this issue is because this is the bug that has f*cked up 2 of my live gigs. not a great feeling.

Yeah, that's the worst case scenario and should never happen again. In addition, you are most probably not the only one affected by it.

1 'tweaked' version where i somehow disable the 2 Ruler update routines and that i can use for playing live (for playing live i think i can do without the moving playhead)

Could work. Would you post the corresponding branch in here? Maybe someone else has the same problem until we figure out a way to resolve this bug.

Or are the 2 ruler update routines that you mention only 2 of many things that can go wrong because of the way the GUI currently works ?

Fortunately not. The current culprit, the m_pPlayingPatterns object, is only queried in those two functions and in AudioEngineInfoForm. But there could be very well another race condition lurking.

Thing is, I have no idea right now how to produce a dirty fix. Since the object mentioned is a list of pointers to the individual patterns, we would have to do a deep copy in the GUI. And to do this, it would require to lock the core part. Else the data structure could change during copying and we would be in even deeper trouble. But we certainly do not want to allow the GUI to look our real-time audio process either. I think what we need instead is a thorough understanding of all data structures/classes involved and a clever way to redesign some flawed part to avoid the race condition the begin with. I guess I will need more than two weeks to do it (the dirty one) since I'm quite busy right now and I have to read into a couple of C++ documentation beforehand.

@mauser what are your thoughts on this bug or a possible workaround? You certainly do see a bigger picture than I do.

[mauser]

Hi!

I suppose that such race conditions can't be avoided easily. The two current bugs in the update() routines are quite a worst case, since patterns can change quickly (m_pPlayingPatterns seems to be constantly beeing rebuild, which is also quite irritating) and UI drawing is quite slow. Very slow, if you have a PC with a low performance.

Copying is quite complex since you can't copy only one object (since on objects often aggregates other objects via pointer). You would have to copy the complete internal state of Hydrogen at one point in time and work an that (read-only) copy. There migtht be places where copying (free-standing) objects is feasiblem, but this will be not the 80% case with Hydrogen.

In my opinion the only feasible solution at the moment is to lock the audio engine while the UI makes iterations over variables / objects / lists which are being changed frequently by the audio engine. I can prepare a branch with such a quick fix to see if this solution is feasible and helpful.

[mauser]

@thijz : Have a look at the branch https://github.com/hydrogen-music/hydrogen/tree/Bugfix_UI_PatternList_Locks .

[theGreatWhiteShark]

Yesterday I read some pages about the vector class in the STL and there might be a more convenient way of solving this issue.

We could ensure the GUI only uses the range for and another, more safe, member function to access a pattern in a patternlist. In PatternList::get the individual Patterns are accessed via __pattern[idx] which does not provide any range checking at all. Instead, the methods invoked by the GUI should access the components using the vector::at(idx) method. It uses range checking and throws an out_of_range exception if anything goes wrong. It also can be about 10% slower but this weight is only for the GUI and not the audio engine to carry.

The GUI code could be something like this (I did not found time to implement it yet)

success = false;

while (!success) {
  try{
       get_pattern_function();
       success = true;
  } catch( out_of_range& ){
       INFOLOG(QString("Updating pattern failed"));
  }
}

Instead of locking the audio engine and making the UI top priority, this would let the audio engine run smoothly and forces the UI to retry getting the patterns until it was able to.

But this shouldn't cover the fact that the audio engine and its object management does need some care.

[thijz]

@mauser : will test the 'lock' branch asap
thnx!

[thijz]

did a quick test on the 'gui lock' brach and it seems that i get clicks and pops when i quickly select different patterns in the song editor while playing
sounds like xruns, but jack doesnt report any
running in verbose mode but i only see this (constantly) after some time :

(W) JackAudioDriver::updateTransportInfo Resyncing! bbt_frame_offset: 0

[thijz]

have been looping for a couple of hours and it does seem stable
will test more tomorrow

[mauser]

did a quick test on the 'gui lock' brach and it seems that i get clicks and pops when i quickly select different patterns in the song editor while playing
sounds like xruns, but jack doesnt report any
running in verbose mode but i only see this (constantly) after some time :

(W) JackAudioDriver::updateTransportInfo Resyncing! bbt_frame_offset: 0

Could you re-test this issue with the current version of the branch? I've pushed an update which takes the cost-intensive parts out of the locked area.

[mauser]

We could ensure the GUI only uses the range for and another, more safe, member function to access a pattern in a patternlist. In PatternList::get the individual Patterns are accessed via __pattern[idx] which does not provide any range checking at all. Instead, the methods invoked by the GUI should access the components using the vector::at(idx) method. It uses range checking and throws an out_of_range exception if anything goes wrong. It also can be about 10% slower but this weight is only for the GUI and not the audio engine to carry.

Hi!

In my opinion, such code should be only the last option if all other approaches fail. And one has to be very careful to handle the errors correctly, since you don't want the UI to freeze or show graphical artifacts. In addition, it works best in read-only places.

I've did another approach today by splitting up the pattern processing and the time-intense UI code in SongEditorPatternList::createBackground() (3537f5262f17bee00e7c1ddb0815c3406e747948). After that, i did a very simple profiling (with std::chrono) to see how long each of those parts run. You can see the results here: https://gist.github.com/mauser/1e92e560de0f57e86987c1cbae9f4045 (those numbers are microseconds, not milliseconds as my output suggests).
I hope that @thijz won't recognize any pops or other noises anymore, since the locked period is now a lot smaller.

[theGreatWhiteShark]

Ah, okay. I just read a book about C++ praising error handling as one of the ultimate solutions. But I do not have any experience in writing concurrent code in C++ yet.

Another short question regarding the topic: why not using a shared_mutex and a shared_lock for read access and a unique_lock for write access? This way the locking would be done by the engine and only at times when it is actually required. But again, I have no practical experience with those STL features yet. Just read about them while searching for a solution for this issue.

[thijz]

tested latest 'gui lock' version and i can confirm that the pops are gone and i see no gui lag or any other issues
so far so good but i need to test some more to be 100% sure
thanks for the update !!

[mauser]

Another short question regarding the topic: why not using a shared_mutex and a shared_lock for read access and a unique_lock for write access? This way the locking would be done by the engine and only at times when it is actually required.

The current implementation uses only one lock for the whole shared state of the audio engine. This is a pessimistic approach, but it is quite robust. Of course it would be more efficient to lock only the needed data structures independently, but this is also a lot more complicated. Say you have one lock for the audio driver and one lock for the playing pattern variable. If now one thread aquires the (write) lock of the playing pattern variable and then aquires the lock of the audio engine, and the other thread does the some actions but in reverse order, you have a deadlock. This is a problem we had frequently in the earlier time when i joined the Hydrogen project, i guess it was 0.9.4 or something before. Those problems were also very hard to debug and to inspect, since they often occured only in very special occasions (like this issue..)

There would be also the possibility to aquire all needed locks at once to solve the deadlock problem. But since locks are aquired in Hydrogen often in subfunctions (which call other functions...) , this seems not to be a possible solution.

The other "classic" solution would be to guarantee that all needed locks are aquired in the same order. This is as hard as the "aquire all locks at once" strategy for Hydrogen.

The central lock is for the audio engine easy to understand and easy to debug, which makes it quite well suited to such a hobbyist-project like Hydrogen. But yes: if we have performance issues with the central audio engine lock, we will have to investigate other solutions. I'm just afraid that those solutions will have to be vey well designed and tested.

[theGreatWhiteShark]

Ah, I see. You are totally right. It would make the code much harder to maintain.

[thijz]

fyi: been using this 'lock branch' version for quite some time now without any issues
is there anything else that needs to be tested before this fix can be applied to the master branch ?

[thijz]

bump ?

[mauser]

Hey Thijz!

Thanks a lot, i suppose we can close that issue now. The code is (along with another related bugfix..) now in the master branch.