crash instantiating synths

[spamatica]

Hi guys,

had some inspiration for music and started a very synth heavy piece. Had synths crashing sometimes on instantiating synths, from experience I know there are sometimes issues so I just restarted. But it happened a bit too frequent, then I remembered I was running debug and had enabled core dumping so I have a back trace.
Wasn't really in a mood to hunt right now but maybe one of you has any idea if anything has changed in recent master?

Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/ddskrjo/muse_debug/bin/muse4 -j -len'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007f95d2dabf07 in QKeySequence::operator==(QKeySequence const&) const () from /lib/x86_64-linux-gnu/libQt5Gui.so.5
[Current thread is 1 (Thread 0x7f95cb2ded00 (LWP 301322))]
(gdb) bt
#0 0x00007f95d2dabf07 in QKeySequence::operator==(QKeySequence const&) const () at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#1 0x00007f95d33b7fd6 in QAction::setShortcut(QKeySequence const&) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#2 0x00007f95d1c7e4a4 in MusEGui::EditToolBar::configChanged() (this=0x5614dbe33290)
at /home/ddskrjo/dev/muse/muse3/muse/components/tools.cpp:178
#3 0x00007f95d1c7ee3b in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (MusEGui::EditToolBar::)()>::call(void (MusEGui::EditToolBar::)(), MusEGui::EditToolBar*, void**)
(f=(void (MusEGui::EditToolBar::)(MusEGui::EditToolBar * const)) 0x7f95d1c7e37e MusEGui::EditToolBar::configChanged(), o=0x5614dbe33290, arg=0x7ffe7cf54d80) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:152
#4 0x00007f95d1c7edba in QtPrivate::FunctionPointer<void (MusEGui::EditToolBar::)()>::call<QtPrivate::List<>, void>(void (MusEGui::EditToolBar::)(), MusEGui::EditToolBar, void**)
(f=(void (MusEGui::EditToolBar::)(MusEGui::EditToolBar * const)) 0x7f95d1c7e37e MusEGui::EditToolBar::configChanged(), o=0x5614dbe33290, arg=0x7ffe7cf54d80) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:185
#5 0x00007f95d1c7ed20 in QtPrivate::QSlotObject<void (MusEGui::EditToolBar::)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (which=1, this_=0x5614dbe3c1d0, r=0x5614dbe33290, a=0x7ffe7cf54d80, ret=0x0)
at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:414
#6 0x00007f95d29ce458 in QMetaObject::activate(QObject*, int, int, void**) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#7 0x00007f95d3e5d5ab in MusEGui::MusE::configChanged() (this=0x5614cdff3d00)
at /home/ddskrjo/dev/muse-build_debug/muse/moc_app.cpp:776
#8 0x00007f95d3e80499 in MusEGui::MusE::changeConfig(bool) (this=0x5614cdff3d00, writeFlag=true)
at /home/ddskrjo/dev/muse/muse3/muse/app.cpp:3063
#9 0x00007f95d4009941 in MusECore::Song::addNewTrack(QAction*, MusECore::Track*) (this=
0x5614ce2eda10, action=0x5614d1cd75a0, insertAt=0x0) at /home/ddskrjo/dev/muse/muse3/muse/song.cpp:274
#10 0x00007f95d22de130 in MusEGui::TList::mousePressEvent(QMouseEvent*) (this=0x5614ce3dd8c0, ev=0x7ffe7cf55a90)
at /home/ddskrjo/dev/muse/muse3/muse/arranger/tlist.cpp:1873
#11 0x00007f95d33ff2d1 in QWidget::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x00007f95d22d4fe7 in MusEGui::TList::event(QEvent*) (this=0x5614ce3dd8c0, event=0x7ffe7cf55a90)
at /home/ddskrjo/dev/muse/muse3/muse/arranger/tlist.cpp:231
#13 0x00007f95d33bca66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007f95d33c6343 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00005614cbf37730 in MuseApplication::notify(QObject*, QEvent*)
(this=0x7ffe7cf564f0, receiver=0x5614ce3dd8c0, event=0x7ffe7cf55a90) at /home/ddskrjo/dev/muse/muse3/muse/main.cpp:164
#16 0x00007f95d29a293a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#17 0x00007f95d33c5457 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer&, bool, bool) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007f95d341b35d in () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#19 0x00007f95d341e1ec in () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x00007f95d33bca66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x00007f95d33c60f0 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00005614cbf37730 in MuseApplication::notify(QObject*, QEvent*)
(this=0x7ffe7cf564f0, receiver=0x5614d212beb0, event=0x7ffe7cf55f80) at /home/ddskrjo/dev/muse/muse3/muse/main.cpp:164
#23 0x00007f95d29a293a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#24 0x00007f95d2d8d7d3 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) ()
at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#25 0x00007f95d2d8f10b in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) ()

