chord list in default style modified by certain operations that load a chord list - can lead to crash

• May 23, 2013 - 16:26
Type
Functional
Severity
S2 - Critical
Status
closed
Project
MuseScore

Discovered first using self-built code, updated from master 5/22/2013. Reproduced using nightly build from before I added any of my code:

GIT commit: a7e207d

Steps to reproduce:

1) load uses_sym.mscx from attached archive
2) verify chords display with triangle and dash, in accordance with chord description style cchords_sym
3) close file
4) open xmlchords2.xml
5) open uses_sym.mscx again

Expected result: chords display as before - with triangle and dash
Actual result: chords now display as Cmaj7 and Cm7

I can try to look this myself in a while, since I'm about to be looking at import of chords from MusicXML as it is (which is why I ran into this). But this code is a bit further upstream that I was planning on touching. So if someone else wants to take my initial investigation and run with it, be my guest. I'm leaving it unassigned for now. But for whomever decides to tackle it (including "future me"):

From debugging and tracing through the code, it appears the act of loading xmlchords2.xml somehow alters the default style of MuseScore such that when uses_sym.mscx is subsequently loaded, the chord description file cchords_sym is not read.

Apparently, chord description files are not necessarily read when a score first loaded, but only on demand when the first chord is encountered. If a score has chords, that occurs immediately on the first attempt to render a chord. At that point, there is a call to StyleData::chordList that loads the chord description file if the chord list for the score is empty. Normally, on initial score load, this chord list *is* empty, so the chord description file specified in the score is loaded. But after loading xmlchords2.xml, all subsequently loaded scores find they already have a chordList, and thus their own chord description files are ignored.

It appears that loaded scores start off with a copy of defaultStyle, which normally has no chordList itself, but after a load of xml2chords.xml, the default style has been changed and now has the chordList from stdchords.xml.

Comments

• Jun 11, 2013 - 16:38
Title chord description file named in loaded MSCX/Z file not honored after load of certain MusicXML files ⇒ chord list in default style modified by certain operations that load a chord list - can lead to crash

My initial description of the problem and steps to reproduce remains accurate (at least with the code as it stands June 11, 2013), but I have new information. Sorry this is long and kind of confusing, but I think there is an important issue that needs to be addressed here, and it's a bit higher up in the code than I am comfortable trying to fix myself. So I am explaining what I see to be the case. Hopefully someone else can sort through all this and figure out what makes sense.

First, I have changed the title to better reflect what is going on. And I have upgraded the severity, because the condition described can also lead to a crash (the specific steps required will vary according to which build you test with; see below for a description of when to expect a crash).

The core problem is that the import of xmlchords2.xml changes the chord list in defaultStyle. This list should normally be empty, but importing a MusicXML file will populate it, and this chord list is then inherited by all scores loaded or new scores created afterwards, which leads to various different (bad) symptoms. It turns out there are other operations that can also populate defaultStyle's chord list, leading to a similar variety of problems down the road.

I'd like to focus on keeping defaultStyle from being inadvertently modified, rather than on any specific symptoms caused by this core problem. There are actually a number of symptoms you can see because the defaultStyle has been modified, but the details will differ according to which build you are using.

Some of the things that can modify defaultStyle's chord list:

As originally described, import of a MusicXML file that contains chord symbols modifies defaultStyle's chord list. Prior to pull request #382 (not yet merged as of this writing), that would trigger a load of chords.xml and stdchords.xml, and all 150+ chord id's would then be added to defaultStyle's chord list. Starting with pull request #382, it is chords_std.xml that is loaded and added to defaultStyle's chord list. This is mostly empty, but all chords actually contained in the imported file will also be added to the list, causing a different set of problems for future scores.

Import of a BIAB score will also modify defaultStyle's chord list, in a similar way. Both before and after pull request #382, importing a BIAB file loads chords.xml, and all the chord id's it defines then become part of defaultStyle's chord list, and thus inherited by all future scores.

Another operation that modifies defaultStyle's chord list is the initial reading of MuseScore's palettes on startup. One of the palette items is a Harmony object, and reading a Harmony object triggers a read of the default chord description file, which then gets added to defaultStyle's chord list. I should note that simply reading a Harmony object formerly did not cause a chord list to be read, but my recent changes to chord symbol handling now require this (to make sure all Harmony objects have a valid ChordDescription in the chord list).

Worse, upon *unloading* a score that inherited defaultStyle's chord list, the chord list is destroyed, potentially leaving defaultStyle with an invalid chord list for future scores to inherit. If so, then the next score loaded or created will then crash the first time a chord symbol is encountered. The exact conditions in which unloading a score destroys the defaultStyle's chord list vary according to which build you are using.

Now, as it turns out, the code I have checked in as pull request #382 actually works around these symptoms in some cases. At the end of StyleData::load, I check to see if a new chord description file was specified, and I delete the existing chord list from the score if so, thus triggering a load of the new chord description file the next time anyone tries to access the chord list. This code was originally added just to make sure loading an MSS file that specified a new chord description file would work - it didn't before, because with "on demand" loading of chord description files in 2.0, no one ever loaded the new chord file description since the old one was still in place. It just so happens this also covers some of the cases where the initial style inherited from defaultStyle would have already had a chord list attached. But not all of these cases.

Reply