Crash when loading file with orphan Segment tags

Confirmed, een with latest master, stack trace:
1 Ms::Segment::read segment.cpp 855 0x902189
2 Ms::Measure::readVoice measure.cpp 2370 0x8b0543
3 Ms::Measure::read measure.cpp 1968 0x8adf37
4 Ms::Score::readStaff scorefile.cpp 301 0x9c451d
5 Ms::Score::read read301.cpp 45 0xa3384a
6 Ms::MasterScore::read read301.cpp 277 0xa3503b
7 Ms::MasterScore::read301 read301.cpp 329 0xa35333
8 Ms::MasterScore::read1 scorefile.cpp 962 0x9c82bf
9 Ms::MasterScore::loadCompressedMsc scorefile.cpp 822 0x9c75f2
10 Ms::MasterScore::loadMsc scorefile.cpp 874 0x9c794d
11 Ms::MasterScore::loadMsc scorefile.cpp 865 0x9c7860
12 Ms::readScore file.cpp 2270 0x64d6b4
13 Ms::MuseScore::readScore file.cpp 349 0x63c16e
14 Ms::MuseScore::openScore file.cpp 327 0x63c034
15 Ms::MuseScore::loadFiles file.cpp 304 0x63bce2
16 Ms::MuseScore::cmd musescore.cpp 6035 0x4f92ee
17 Ms::MuseScore::cmd musescore.cpp 5843 0x4f84a9
18 Ms::MuseScore::qt_static_metacall moc_musescore.cpp 857 0x40a2a0
19 ZN11QMetaObject8activateEP7QObjectiiPPv 0x68c82085
20 ZN12QActionGroup7hoveredEP7QAction 0x26994ee5
...

I was able to circumvent the crash (in a debug build of mine) and save the file again, now it opens without crashing in 3.2.3

Code change:

diff --git a/libmscore/measure.cpp b/libmscore/measure.cpp
index 8b7713a45..6318c8db2 100644
--- a/libmscore/measure.cpp
+++ b/libmscore/measure.cpp
@@ -2366,7 +2366,7 @@ void Measure::readVoice(XmlReader& e, int staffIdx, bool irregular)
                         }
                   startingBeam = beam;
                   }
-            else if (tag == "Segment")
+            else if (tag == "Segment" && segment)
                   segment->read(e);
             else if (tag == "Ambitus") {
                   Ambitus* range = new Ambitus(score());

I think this just fixes the symptom rather than the cause though.

No idea, that's why I don't believe my 'fix' to be a good one.
I don't think it is related to tuplets though

Relevant (?) difference between the non-working and the working score:

$ diff -u ~/Desktop/Three_Etudes_1_copy[34].mscx
--- /c/Users/Jojo/Desktop/Three_Etudes_1_copy3.mscx     2019-08-17 18:55:18.000000000 +0200
+++ /c/Users/Jojo/Desktop/Three_Etudes_1_copy4.mscx     2019-08-17 19:16:06.000000000 +0200
@@ -8675,16 +8675,7 @@
           </voice>
         <voice>
           <location>
-            <fractions>1/2</fractions>
-            </location>
-          <location>
-            <fractions>1/4</fractions>
-            </location>
-          <Segment>
-            <leadingSpace>0.7</leadingSpace>
-            </Segment>
-          <location>
-            <fractions>1/16</fractions>
+            <fractions>13/16</fractions>
             </location>
           <StaffText>
             <offset x="3.10725" y="-1.78571"/>

Don't ask what that means, but it seems those 3 fractions got added up into one, losing that leadingSpace segment on the way.

Jojo is right. This has nothing to do with tuplets. And I think his fix is a good one.

I can reproduce this issue from scratch, with the following steps.

1. In a new score, enter two quarter rests in voice 2.
2. Increase the leading space of the second quarter rest's segment.
3. Delete the first quarter rest. Then delete the second quarter rest.
4. Save.

The modified segment will be written to file, but the gap rests will not. This means that upon load, segment will be equal to nullptr when the <Segment> tag is read.

Note that in step 3, if you delete the second quarter rest before deleting the first one, the segment will not be written to file, and so the resulting score will open normally.

Thanks for checking and confirming. See https://github.com/musescore/MuseScore/pull/5273

Question is whether(and jhow) we could prevent this from happening in the first place rather than just fixing it on reading such a score. Next question is whether such a rare scenario (non-voice 1 rests with leading space on the non-first one getting deleted in a certain order) is worth the effort, if we can just simply fix it on read?

if a voice has a note on current measure, don't let its rest(s) be erased/deleted, just make it invisible.

Ziya, I am pretty sure that is not the solution we are looking for.

One way to prevent this would be to not write the Segment for gap rests, since gap rests are not written.

diff --git a/libmscore/scorefile.cpp b/libmscore/scorefile.cpp
index 55927c885..65fa46baf 100644
--- a/libmscore/scorefile.cpp
+++ b/libmscore/scorefile.cpp
@@ -1287,7 +1287,8 @@ void Score::writeSegments(XmlWriter& xml, int strack, int etrack,
                         cr->writeTupletEnd(xml);
                         }
 
-                  segment->write(xml);    // write only once
+                  if (!(e->isRest() && toRest(e)->isGap()))
+                        segment->write(xml);    // write only once
                   if (forceTimeSig) {
                         if (segment->segmentType() == SegmentType::KeySig)
                               keySigWritten = true;

But I think that fixing it on read is good enough.

Hmm, that might be exactly what @dmitrio95 is asking for ;-)

This indeed seems to work well. I'll add it to my PR

Fixed in branch master, commit 32cbcfe034

_Fix #293460: Crash when loading file with orphan Segment tags

correct broken files on the fly and don't create them anymore._

Fixed in branch master, commit 5f5d0903de

_Merge pull request #5273 from Jojo-Schmitz/orphaned-segment

Fix #293460: Crash when loading file with orphan Segment tags_

Automatically closed -- issue fixed for 2 weeks with no activity.