[spamatica]

So, I pressed on for a bit but attached qtcreator incase it happened again.
And it did, guessing some timing issue but not sure.

It crashes below at the arrow

void EditToolBar::configChanged() {

    initShortcuts();

    for (int i = 0; i < nactions; ++i) {
        Action *a = actions[i];
        ToolB* t = nullptr;
        for (unsigned j = 0; j < sizeof(toolList)/sizeof(*toolList); ++j) {
            if ((a->id() == (1 << j))) {
                t = &toolList[j];
                break;
            }
        }

        a->setShortcut(shortcuts[toolShortcuts[a->id()]].key);     <-----------------
        a->setToolTip(tr(t->tip) + " (" + a->shortcut().toString() + ")");
    }
}

because a->id() is 24 but toolShortcuts only contain 15 items.
The session timed out so that's as far as I got, thought I would report that back.
...Oh, it's a map, so the number of elements should not be the problem. Can't find an index 24 though.

[terminator356]

Hey I wonder if it might possibly have to do with my hotkeys issue 940 from yesterday.
@kybos has patched that, maybe it'll work now?

[kybos]

It looks more like I caused this problem with my patch yesterday... No idea why, will have a look.

[kybos]

I removed the edit toolbar shortcuts update for the time being.
Can this issue somehow be reproduced? I got no crashes at my tests yesterday...

[kybos]

Oh sorry @spamatica , I didn't notice your analysis, the lines were closed...
That makes it clearer, must have been my misunderstanding of the code.

[spamatica]

Alright, cool, hoppfully l will try it tonight! Den tors 7 jan. 2021 12:02kybos <[email protected]> skrev:

…

Oh sorry @spamatica <https://github.com/spamatica> , I didn't notice your analysis, the lines were closed... That makes it clearer, must have been my misunderstanding of the code. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/orgs/muse-sequencer/teams/musedevteam/discussions/66/comments/5>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCFANZ7ODMBRT6P22JSHYTSYWIEBANCNFSM4VYEDCYA> .

[kybos]

No fix so far... I thought it was some stupid mistake of mine, but no, the code looks ok.
You mention a->id() was 24 at the crash but I have no idea how that should happen - the id is filled with powers of 2 in the EditToolBar constructor ("1 << i") and never changed afterwards:

Action* a = new Action(actionGroup, 1 << i, tr(t->tip).toLatin1().data(), true);

When printing out the values in the configChanged function, they always look right in my tests with various toolbars:

*** Toolbar: Action ID: 1
*** Toolbar: Action ID: 2
*** Toolbar: Action ID: 4
*** Toolbar: Action ID: 8
*** Toolbar: Action ID: 16
*** Toolbar: Action ID: 64
*** Toolbar: Action ID: 128
*** Toolbar: Action ID: 2048
*** Toolbar: Action ID: 4096
*** Toolbar: Action ID: 1
*** Toolbar: Action ID: 2
*** Toolbar: Action ID: 4
*** Toolbar: Action ID: 8
*** Toolbar: Action ID: 32
*** Toolbar: Action ID: 64
*** Toolbar: Action ID: 128
*** Toolbar: Action ID: 16384
*** Toolbar: Action ID: 32768
*** Toolbar: Action ID: 1
*** Toolbar: Action ID: 2
*** Toolbar: Action ID: 4
*** Toolbar: Action ID: 64
*** Toolbar: Action ID: 128
*** Toolbar: Action ID: 1024
*** Toolbar: Action ID: 8192

So I guess I need some more hints on how it's happening and if possible how I can reproduce it.
Thanks.

[kybos]

It shouldn't crash any more now, but there should be error output in the console with the wrong id.

[spamatica]

