Changing note pitch via Piano Roll editor causes mscore to eat %100 of a cpu

• Nov 15, 2017 - 09:15
Reported version
2.1
Type
Functional
Severity
S3 - Major
Status
closed
Project

Happens in both 2.1 release (windows & parabola linux) as well as master ce09d98 and 2.2 5bdaff8

reproduction steps:
1. new score
2. add a few notes
3. right click measure and select "Piano Roll Editor"
4. open up your computers system resource monitor
5. click on a note in piano roll
6. press up/down

result: your computer resource monitor now shows that mscore is using 100% of a core.


Comments

I've discovered that according to htop -H, the main mscore thread is reported to be consuming all the cpu usage of one core, while all the other threads are reporting 0% usage. But when I break execution, I find that main thread is simply at qApp->exec(); at the bottom of musescore.cpp main function. The rest of the code is qt code, but the lowest level of the stack trace is a function called "poll". I'm guessing there is some infinite loop of polling for events...here's the stack trace:

1 poll 0x7fffee951f7b
2 ?? 0x7fffea0e8ed3
3 g_main_context_iteration 0x7fffea0e8fae
4 QEventDispatcherGlib::processEvents qeventdispatcher_glib.cpp 423 0x7fffef7f4d41
5 QEventLoop::exec qeventloop.cpp 212 0x7fffef7984fb
6 QCoreApplication::exec qcoreapplication.cpp 1291 0x7fffef7a1548
7 main musescore.cpp 5900 0x555555dfda20

In reply to by ericfontainejazz

Paused and resumed execution a few times...stopped in same place, but then did it again and the stack trace is longer...must have gone beyond the poll system call?

1 QWeakPointer::data qsharedpointer_impl.h 565 0x7fffef799c7f
2 QPointer::data qpointer.h 86 0x7fffef799c7f
3 QPointer::operator QObject * qpointer.h 92 0x7fffef799c7f
4 QCoreApplicationPrivate::sendThroughApplicationEventFilters qcoreapplication.cpp 1104 0x7fffef799c7f
5 QApplicationPrivate::notify_helper qapplication.cpp 3697 0x7ffff04bf00a
6 QApplication::notify qapplication.cpp 3481 0x7ffff04c6aa6
7 QCoreApplication::notifyInternal2 qcoreapplication.cpp 1018 0x7fffef79a0d0
8 QCoreApplication::sendEvent qcoreapplication.h 233 0x7fffef7f4188
9 QTimerInfoList::activateTimers qtimerinfo_unix.cpp 643 0x7fffef7f4188
10 timerSourceDispatch qeventdispatcher_glib.cpp 182 0x7fffef7f49ba
11 idleTimerSourceDispatch qeventdispatcher_glib.cpp 229 0x7fffef7f49ba
12 g_main_context_dispatch 0x7fffea0e7270
13 ?? 0x7fffea0e8f69
14 g_main_context_iteration 0x7fffea0e8fae
15 QEventDispatcherGlib::processEvents qeventdispatcher_glib.cpp 423 0x7fffef7f4d41
16 QEventLoop::exec qeventloop.cpp 212 0x7fffef7984fb
17 QCoreApplication::exec qcoreapplication.cpp 1291 0x7fffef7a1548
18 main musescore.cpp 5900 0x555555dfda20

Well I don't know how much more knowledge I can glean from looking at disassembly of qt code...but my best guess is that musescore did something to Qt's event queue such that Qt's event dispatcher got stuck in an infinite loop. Next time I look at this I'll inspect just what code is performed when user changes pitch in the piano roll editor.

