Crash when saving with instrument name selected and continuous view

• Oct 21, 2015 - 14:03
Type
Functional
Severity
S2 - Critical
Status
closed
Project
Tags

See https://musescore.org/en/node/84176
Steps:
1- Open My_First_Score
2- Add one additional instrument (i.e. press "i" and add one instrument, e.g. flute)
3- Page view -> Continuous view
4- Left click one of the instrument (long) names found at the left of the staves
5- While the instrument name is still selected (still highlighted in blue), save the file -> crash
(or wait until autosave takes place -> crash)

I found that the presence of the vertical frame and the continuous view (in which the vertical frame is not visible) are necessary to trigger this crash.

From the backtrace, it crashes at the instruction clear() at line 270 of libmscore\select.cpp:270, inside function Ms::Selection::deselectAll.

Windows 8.1, commit 130c710

Attached the log from AddressSanitizer under Linux Mint 17.2 (64bit), under which the crash is reproducible as well.

The program is trying to access a deleted object. The object, i.e. the instrument name, was deleted at line 3110 of libmscore/layout.cpp when the staves are deleted.

Attachment Size
instrName_addressSanitizer_log.txt 10.31 KB

Comments

FWIW, it only crashes for me if I select the second (newly added) instrument, but it does crash the same way. It seems that this happens while creating the thumbnail, which involves first temporarily changing to page view and then undoing this operation. It's during the undo that the crash occurs. So my first thought was, we could implement a cheap fix by not doing an undo saving/restoring the original view.

However, while investigating this - without making any code changes - I also encountered a crash by simply switching back to page view manually. The crash occurs somewhere else - in ScoreAccessibility::currentInoChanged() - but the basic symptom is the same: the instrument name is selected but has been deleted, and when the accessibility code tries to print status for the selection, which is now garbage, bad things happen.

So I think probably the correct fix is to be careful when deleting instrument names, that we also unselect them.

But it's not as simple as just deselecting it while removing it. I guess we are probably trying to save and restore the original selection. This is getting a bit too hairy for me.

Well, I notice two steps in the time for this issue.

1) Until beginning January 13, all works fine all the time.

But with this Nightly (end January 13): 63c2174, by adding a sixth step, ie: just before leaving the score with "X" (so, I repeat after the 5 steps and save), if I switches again in Page View (with the long name instrument always highlighted, if not, works), I receive a crash.

I looked the dozen of commits this day, really I don't know where is the change:
Almost at random, sorry: here: https://github.com/musescore/MuseScore/pull/1625/files, or the previous Nightly mentionned initially?

Or, the previous commit: https://github.com/musescore/MuseScore/commit/151401e4b9810ccf954ca0133…
As you see, I am in the fog!

2) This behavior continues until July 8. The moment that I get a crash immediatly the 5 steps performed (after Save)

I read that it was cited the name of "thumbnail" above. This could correspond with this commit?
https://github.com/musescore/MuseScore/commit/883e38eb6b8eaef62d9f22ef5…

Others ideas without really know (complicated always in this period, commits numbers don't correspond with nightlies numbers)

Eg: https://github.com/musescore/MuseScore/commit/57deb726537c5673c00234bdf…
Or, https://github.com/musescore/MuseScore/commit/5fccfc5ccdc24335b6fe838fd…

In the same league. cc3b529c38

1/ Create a score with a vertical frame and two instrument
2/ switch to continuous view
3/ select instrument name
4/ switch to page view

--> crash

Poor man solution for this problem... Check if the selection contains an instrument name before changing the layout mode, if yes, deselect it...

LayoutMode is changed when switching from continuous view to page view, but also when printing, saving to graphical format or when creating a thumbnail. It would solve the crash, but it would have one weird side effect, the selection of an instrument name would disappear during autosave. Poor man again, we could disable the thumbnail creation for autosave...

The real problem, as explained by Marc, is that the instrument names is deleted during layout (which happen when we change views). It happens in the destructor of SysStaff when the sysStaff is deleted during layoutSystems, only when changing to page view.

I think the 2nd poor man's solution, to not create thumbnails on autosave, is not that much of a problem, those files are only (mainly?) needed after a crash and then would typically get saved again in a regular way, using the regulat filenam etc., this time with the thumbnail.
The 1st poos man's solution might be good enough for 2.0.3 maybe? At least far better than doing nothing at all. For master we may find a better solution later?

So by not deleting the instrument names, they automatically get re-used for the system on the switch back? I didn't try building this to test, but if works, great! It just doesn't seem obvious to me that this would be good enough in all cases. I'd still worry that new systems might be generated for whatever reason during the switch back to continuous view. I guess since it's actually an "undo" and not a new switch, that's why it is safe to assume the old instrument names will be re-used on the system?