Not sure I have run with the proper fix but I did run with the disabled function and have not seen any more crashes. Plugins can due to their nature cause all kinds of havoc but have so far not seen any such evidence so it looks safe.
I will run it more though and see if I can catch the print outs.

[spamatica]

New information. I'm lazy so haven't tried on multiple systems yet, but I can reproduce a similar crash with both debug and release.
If you have the time please see if you can reproduce:

1. start muse
2. create a midi track and add one part with one midi event
3. instantiate a synth (I tried fluidsynth and Dssi version of hexter)
   this consistently crashes on my system and I get one of those toolbar printouts.

fluidsynth sampleRate 48000
fluidsynth: warning: SDL2 not initialized, SDL2 audio driver won't be usable
Error: EditToolBar configChanged: Tool ID doesn't exist: 0
Error: EditToolBar configChanged: Tool ID doesn't exist: 0

Since it involves plugin loading I fear it might be memory leaks involved but I don't know,

Here's a snip from the backtrace.

Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007f3e1b469f07 in QKeySequence::operator==(QKeySequence const&) const ()
#1 0x00007f3e1ba75fd6 in QAction::setShortcut(QKeySequence const&)
#2 0x00007f3e1a3356d3 in MusEGui::EditToolBar::configChanged() (this=0x55f9ac678980)

[kybos]

I still can't reproduce it here.
But I did some changes, trying to make the code more robust, please try if it changed something.
Could be improved further, still too many unnecessary global variables involved etc.

[spamatica]

Tried on two different computers now and it happens every time on these (both have Kubuntu 20.04.1 LTS)
Didn't look to closely at the changes you did but I think the problem might be elsewhere and this is a symptom, overwritten memory maybe.

Inspecting a core dump I see the actions QMap QVector (which I think should contain static tool actions) at this point contain garbage.

		actions	<6 items>	QVector<MusEGui::Action*>
			[0]	@0x5620522f5270	MusEGui::Action
				[QAction]	@0x5620522f5270	QAction
				_id	1379442864	int
			[1]	@0x5620522f52f0	MusEGui::Action
				[QAction]	@0x5620522f52f0	QAction
				_id	575529392	int
			[2]	@0x5620522ed920	MusEGui::Action
				[QAction]	@0x5620522ed920	QAction
				_id	64	int
			[3]	@0x562052284fb0	MusEGui::Action
				[QAction]	@0x562052284fb0	QAction
				_id	0	int
			[4]	@0x5620522ea2b0	MusEGui::Action
				[QAction]	@0x5620522ea2b0	QAction
				_id	1380628976	int
			[5]	@0x5620522f8950	MusEGui::Action
				[QAction]	@0x5620522f8950	QAction
				_id	64	int

MusE printed
Error: EditToolBar configChanged: Tool ID doesn't exist: 1379442864
Error: EditToolBar configChanged: Tool ID doesn't exist: 575529392
Segmenteringsfel (minnesutskrift skapad)

And crashed on the next one with id = 64

[kybos]

No matter what I do, I still can't reproduce the crash. Tried even KDE (I have no Kubuntu, though).
I did some more changes, you can try it out.

It definitely looks like an external corruption of the memory, as the Action vector is filled once in the constructor and not changed anymore.

[spamatica]

Yes looks very much like memory corruption, will do more tests but it is very repeatable here.

Just to remove any possibility that the steps are unclear I attached a small video where I do the steps to make it crash.
https://user-images.githubusercontent.com/4476983/106198115-e5f4d480-61b3-11eb-8afa-6bc4ca0aeda3.mp4

[kybos]

I checked the video - yes, that's exactly what I did too. Quite a standard procedure, so it's surprising nobody else is complaining.
Does that happen with the AppImage as well?

[spamatica]

With pre1 appimage I don't see anything bad.
With pre2 it does not crash but I get these printouts that probably hint at the same problem:
Error: EditToolBar configChanged: Tool ID doesn't exist: -2147481595
Error: EditToolBar configChanged: Tool ID doesn't exist: 262196
Error: EditToolBar configChanged: Tool ID doesn't exist: 285468800
Error: EditToolBar configChanged: Tool ID doesn't exist: 0
Error: EditToolBar configChanged: Tool ID doesn't exist: 0
Error: EditToolBar configChanged: Tool ID doesn't exist: 0

[terminator356]

Saturning51 reported a crash when importing a midi file using pre1