I can also produce this bug on a blank score by simply only pressing the enable repeats button with a blank score in Piano Roll editor (either toggling repeats on or off). I can also produce this bug when there is no audio sequencer (the fact that the button is not greyed out I've reported as a separate bug #265303: Piano Roll editor transport buttons (rewind, play, loop enable, play repeats, pan enable) are not greyed out when no sequencer, but should be).

bug does NOT happen when I:
- Delete a note
- Change Velocity QComboBox between User and Offset
- Press midi button
- click on different notes and empty space
- press metronome button
- press play button

bug DOES happen when I select a note & modify it by:
- pressing the down/up arrow for onTime or length.
- pressing a different voice button (1,2,3,4)

(aside, I just created a crash by pressing things in piano roll editor...)

even more interesting, bug happens even if I open and close piano roll editor, and then change a note's pitch in score view, even though the piano roll was closed before changing the pitch.

I'm noting that the PianoRoll object is created once, when the user first invokes the PianoRoll, and then that object persists throughout the rest of the life of MuseScore, even when closed, and is reused whenever the piano roll editor is re-invoked.

PianorollEditor's deconstructor is interestingly never called...even when I close musescore entirely! That means it is effectively dead code! I'm noticing there are some action->disconnect calls in the deconstructor...maybe these actions need to be disconnected...so maybe the fact that actions aren't being disconnected is why the bug persists even when closing all scores.

setting PianorollEditor attribute WA_DeleteOnClose does indeed cause PianorollEditor's deconstructor to be called upon closing the Piano Roll editor. (However it doesn't zero the main MuseScore object's pianoRollEditor pointer, which would be necessary to do to ensure that the next call to MuseScore::editInPianoroll will know that the object has been deleted (and hence would need to be recreated).

when I do force the deconstructor to be called, there are 5 actions in PianorollEditor's action, and they are disconnected:
1. "Delete"
2. "Up"
3. "Down"
4. "Up Octave"
5. "Down Octave"

Those correspond to the 5 calls to actions.append in PianorollEditor's constructor.

It turns out that invoking this deconstructor does indeed free the CPU if it was hung.

I'm going to look some more tomorrow...getting tired.

I'm noting that the DrumrollEditor does NOT have the corresponding problem. In other words, adding new drums after drumroll was opened will not cause the mscore process cpu usage to hit %100. So maybe if I study how the drumroll code differs from the pianoroll code, I can narrow down the suspects to how those two classes are different.

Because I'm not familiar with the source code, I usually only read what you write.
But it makes sense to compare these two editor codes.
I think: The only place they can not be similar is the location and placement of the drum instrument's notes.

you might not be able to help me too much on this one...seems to be a highly technical coding bug.

here are some of my observations looking at drumroll code: drumroll editor seems even more buggy than piano roll editor. For instance if I select a note in drumroll editor and pressing delete in drumroll, that delete does not become visible in scoreview until I do something like right click the drum staff and press the drumroll editor command again. So I'm wondering the reason why drumroll is not affected by the CPU bug is because the syncing code for drums is not running correctly to begin with. anyway, I'm noticnig that 90% of the code between drum roll and piano roll is identical...makes me think they should share a base class which handles all their similarities to simplify the code.

bug interestingly happens when clicking a note in scoreview and changing it's velocity offset or changing velocity from offset to user. Curiously, using piano roll editor to change from offset to user or change the velocity offset does NOT cause the bug. So I believe that narrows down the bug to be the communication direction MuseScore -> PianoRoll and not the other way around...

(drum roll editor does not have that issue at all)

Status (old) patch (code needs review) fixed
Status fixed

Fixed in branch 2.2, commit d64a2bd858

fix #265183 Piano Roll editor eats %100 CPU for most edits

If the PianoRoll editor window had been started during current musescore session, then many basic edits will cause the mscore process to hog 100% of a CPU core. Simply removing this 'delayed update' will avoid this problem. I do not know why this delayed update was here.

Fixed in branch master, commit 10e7789532

fix #265183 Piano Roll editor eats %100 CPU for most edits

If the PianoRoll editor window had been started during current musescore session, then many basic edits will cause the mscore process to hog 100% of a CPU core. Simply removing this 'delayed update' will avoid this problem. I do not know why this delayed update was here.