Copy-paste a multimeasure rest when the number exceeds the remaining measures causes a crash

• Feb 12, 2017 - 10:13
Reported version
2.2
Type
Functional
Severity
S2 - Critical
Status
closed
Project
MuseScore

GIT commit ca4d9e9 / Windows 7/10

1) Load this file: Test file1 crash.mscz
2) Create parts (new all → ok)
3) View part
4) Range selection of the first MM rest (image below)
photo 1 select.jpg
5) Copy-paste into the last measure

Result : crash

- Another one  : File test 1 bis.mscz
Same initial three first steps :
Then:
4) Range selection of the first MM rest (including the whole note, or not, no matter)
5) Copy-paste into the second MM rest

Result : crash

- Another one, a bit different, involving « R » key and with a part already in place (don’t know by now if it’s completely related or not) :

Steps :
1) Load this file: File test 2 R.mscz
2) View part
3) Range selection of the first MM rest
4) Press « R » key twice

Result : crash

- A last one but with a first result of corruption (maybe/likely different (?), because involving Undo, so I prefer to file more completely soon in another bug report)

1) Load this file (ie, the same of the first one, but after appending a measure): Test file 3 corruption.mscz
2) Create parts (new all → ok)
3) Range selection of the first MM rest
4) Copy-paste into the third measure (on the whole note)
5) Undo → press « M »

Result : corruption (the copyed measures have erased/ corrupted the pasted measures type 0/1)
corr.jpg

Comments

• Feb 13, 2017 - 09:59

The first attached test file fails (as on this other issue: #173381: Copy-paste a multimeasure rest on another one causes corruption after undo) also on December 12 (branch 2.1) with this nightly: e99be61

And not in December 12 (branch master: 01e5df4 ) and current 3.0 dev. ba6d304

So, maybe the same origin and mentioned commit in the other issue?

The test file: Test file1 crash.mscz
Recall of steps:
1) Load this file
2) Create parts (new all → ok)
3) View part
4) Range selection of the first MM rest
5) Copy-paste into the last measure
Result: crash

Reply

• Feb 13, 2017 - 10:14
Title Copy-paste a multimeasure rest hence the number exceeds the remaining measures causes a crash ⇒ Copy-paste a multimeasure rest when the number exceeds the remaining measures causes a crash

The second attached file fails also on December 12 (branch 2.1, same mentioned nigthly above). But also in branch master (December 12: 01e5df4) and current 3.0 dev. ba6d304

So, worths probably a further investigation and specific bug report.

The test file: File test 1 bis.mscz
Recall of steps:
1) Load this file
2) Create parts (new all → ok)
3) View part
4) Range selection of the first MM rest (including the whole note, or not, no matter)
5) Copy-paste into the second MM rest
Result: crash

Reply

• Feb 13, 2017 - 11:09

And, finally, about the third attached file, it fails also on December 12 (branch master: see mentioned commit above) et current 3.0 dev.

But there is a difference with the file in previous comment. The failure occurs not on December 12 (branch 2.1), but four days later, ie on December 16 with this nigthly: 213632d. So, maybe a side effect of this commit/nightly?

For fix: #151446: Copy-paste with "R " key before a system change with MM rests leads to corruption/crash
See specifically: https://musescore.org/en/node/151446#comment-617676

The test file: File test 2 R.mscz
Recall of steps :
1) Load this file: File test 2 R.mscz
2) View part
3) Range selection of the first MM rest
4) Press « R » key twice
Result: crash

In provisary conclusion, I would have preferred to report these bugs individually.

But they seemed really related (considering the fact of copying and pasting conjugated to the exceeding of the number of remaining measures), and the whole is really entangled and tricky :(

But, by investigating on each one, and with a specific comment for each one too, I hope it will help.

Reply

• Feb 15, 2017 - 06:37

About the attached file and issue described in comment #3: https://musescore.org/fr/node/173366#comment-654356, I note this issue occurs on last June 3, 2016.

- This nigthly works: 41b89ae
(with an unexpected result however after copy-paste the first MM rest and whole note into the second MM rest: ie an "extra" end barline after pasting)
unex.jpg

- With this nightly: crash: a41b109

Many commits this day.
But after seeing, I observe MM rests are involved, besides a41b109, in the two previous commits
No sure of course. But, maybe also?
- https://github.com/musescore/MuseScore/commit/48f88470632a5314ee79296e0…
- https://github.com/musescore/MuseScore/commit/fab01c5ec6822f89c92d7dd69…
(with a typo in the second one? ie "checkMeasue")

Reply

• Mar 13, 2017 - 21:56

I think the issue (no crash for me right now, but corruption in the score) is that the selection after the first "R" operation is bad. The endSegment is the clef segment of the mmrest. I think that's because tick2segment forces the "tick" parameter to the first segment of the mmrest (which is the clef in this case). I'm sure there is a good reason for that, but I don't know what. So I'm not sure what the fix should be. But at least, we could do a sanity check on the selection at least at the end of pasteStaff(), verifying that the end segment comes after the start or something like that.

Reply

• Mar 14, 2017 - 17:44
Status (old) active ⇒ patch (code needs review)

The following PR does the sanity check, and corrects the selection to do the right thing in this particular case:

https://github.com/musescore/MuseScore/pull/3061

A "real" fix would be to re-look at how we do the selection, but this whole area has been problematic for some time - lots of reports of crashes and corruptions over the years - and I think and more significant changes would be dangerous, as this has proven difficult to get right.

Note we could also fix the problem during the *second* paste, checking the validity of the selection then. But realistically, with a corrupt selection, we are likely to run into other problems as well. Another possibility is to do a sanity check in Selection::setRange(), in case there are other places where we try setting the end segment before the start.

Reply