If you mean this one:
https://linuxmusicians.com/viewtopic.php?f=61&t=22521
That is the one caused by duplicate controllers, which I partially fixed a few commits ago.

[ Edit: ] Oh wait, sorry, maybe that's not the one, it's that newer post from him, eh?

[spamatica]

No, it was the one discussed here that in the end was not repeatable
https://linuxmusicians.com/viewtopic.php?f=61&t=22588

[spamatica]

Woops didn't see the edit :P
Tried to find the commit where it starts to fail. It looked very promising, found a commit where it was triggered and that commit before did not.
But then I discovered that it was the same commit where kybos first tried to fix it, so by definition it cannot be that one...

I'm pretty sure that it started failing rather recently but if it IS memory corruption it could just as well be very old and changed memory layout is what started triggering it.

I will keep trying

[kybos]

My last attempt at cleaning up the EditToolBar code, removing the mix of different scopes involved etc.
Please try out. If it doesn't help, then I guess it really must be some other unrelated code corrupting the memory...

[spamatica]

Good with cleanups but unfortunately it did not help.

I have some new information though. It turns out I don't have to load a plugin, I can just as easily trigger it by clicking on some of the other toolbar buttons, so far I tried the Visible Track Types toolbar.

I recall having used a debugger for an embedded system where you could trigger a breakpoint when a memory area was accessed, I wonder if it's possible to do that with gdb? It must be possible to catch this somehow.

I can repeat this crash on three computers, all with the same Kubuntu 20.04, tried with a VM that had debian 10 and it does not crash. So I guess it is possible the problem is in the OS.

[spamatica]

And it looks like gdb does support that so I will check it out
https://stackoverflow.com/questions/58851/can-i-set-a-breakpoint-on-memory-access-in-gdb

[spamatica]

I might have found the actual problem.
Trying the watch stuff I realized that the toolbar class is instantiated in a lot of places and it was necessary that I close the pianoroll to trigger the problem.
So I figured maybe the configChanged qt-slot is triggered on a destroyed object.

But that is isn't it, though it is similar... The toolbar has a destructor but it is never called, infact the pianoroll has no destructor so nothing new:ed is deleted when the pianoroll goes.
After adding a destructor in the pianoroll and explicitly deleting the toolbar:
PianoRoll::~PianoRoll()
{
printf("kill pianoroll\n");
delete tools2;
}
I cannot recreate this scenario.

This did however leave me a bit scared, assuming this is close to the truth, there might be many more similar cases coming.

[spamatica]

Still, we've been pretty stable for a long time so I shouldn't sound too fearful :D
But it would be good to get your opinion. Maybe we should do some destructor maintenance.

[kybos]

That's a very important hint. Apparently the EditToolBar is not properly deleted when the Pianoroll is destroyed.
I'm not at my PC so can't test much ATM.
Explicit deletion in the Pianoroll destructor is not the right way for QObject though, this could cause more problems than it solves AFAIK. It must work through parenting, obviously there is something wrong with the parent tree.

[spamatica]

That sounds very reasonable. Not been able to test stuff now but I see this in the code, which looks like it is the problem.
EditToolBar(QWidget* /parent/, int /tools/, const char* name = 0); // Needs a parent !
questions is why it is commented out

[kybos]

I think these comments have no effect, the variables are there in the definition.
I was now able to reproduce the crash at last.
The toolbar is parented correctly in the Pianoroll constructor, but at some point the parent is lost. I suspect it's something to do with the toolbar sharing hacks, but I'm investigating further.

[kybos]

Tried lot of things but there seems to be no other way than to delete all the toolbars manually. They disappear from the parent tree of Pianoroll in the ctor already. I traced it - it happens exactly at canvas->setFocus(), which makes no sense, there must be some other reason.
So seems to be an old memory leak which now became obvious through the new EditToolBar::configChanged function. Please check if my latest fix works for you.

[spamatica]

Tried it now and my test scenario works fine now, thanks :)

Do you see any drawback to your solution? Or rather what I'm asking, should we put a comment with the reason for the explicit delete, in case it later causes problems?

[kybos]

I did some more tests and debugging and it seems ok. Still a good idea to put a comment in there, I will do it.
I also checked the editor-specific menus, but they are parented correctly and remain so until destroyed. So it's "only" all the (editor-specific) tool bars that have this problem.